Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-29 Thread Jakub Narębski
On Thu, May 29, 2014 at 4:33 AM, Pasha Bolokhov
pasha.bolok...@gmail.com wrote:

 Agree, but partial here means... what? just a little doc?

I'm sorry, I tried to be cute and failed :$

This is not official name for documentation files that are meant for inclusion
and not as standalone manpages.  I think they are similar to 'partial templates'
http://guides.rubyonrails.org/layouts_and_rendering.html#using-partials
- usually just called partials - hence my use of this name.

Example: date-formats.txt included in git-commit(1), git-commit-tree(1)
and git-tag(1).

 On Wed, May 28, 2014 at 11:53 AM, Jakub Narębski jna...@gmail.com wrote:
 W dniu 2014-05-27 19:16, Pasha Bolokhov pisze:
[...]
  I suggest this. There appears to be a notion of standard
 excludes both in the code (dir.c) and in the man pages (git-grep,
 git-ls-files). However, it doesn't actually appear to be defined
 strictly speaking. So my suggestion is to define those standard
 excludes in one place (say gitignore(5)), and make other man pages
 (git-config, git-grip, git-ls-files) have references to that place
 instead of explaining every time in detail what is being excluded.


 Or define those standard excludes in standard-excludes.txt partial,
 and include said file from git-grep(1), git-ls-files(1), and perhaps
 gitignore(5).

-- 
Jakub Narębski
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-28 Thread Jakub Narębski

W dniu 2014-05-27 19:16, Pasha Bolokhov pisze:

Add GIT_DIR to the list of excludes in setup_standard_excludes(),
while checking that GIT_DIR is not just '.git', in which case it
would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE


gives git-grep.txt and git-ls-files.txt. I don't know if we need to
add something about this extra exclude rule to those .txt. If it's so
obvious that this should be the expected behavior, then probably not.


 I suggest this. There appears to be a notion of standard
excludes both in the code (dir.c) and in the man pages (git-grep,
git-ls-files). However, it doesn't actually appear to be defined
strictly speaking. So my suggestion is to define those standard
excludes in one place (say gitignore(5)), and make other man pages
(git-config, git-grip, git-ls-files) have references to that place
instead of explaining every time in detail what is being excluded.


Or define those standard excludes in standard-excludes.txt partial,
and include said file from git-grep(1), git-ls-files(1), and perhaps 
gitignore(5).


--
Jakub Narębski


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-28 Thread Pasha Bolokhov
Agree, but partial here means... what? just a little doc?


On Wed, May 28, 2014 at 11:53 AM, Jakub Narębski jna...@gmail.com wrote:
 W dniu 2014-05-27 19:16, Pasha Bolokhov pisze:

 Add GIT_DIR to the list of excludes in setup_standard_excludes(),
 while checking that GIT_DIR is not just '.git', in which case it
 would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE

 gives git-grep.txt and git-ls-files.txt. I don't know if we need to
 add something about this extra exclude rule to those .txt. If it's so
 obvious that this should be the expected behavior, then probably not.


  I suggest this. There appears to be a notion of standard
 excludes both in the code (dir.c) and in the man pages (git-grep,
 git-ls-files). However, it doesn't actually appear to be defined
 strictly speaking. So my suggestion is to define those standard
 excludes in one place (say gitignore(5)), and make other man pages
 (git-config, git-grip, git-ls-files) have references to that place
 instead of explaining every time in detail what is being excluded.


 Or define those standard excludes in standard-excludes.txt partial,
 and include said file from git-grep(1), git-ls-files(1), and perhaps
 gitignore(5).

 --
 Jakub Narębski


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-27 Thread Pasha Bolokhov
 Add GIT_DIR to the list of excludes in setup_standard_excludes(),
 while checking that GIT_DIR is not just '.git', in which case it
 would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE

 gives git-grep.txt and git-ls-files.txt. I don't know if we need to
 add something about this extra exclude rule to those .txt. If it's so
 obvious that this should be the expected behavior, then probably not.

I suggest this. There appears to be a notion of standard
excludes both in the code (dir.c) and in the man pages (git-grep,
git-ls-files). However, it doesn't actually appear to be defined
strictly speaking. So my suggestion is to define those standard
excludes in one place (say gitignore(5)), and make other man pages
(git-config, git-grip, git-ls-files) have references to that place
instead of explaining every time in detail what is being excluded.
Now, gitignore(5) actually does have this list of ignored items, we
only need to call it standard excludes.
If done this way, then all that needs to be done regarding GIT_DIR
is to insert it into that list in gitignore(5). Please let me know if
that'd work

