Re: [PATCH v2] status: always report ignored tracked directories

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2013, 19:06 UTC
Message-ID: <7vip78izir.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1357510179-22852-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

First, thanks for working on this.

The explanation is a bit confusing, especially for people like me, as it does not make it clear that there are two kinds of "paths in the index". Files can be added to the index ("git add" and then shown via "ls-files") and these are what we usually call "files in the index". But there is a separate "name hash" that keeps track of "paths the index knows about" in play, and that is what is discussed in the description.

Show 6 quoted lines

> Tracked directories (i.e. directories containing tracked files)
> that are ignored must be reported as ignored if they contain
> untracked files.
>
> Currently, files in the index can't be reported as ignored and are
> automatically dropped from the list:

"file in the index" will never be ignored, period. Some paths the index knows about, namely, directory names in name hash, may need to be reported as ignored.

It is not clear what "list" the above "dropped from the list" refers to. The list of paths that becomes the result of "status"?

Show 16 quoted lines

>  - When core.ignorecase is false, directories (which are not directly
>  tracked) are not listed as part of the index, and the directory can be
>  shown as ignored.
>
>  - When core.ignorecase is true on the other hand, directories are
>  reported as part of the index, and the directory is dropped, thus not
>  displayed as ignored.
>
> Fix that behavior by allowing indexed file to be added when looking for
> ignored files.
>
>  - Ignored tracked and untracked directories are treated the same way
>  when looking for ignored files, so we should not care about their index
>  status.
>  - Files are dismissed by treat_file() if they belong to the
>  index, so that step would have been a no-op anyway.

Here is my attempt...

    When enumerating paths that are ignored, paths the index knows
    about are not included in the result.  The "index knows about"
    check is done by consulting the name hash, not the actual
    contents of the index:

     - When core.ignorecase is false, directory names are not in the
       name hash, and ignored ones are shown as ignored (directories
       can never be tracked anyway).

     - When core.ignorecase is true, however, the name hash keeps
       track of the names of directories, in order to detect
       additions of the paths under different cases.  This causes
       ignored directories to be mistakenly excluded when
       enumerating ignored paths.

    Stop excluding directories that are in the name hash when
    looking for ignored files in dir_add_name(); the names that are
    actually in the index are excluded much earlier in the callchain
    in treat_file(), so this fix will not make them mistakenly
    identified as ignored.

    Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
    Reviewed-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/dir.c b/dir.c
index 9b80348..f836590 100644
--- a/dir.c
+++ b/dir.c
@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)
 
 static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
 {
-	if (cache_name_exists(pathname, len, ignore_case))
+	if (!(dir->flags & DIR_SHOW_IGNORED) &&
+	    cache_name_exists(pathname, len, ignore_case))
 		return NULL;
 
 	ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
@@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,
 	if (exclude)
 		exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
 	else if (dir->flags & DIR_SHOW_IGNORED) {
-		/*
-		 * Optimization:
-		 * Don't spend time on indexed files, they won't be
-		 * added to the list anyway
-		 */
+		/* Always exclude indexed files */
 		struct cache_entry *ce = index_name_exists(&the_index,
 		    path->buf, path->len, ignore_case);