Wrong gitignore precedence?

Wrong gitignore precedence?

[Yohei Endo]

Hello All,

I read the document of gitignore (http://git-scm.com/docs/gitignore),
and learned that $GIT_DIR/info/exclude has higher precedence than
the file specified by core.excludesfile.

But I noticed that patterns in core.excludesfile override patterns in
$GIT_DIR/info/exclude.

I tested as below:

   1. Make a new git repository for test, and move into the repository.
      $ git init testrepo
      $ cd testrepo
   2. Change core.excludesfile configuration.
      $ touch ../core_excludesfile
      $ git config core.excludesfile `realpath ../core_excludesfile`
   3. Create test~. In each step I check if the file is ignored or not.
      $ touch test~
   4. See git status. In this case I expect that test~ should not be ignored.
      $ git status --ignored
   5. Change settings in .git/info/exclude.
      $ echo '*~' > .git/info/exclude
   6. See git status. In this case I expect that test~ should be ignored.
      $ git status --ignored
   7. Change settings in .git/info/exclude and core.excludesfile.
      $ echo '*~' > ../core_excludesfile
      $ echo '!*~' > .git/info/exclude
   8. See git status. In this case I expect that test~ should not be ignored.
      $ git status --ignored
   9. Change settings in .git/info/exclude and core.excludesfile
      $ echo '!*~' > ../core_excludesfile
      $ echo '*~' > .git/info/exclude
  10. See git status. In this case I expect that test~ should be ignored.
      $ git status --ignored

Steps 4. and 6. worked as I expected, but 8. and 10. didn't.

I read the source code of Git, and found out the point that seems to
cause the problem.

In dir.c, setup_standard_excludes() adds patterns in .git/info/exclude to
the excludes list first, and patterns in core.excludesfile are added next.

In last_exclude_matching_from_list(), pattern is searched from the end of
the list, and the first matching pattern is used. Therefore the patterns
in core.excludesfile are used in precedence to .git/info/exclude.

To meet the precedence described in the document of gitignore, I guess
setup_standard_excludes() should be fixed so that patterns in
core.excludesfile are added to the list before those in
.git/info/ecdlude are added.

Thanks

-- 
  遠藤 陽平 (Yohei Endo)
    yoheie@gmail.com

Re: Wrong gitignore precedence?

[Junio C Hamano]

Yohei Endo <yoheie@gmail.com> writes:

> I read the document of gitignore (http://git-scm.com/docs/gitignore),
> and learned that $GIT_DIR/info/exclude has higher precedence than
> the file specified by core.excludesfile.
>
> But I noticed that patterns in core.excludesfile override patterns in
> $GIT_DIR/info/exclude.

I tend to agree that info/exclude which is per-repository personal
preference should take precedence over $XDG_HOME/git/ignore which is
a personal preference across repositories that are accessed from
that machine.

It appears that the precedence was screwed-up between these two
files from the very beginning when 896bdfa2 (add: Support specifying
an excludes file with a configuration variable, 2007-02-27)
introduced core.excludesfile variable; seeing that nobody so far
complained with the discrepancy between the documentation and the
behaviour, it would indicate either (1) nobody reads the docs, or
(2) nobody uses both at the same time.

Swapping the order in the code this late in the game after 8 years
may affect people who have come to rely on the current behaviour and
never read the doc, which is somewhat worrying, though.

[PATCH] ignore: info/exclude should trump core.excludesfile

[Junio C Hamano]

$GIT_DIR/info/exclude and core.excludesfile (which falls back to
$XDG_HOME/git/ignore) are both ways to override the ignore pattern
lists given by the project in .gitignore files.  The former, which
is per-repository personal preference, should take precedence over
the latter, which is a personal preference default across different
repositories that are accessed from that machine.  The existing
documentation also agrees.

However, the precedence order was screwed up between these two from
the very beginning when 896bdfa2 (add: Support specifying an
excludes file with a configuration variable, 2007-02-27) introduced
core.excludesfile variable.

Noticed-by: Yohei Endo <yoheie@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is done on an old codebase and may not apply cleanly to more
   modern codebase easily.

 dir.c              | 10 +++++++---
 t/t0008-ignores.sh | 10 ++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 23b6de4..e67b6f9 100644
--- a/dir.c
+++ b/dir.c
@@ -1530,15 +1530,19 @@ void setup_standard_excludes(struct dir_struct *dir)
 	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
-	path = git_path("info/exclude");
+
+	/* core.excludefile defaulting to $XDG_HOME/git/ignore */
 	if (!excludes_file) {
 		home_config_paths(NULL, &xdg_path, "ignore");
 		excludes_file = xdg_path;
 	}
-	if (!access_or_warn(path, R_OK, 0))
-		add_excludes_from_file(dir, path);
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_excludes_from_file(dir, excludes_file);
+
+	/* per repository user preference */
+	path = git_path("info/exclude");
+	if (!access_or_warn(path, R_OK, 0))
+		add_excludes_from_file(dir, path);
 }
 
 int remove_path(const char *name)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index b4d98e6..38405de 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -775,4 +775,14 @@ test_expect_success PIPE 'streaming support for --stdin' '
 	echo "$response" | grep "^::	two"
 '
 
+test_expect_success 'info/exclude trumps core.excludesfile' '
+	echo >>global-excludes usually-ignored &&
+	echo >>.git/info/exclude "!usually-ignored" &&
+	>usually-ignored &&
+	echo "?? usually-ignored" >expect &&
+
+	git status --porcelain usually-ignored >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.4.0-rc3-227-gd45ce82

Re: Wrong gitignore precedence?

[Yohei Endo]

On Wed, 22 Apr 2015 11:59:04 -0700
Junio C Hamano <gitster@pobox.com> wrote:

> Swapping the order in the code this late in the game after 8 years
> may affect people who have come to rely on the current behaviour and
> never read the doc, which is somewhat worrying, though.

I agree. I think the change should be well announced before
the fix is applied, like when the implicit value of
push.default was changed.

Thanks.
-- 
  遠藤 陽平 (Yohei Endo)
    yoheie@gmail.com