Pasha
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-27 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Sat, May 24, 2014 at 12:33 AM, Pasha Bolokhov
 pasha.bolok...@gmail.com wrote:
 When an explicit '--git-dir' option points to a directory inside
 the work tree, git treats it as if it were any other directory.
 In particular, 'git status' lists it as untracked, while 'git add -A'
 stages the metadata directory entirely

 Add GIT_DIR to the list of excludes in setup_standard_excludes(),
 while checking that GIT_DIR is not just '.git', in which case it
 would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE

 Although an analogous comparison of any given path against '.git'
 is done in treat_path(), this does not seem to be the right place
 to compare against GIT_DIR. Instead, the excludes provide an
 effective mechanism of ignoring a file/directory, and adding GIT_DIR
 as an exclude is equivalent of putting it into '.gitignore'. Function
 setup_standard_excludes() was chosen because that is the place where
 the excludes are initialized by the commands that are concerned about
 excludes

 I like this approach. A search of exclude-standard in Documentation/
 gives git-grep.txt and git-ls-files.txt. I don't know if we need to
 add something about this extra exclude rule to those .txt. If it's so
 obvious that this should be the expected behavior, then probably not.

OK, so is that an Acked/Reviewed-by?


 The case of git grep --exclude-standard is interesting because it's
 intended to work without a repository. First reaction was would
 get_git_dir() return NULL in that case. But it should return .git so
 we're good.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-27 Thread Pasha Bolokhov
I've sent out version [PATCH v4] with most of the things addressed.
That one hasn't been reviewed by anyone yet


On Tue, May 27, 2014 at 11:04 AM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@gmail.com writes:

 On Sat, May 24, 2014 at 12:33 AM, Pasha Bolokhov
 pasha.bolok...@gmail.com wrote:
 When an explicit '--git-dir' option points to a directory inside
 the work tree, git treats it as if it were any other directory.
 In particular, 'git status' lists it as untracked, while 'git add -A'
 stages the metadata directory entirely

 Add GIT_DIR to the list of excludes in setup_standard_excludes(),
 while checking that GIT_DIR is not just '.git', in which case it
 would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE

 Although an analogous comparison of any given path against '.git'
 is done in treat_path(), this does not seem to be the right place
 to compare against GIT_DIR. Instead, the excludes provide an
 effective mechanism of ignoring a file/directory, and adding GIT_DIR
 as an exclude is equivalent of putting it into '.gitignore'. Function
 setup_standard_excludes() was chosen because that is the place where
 the excludes are initialized by the commands that are concerned about
 excludes

 I like this approach. A search of exclude-standard in Documentation/
 gives git-grep.txt and git-ls-files.txt. I don't know if we need to
 add something about this extra exclude rule to those .txt. If it's so
 obvious that this should be the expected behavior, then probably not.

 OK, so is that an Acked/Reviewed-by?


 The case of git grep --exclude-standard is interesting because it's
 intended to work without a repository. First reaction was would
 get_git_dir() return NULL in that case. But it should return .git so
 we're good.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-23 Thread Junio C Hamano
Pasha Bolokhov pasha.bolok...@gmail.com writes:

 diff --git a/dir.c b/dir.c
 index 98bb50f..76969a7 100644
 --- a/dir.c
 +++ b/dir.c
 @@ -1588,6 +1588,26 @@ void setup_standard_excludes(struct dir_struct *dir)
  {
   const char *path;
   char *xdg_path;
 + const char *gitdir = get_git_dir();
 +
 + /* Add git directory to the ignores first */
 + if (strcmp(gitdir, .git) != 0) {

It is more idiomatic and common to say

if (strcmp(gitdir, .git)) {

around here, I think.

 /* --git-dir has been given */

... or it could have come from GIT_DIR environment, no?

Does this additional exclude need to kick in if GIT_DIR is set to
/home/pasha/w/.git?  That is, when gitdir is .git or ends with
/.git?

 + char ngit[PATH_MAX + 1];

Hmmm.  As I recall that recently we had flurry of changes to
eradicate PATH_MAX from our codebase, I am not happy to see an
introduction of a new buffer that is fixed-size.

 + /*
 +  * See if GIT_DIR is inside the work tree; need to normalize
 +  * 'gitdir', whereas 'get_git_work_tree()' always appears
 +  * absolute and normalized
 +  */
 + normalize_path_copy(ngit, real_path(absolute_path(gitdir)));
 +
 + if (dir_inside_of(ngit, get_git_work_tree()) = 0) {
 + struct exclude_list *el = add_exclude_list(dir, 
 EXC_CMDL,
 + --git-dir option);
 +
 + add_exclude(gitdir, , 0, el, 0);
 + }
 + }
  
   dir-exclude_per_dir = .gitignore;
   path = git_path(info/exclude);
 diff --git a/t/t2205-add-gitdir.sh b/t/t2205-add-gitdir.sh
 new file mode 100755
 index 000..0c99508
 --- /dev/null
 +++ b/t/t2205-add-gitdir.sh
 @@ -0,0 +1,84 @@
 +#!/bin/sh
 +#
 +# Copyright (c) 2014 Pasha Bolokhov
 +#
 +
 +test_description='alternative repository path specified by --git-dir is 
 ignored by add and status'
 +
 +. ./test-lib.sh
 +
 +#
 +# Create a tree:
 +#
 +#a  b  c  d  subdir/
 +#
 +# subdir:
 +#e  f  g  h  meta/  ssubdir/
 +#
 +# subdir/meta:
 +#aa
 +#
 +# subdir/ssubdir:
 +#meta/
 +#
 +# subdir/ssubdir/meta:
 +#aaa
 +#
 +# Name the repository meta and see whether or not git status includes
 +# or ignores directories named meta. The slightly deeper hierarchy is
 +# needed in order to be able to put the repository into ../meta, that is,
 +# outside the work tree and still have files called meta within the tree
 +#

It is not quite clear with this large blob of comment what are
noises and what exactly are being tested.  I think you have two
directories called meta, but which one is the repository?  Or do
you have yet another one next to {a,b,c,d,subdir} called meta that
is not listed above?

Given that the reason why people use --git-dir is so that they can
put it completely outside the working tree (in which case, the usual
start from cwd and go upwards while trying to see if there is .git/
that governs the working tree logic would not work), readers would
not expect to find the directory to be used as GIT_DIR in the
hierarchy you are creating in the first place.  Because of that, it
is even more important to clearify which meta you mean to use as
your GIT_DIR if you want to be understood by readers.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-23 Thread Pasha Bolokhov
   Hi,


 /* --git-dir has been given */

 ... or it could have come from GIT_DIR environment, no?
Yes, it does not matter where it came from, but I'll correct the comment


 Does this additional exclude need to kick in if GIT_DIR is set to
 /home/pasha/w/.git?  That is, when gitdir is .git or ends with
 /.git?
I don't think it needs to kick in in either of these cases, as
.git is already handled by treat_path(). Now, here .git is
excluded by if (strcmp()) {, while the first case needs to be
addressed too. Agree.

 +#
 +# Create a tree:
 +#
 +#a  b  c  d  subdir/
 +#
 +# subdir:
 +#e  f  g  h  meta/  ssubdir/
 +#
 +# subdir/meta:
 +#aa
 +#
 +# subdir/ssubdir:
 +#meta/
 +#
 +# subdir/ssubdir/meta:
 +#aaa
 +#
 It is not quite clear with this large blob of comment what are
 noises and what exactly are being tested.  I think you have two
 directories called meta, but which one is the repository?  Or do
 you have yet another one next to {a,b,c,d,subdir} called meta that
 is not listed above?

 Given that the reason why people use --git-dir is so that they can
 put it completely outside the working tree (in which case, the usual
 start from cwd and go upwards while trying to see if there is .git/
 that governs the working tree logic would not work), readers would
 not expect to find the directory to be used as GIT_DIR in the
 hierarchy you are creating in the first place.  Because of that, it
 is even more important to clearify which meta you mean to use as
 your GIT_DIR if you want to be understood by readers.

I guess I was too simplistic, need to clarify a bit. And indeed,
perhaps two distinct subtrees are needed to test the repository that
is outside the work-tree, that would just do a slightly cleaner job

Pasha
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

2014-05-23 Thread Duy Nguyen
On Sat, May 24, 2014 at 12:33 AM, Pasha Bolokhov
pasha.bolok...@gmail.com wrote:
 When an explicit '--git-dir' option points to a directory inside
 the work tree, git treats it as if it were any other directory.
 In particular, 'git status' lists it as untracked, while 'git add -A'
 stages the metadata directory entirely

 Add GIT_DIR to the list of excludes in setup_standard_excludes(),
 while checking that GIT_DIR is not just '.git', in which case it
 would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE

 Although an analogous comparison of any given path against '.git'
 is done in treat_path(), this does not seem to be the right place
 to compare against GIT_DIR. Instead, the excludes provide an
 effective mechanism of ignoring a file/directory, and adding GIT_DIR
 as an exclude is equivalent of putting it into '.gitignore'. Function
 setup_standard_excludes() was chosen because that is the place where
 the excludes are initialized by the commands that are concerned about
 excludes

I like this approach. A search of exclude-standard in Documentation/
gives git-grep.txt and git-ls-files.txt. I don't know if we need to
add something about this extra exclude rule to those .txt. If it's so
obvious that this should be the expected behavior, then probably not.

The case of git grep --exclude-standard is interesting because it's
intended to work without a repository. First reaction was would
get_git_dir() return NULL in that case. But it should return .git so
we're good.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html