[BUG] git status doesn't handle submodules properly on OSX

[BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

Greetings all,

two days ago I posted about an issue:
http://thread.gmane.org/gmane.comp.version-control.git/98171

After more testing I can confirm it is indeed a bug.

1.6.0.2 on OSX Leopard doesn't work. After adding a submodule
the folder of the submodule will always get listed under
"Untracked files" when calling git status.

1.5.6, however, works fine. I have not tested trunk yet.

So as a workaround I have to go back to 1.5.6 for now.

Yours,
Lars

Re: [BUG] git status doesn't handle submodules properly on OSX

[Jeff King]

On Wed, Oct 15, 2008 at 02:07:17PM +0200, Lars Hoss wrote:

> two days ago I posted about an issue:
> http://thread.gmane.org/gmane.comp.version-control.git/98171
> 
> After more testing I can confirm it is indeed a bug.
> 
> 1.6.0.2 on OSX Leopard doesn't work. After adding a submodule
> the folder of the submodule will always get listed under
> "Untracked files" when calling git status.
> 
> 1.5.6, however, works fine. I have not tested trunk yet.

I wasn't able to reproduce your bug on my Linux box. Can you bisect to
find the faulty commit?

-Peff

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

> I wasn't able to reproduce your bug on my Linux box. Can you bisect to
> find the faulty commit?

Yes, you are right. This bug does neither appear on Linux nor on Windows
(mysysgit). So far it seems to be a OSX only bug.

Never used bisect myself before since I am new to git. But will try to do
my best - hehehe :)

Yours,
Lars

Re: [BUG] git status doesn't handle submodules properly on OSX

[Pieter de Bie]

On 15 okt 2008, at 14:07, Lars Hoss wrote:

> two days ago I posted about an issue:
> http://thread.gmane.org/gmane.comp.version-control.git/98171
>
> After more testing I can confirm it is indeed a bug.
>
> 1.6.0.2 on OSX Leopard doesn't work. After adding a submodule
> the folder of the submodule will always get listed under
> "Untracked files" when calling git status.
>
> 1.5.6, however, works fine. I have not tested trunk yet.
>
> So as a workaround I have to go back to 1.5.6 for now.

Works for me on Leopard

Vienna:a pieter$ git submodule add ~/projects/GitX/ gitx
Initialized empty Git repository in /Users/pieter/a/gitx/.git/
Vienna:a pieter$ git st
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#	new file:   .gitmodules
#	new file:   gitx
#
Vienna:a pieter$ git --version
git version 1.6.0.2.415.gf9137

- Pieter

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

> Works for me on Leopard
>
> Vienna:a pieter$ git submodule add ~/projects/GitX/ gitx
> Initialized empty Git repository in /Users/pieter/a/gitx/.git/
> Vienna:a pieter$ git st
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #	new file:   .gitmodules
> #	new file:   gitx
> #
> Vienna:a pieter$ git --version
> git version 1.6.0.2.415.gf9137

My git version "1.6.0.2" says:

# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#	new file:   .gitmodules
#	new file:   lib
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#	lib/
git --version
git version 1.6.0.2

Git was build from macports.

Yours,
Lars

Re: [BUG] git status doesn't handle submodules properly on OSX

[Pieter de Bie]

On 15 okt 2008, at 16:51, Lars Hoss wrote:
> My git version "1.6.0.2" says:
>
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #	lib/
> git --version
> git version 1.6.0.2


I just tested 1.6.0.2, and had no problems. Perhaps macports is doing  
something odd?

Re: [BUG] git status doesn't handle submodules properly on OSX

[Richard Bubel]

Hi,

On Oct 15, 2008, at 16:51 , Lars Hoss wrote:

>> Works for me on Leopard
>> [...]
>> Vienna:a pieter$ git --version
>> git version 1.6.0.2.415.gf9137
>
> My git version "1.6.0.2" says:
>
> # On branch master
> [...]
> git --version
> git version 1.6.0.2
>
> Git was build from macports.


works for me too on OS X 10.5.5 with git 1.6.0.2 from MacPorts. As it  
deviates from the default, it might be worth mentioning that the  
filesystem in use here is the case-sensitive version of HFS+.

Best Regards,
   Richard

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

> I just tested 1.6.0.2, and had no problems. Perhaps macports is doing
> something odd?

Actually this was my idea first. Thus I build git from the sources the
portfile links to myself. The bug, however, remains.

The link is: http://www.kernel.org/pub/software/scm/git/git-1.6.0.2.tar.bz2.

I've tested this on two machines, both using latest version of Leopard.
And there was another poster on this forum who had the very same issue.

Now the interesting question is what is different between Pieter's system
and mine? And why does 1.5.6 work fine?

As soon as I am at home I will try bisect. I am currently at work behind a
big bad firewall ;)

Yours,
Lars

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

Ok, this might be the difference. I am pretty sure my
filesystem is not case-sensitive (default?).
At least "mkdir Foo" fails when "foo" exists.

Yours,
Lars

> Hi,
>
> On Oct 15, 2008, at 16:51 , Lars Hoss wrote:
>
>>> Works for me on Leopard
>>> [...]
>>> Vienna:a pieter$ git --version
>>> git version 1.6.0.2.415.gf9137
>>
>> My git version "1.6.0.2" says:
>>
>> # On branch master
>> [...]
>> git --version
>> git version 1.6.0.2
>>
>> Git was build from macports.
>
>
> works for me too on OS X 10.5.5 with git 1.6.0.2 from MacPorts. As it
> deviates from the default, it might be worth mentioning that the
> filesystem in use here is the case-sensitive version of HFS+.
>
> Best Regards,
>    Richard
>

Re: [BUG] git status doesn't handle submodules properly on OSX

[Pedro Melo]

Hi,

On Oct 15, 2008, at 1:07 PM, Lars Hoss wrote:

> Greetings all,
>
> two days ago I posted about an issue:
> http://thread.gmane.org/gmane.comp.version-control.git/98171
>
> After more testing I can confirm it is indeed a bug.
>
> 1.6.0.2 on OSX Leopard doesn't work. After adding a submodule
> the folder of the submodule will always get listed under
> "Untracked files" when calling git status.

I'm using  1.6.0.2.526.g5c283 on Leopard with lots of submodules  
without any problems at all.

I was going to reply to your original message but it slipped my mind  
(commuting et al).

Can you provide a sequence of commands that demonstrate the problem so  
that I can try?

Best regards,
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

Re: [BUG] git status doesn't handle submodules properly on OSX

[Pedro Melo]

On Oct 15, 2008, at 3:51 PM, Lars Hoss wrote:

>> Works for me on Leopard
>>
>> Vienna:a pieter$ git submodule add ~/projects/GitX/ gitx
>> Initialized empty Git repository in /Users/pieter/a/gitx/.git/
>> Vienna:a pieter$ git st
>> # On branch master
>> # Changes to be committed:
>> #   (use "git reset HEAD <file>..." to unstage)
>> #
>> #	new file:   .gitmodules
>> #	new file:   gitx
>> #
>> Vienna:a pieter$ git --version
>> git version 1.6.0.2.415.gf9137
>
> My git version "1.6.0.2" says:
>
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #	new file:   .gitmodules
> #	new file:   lib
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #	lib/
> git --version
> git version 1.6.0.2
>
> Git was build from macports.

hmms.. Really, I know about fink and Macports and all others, but I'm  
using Mac OS X since version 10.1 (when compiling OSS software was  
near impossible due to Apple decisions) and I've always had better  
results with compiling my own than using any of those OSS repositories.

I understand the appeal, it seems easier, but I would recommend that  
you compile yourself the software packages you depend on.

Regarding git, I saw a git-build.sh script floating around. If you  
cannot find it, you might want to try my own recipe (http://tinyurl.com/4ayze6 
). I compile git master every day (cron) and thats the one I use on my  
main machine.

Best regards,
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

Re: [BUG] git status doesn't handle submodules properly on OSX

[Pedro Melo]

Hi,

On Oct 15, 2008, at 4:21 PM, Lars Hoss wrote:

> Ok, this might be the difference. I am pretty sure my
> filesystem is not case-sensitive (default?).
> At least "mkdir Foo" fails when "foo" exists.

My filesystem is Mac OS Extended (Journaled) case-insensitive like  
yours, and submodules work.

Best regards,


>
>
> Yours,
> Lars
>
>> Hi,
>>
>> On Oct 15, 2008, at 16:51 , Lars Hoss wrote:
>>
>>>> Works for me on Leopard
>>>> [...]
>>>> Vienna:a pieter$ git --version
>>>> git version 1.6.0.2.415.gf9137
>>>
>>> My git version "1.6.0.2" says:
>>>
>>> # On branch master
>>> [...]
>>> git --version
>>> git version 1.6.0.2
>>>
>>> Git was build from macports.
>>
>>
>> works for me too on OS X 10.5.5 with git 1.6.0.2 from MacPorts. As it
>> deviates from the default, it might be worth mentioning that the
>> filesystem in use here is the case-sensitive version of HFS+.
>>
>> Best Regards,
>>   Richard
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

Hi!

I've cloned git this morning in order to try a build from this.
But chances are small that it will make a difference. Because from
the portfile I can see that macports just downloads 1.6.0.2 release
and builds it without any special patches or configure options.

As I said before, 1.5.6 works fine for me. But why 1.6.0.2 does not
work on my box - I don't know yet. Since Pedro uses a case-insensitive
volume as well, the problem seems not be related to sensitve/insensitive.

Now that I have a git clone on my box I might try bisect as well.

Yours,
Lars

> On Oct 15, 2008, at 3:51 PM, Lars Hoss wrote:
>
>>> Works for me on Leopard
>>>
>>> Vienna:a pieter$ git submodule add ~/projects/GitX/ gitx
>>> Initialized empty Git repository in /Users/pieter/a/gitx/.git/
>>> Vienna:a pieter$ git st
>>> # On branch master
>>> # Changes to be committed:
>>> #   (use "git reset HEAD <file>..." to unstage)
>>> #
>>> #	new file:   .gitmodules
>>> #	new file:   gitx
>>> #
>>> Vienna:a pieter$ git --version
>>> git version 1.6.0.2.415.gf9137
>>
>> My git version "1.6.0.2" says:
>>
>> # On branch master
>> # Changes to be committed:
>> #   (use "git reset HEAD <file>..." to unstage)
>> #
>> #	new file:   .gitmodules
>> #	new file:   lib
>> #
>> # Untracked files:
>> #   (use "git add <file>..." to include in what will be committed)
>> #
>> #	lib/
>> git --version
>> git version 1.6.0.2
>>
>> Git was build from macports.
>
> hmms.. Really, I know about fink and Macports and all others, but I'm
> using Mac OS X since version 10.1 (when compiling OSS software was
> near impossible due to Apple decisions) and I've always had better
> results with compiling my own than using any of those OSS repositories.
>
> I understand the appeal, it seems easier, but I would recommend that
> you compile yourself the software packages you depend on.
>
> Regarding git, I saw a git-build.sh script floating around. If you
> cannot find it, you might want to try my own recipe
> (http://tinyurl.com/4ayze6
> ). I compile git master every day (cron) and thats the one I use on my
> main machine.
>
> Best regards,
> --
> Pedro Melo
> Blog: http://www.simplicidade.org/notes/
> XMPP ID: melo@simplicidade.org
> Use XMPP!
>
>
>

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

Update:

Ok, I compiled several git versions from the git repo with the following
results:

v1.5.6.5 works
v1.6.0.2 fails
v1.6.0 fails
v1.6.0-rc0 fails

Given a git repo with a submodule 1.5.6.5 works (no untracked files) but
the same repo fails with 1.6.0-rc0 (lists submodule folders under
untracked files).

So something between 1.5.6.5 and 1.6.0-rc0 broke it for me.
A quick diff between the two version revealed a lot of changes. Thus
tracking down the issue might not be easy for someone who is not involved
in the code.

Maybe someone can give me a hint where to look? For example where does
"git status" decide whether a given filesystem folder should be ignored
with regard to .gitmodules?

Yours,
Lars

>
> On Oct 15, 2008, at 3:51 PM, Lars Hoss wrote:
>
>>> Works for me on Leopard
>>>
>>> Vienna:a pieter$ git submodule add ~/projects/GitX/ gitx
>>> Initialized empty Git repository in /Users/pieter/a/gitx/.git/
>>> Vienna:a pieter$ git st
>>> # On branch master
>>> # Changes to be committed:
>>> #   (use "git reset HEAD <file>..." to unstage)
>>> #
>>> #	new file:   .gitmodules
>>> #	new file:   gitx
>>> #
>>> Vienna:a pieter$ git --version
>>> git version 1.6.0.2.415.gf9137
>>
>> My git version "1.6.0.2" says:
>>
>> # On branch master
>> # Changes to be committed:
>> #   (use "git reset HEAD <file>..." to unstage)
>> #
>> #	new file:   .gitmodules
>> #	new file:   lib
>> #
>> # Untracked files:
>> #   (use "git add <file>..." to include in what will be committed)
>> #
>> #	lib/
>> git --version
>> git version 1.6.0.2
>>
>> Git was build from macports.
>
> hmms.. Really, I know about fink and Macports and all others, but I'm
> using Mac OS X since version 10.1 (when compiling OSS software was
> near impossible due to Apple decisions) and I've always had better
> results with compiling my own than using any of those OSS repositories.
>
> I understand the appeal, it seems easier, but I would recommend that
> you compile yourself the software packages you depend on.
>
> Regarding git, I saw a git-build.sh script floating around. If you
> cannot find it, you might want to try my own recipe
> (http://tinyurl.com/4ayze6
> ). I compile git master every day (cron) and thats the one I use on my
> main machine.
>
> Best regards,
> --
> Pedro Melo
> Blog: http://www.simplicidade.org/notes/
> XMPP ID: melo@simplicidade.org
> Use XMPP!
>
>
>

Re: [BUG] git status doesn't handle submodules properly on OSX

[Johannes Sixt]

Lars Hoss schrieb:
> Ok, I compiled several git versions from the git repo with the following
> results:
> 
> v1.5.6.5 works
> v1.6.0.2 fails
> v1.6.0 fails
> v1.6.0-rc0 fails
...
> So something between 1.5.6.5 and 1.6.0-rc0 broke it for me.
> A quick diff between the two version revealed a lot of changes. Thus
> tracking down the issue might not be easy for someone who is not involved
> in the code.

Start with:

$ git bisect start v1.6.0-rc0 v1.5.6.5 v1.6.0~6^2 v1.6.0.2~16^2  # [*]

and test the version that this command gives you. Then say either

$ git bisect good  # if the version works
$ git bisect bad   # if the version fails

Test again and repeat until the command tells you "First bad revision
is..." and report back. (Finally, say 'git bisect reset' to get you back
to where you started.)

[*] The revisions v1.6.0~6^2 v1.6.0.2~16^2 are from gitk and git-gui,
which we assume are good.

-- Hannes

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

>> So something between 1.5.6.5 and 1.6.0-rc0 broke it for me.
>> A quick diff between the two version revealed a lot of changes. Thus
>> tracking down the issue might not be easy for someone who is not
>> involved
>> in the code.
>
> Start with:
>
> $ git bisect start v1.6.0-rc0 v1.5.6.5 v1.6.0~6^2 v1.6.0.2~16^2  # [*]
>
> and test the version that this command gives you. Then say either
>
> $ git bisect good  # if the version works
> $ git bisect bad   # if the version fails
>
> Test again and repeat until the command tells you "First bad revision
> is..." and report back. (Finally, say 'git bisect reset' to get you back
> to where you started.)
>
> [*] The revisions v1.6.0~6^2 v1.6.0.2~16^2 are from gitk and git-gui,
> which we assume are good.
>
> -- Hannes

Wow, thank you Hannes! Will do so.

Re: [BUG] git status doesn't handle submodules properly on OSX

[Lars Hoss]

Thanks again for your feedback, guys!

Ok, I've done it and tracked it down. Unfortunately my MacBook
has no access to the company network or internet (must get an iPhone ;-),
so I cannot copy&paste my finding.

For now I can you give this:
1.5.6.rc1.24.gd629 fails
1.5.6.rc1.23.g6c2ce works

The commit was on Jun 5, 14:47:50 by Marius Storm-Olsen and the relevant
file is wt-status.c.

Ok, I think I found the issue. I enabled showUntrackedFiles in my gitconfig:

status.showUntrackedFiles = all

On the one hand it makes sense to list submodule folder as untracked
files. Then again it feels wrong because it is a registered submodule. Hm
:-)

Anyway, bisect is really impressive!

Yours,
Lars

Re: [BUG] git status doesn't handle submodules properly on OSX

[Jeff King]

On Thu, Oct 16, 2008 at 02:30:00PM +0200, Lars Hoss wrote:

> The commit was on Jun 5, 14:47:50 by Marius Storm-Olsen and the relevant
> file is wt-status.c.
> 
> Ok, I think I found the issue. I enabled showUntrackedFiles in my gitconfig:
> 
> status.showUntrackedFiles = all

Ah, OK. I see what is going on. All code paths call the read_directory
infrastructure to find untracked files. If status.showUntrackedFiles is
"normal", then we set dir.show_other_directories, to indicate that we
want to see the directories, but not their constituent files.

If status.showuntrackedfiles is set to "all", then we don't set the
show_other_directories flag, because we want each file. But the code in
dir.c:treat_directory uses the "show_other_directories" flag to say "oh,
we're just interested in untracked files" and decide whether to ignore
gitlinks.

Meaning that we must still filter the results of read_directory based on
the cache. And indeed, this is what "git ls-files -o" does, as explained
in 5698454e (Fix some "git ls-files -o" fallout from gitlinks). It's
also what the code in wt_status_print_untracked is _supposed_ to do, but
it was never updated to handle this case when git-ls-files was. Which is
probably my fault a long time ago for cutting and pasting the 5 lines of
"is this thing in the cache" when writing wt-status.c.

So the quick fix is to re-cut-and-paste the code:

diff --git a/wt-status.c b/wt-status.c
index d2eac36..792d5f1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -280,10 +280,14 @@ static void wt_status_print_untracked(struct wt_status *s)
 		/* check for matching entry, which is unmerged; lifted from
 		 * builtin-ls-files:show_other_files */
 		struct dir_entry *ent = dir.entries[i];
-		int pos = cache_name_pos(ent->name, ent->len);
+		int len, pos;
+		len = ent->len;
+		if (len && ent->name[len-1] == '/')
+			len--;
+		pos = cache_name_pos(ent->name, len);
 		struct cache_entry *ce;
 		if (0 <= pos)
-			die("bug in wt_status_print_untracked");
+			continue;
 		pos = -pos - 1;
 		if (pos < active_nr) {
 			ce = active_cache[pos];

But the right solution is to refactor this so the code isn't duplicated.
And I'll post a patch for that in a second.

I do have to wonder, though, whether an even better solution would be to
more explicitly tell read_directory "I'm interested only in 'other'
files" rather than relying on guessing based on
dir.show_other_directories. Then we could just avoid ever passing these
gitlinks back to ls-files and status in the first place.

-Peff

[PATCH] refactor handling of "other" files in ls-files and status

[Jeff King]

When the "git status" display code was originally converted
to C, we copied the code from ls-files to discover whether a
pathname returned by read_directory was an "other", or
untracked, file.

Much later, 5698454e updated the code in ls-files to handle
some new cases caused by gitlinks.  This left the code in
wt-status.c broken: it would display submodule directories
as untracked directories. Nobody noticed until now, however,
because unless status.showUntrackedFiles was set to "all",
submodule directories were not actually reported by
read_directory. So the bug was only triggered in the
presence of a submodule _and_ this config option.

This patch pulls the ls-files code into a new function,
cache_name_is_other, and uses it in both places. This should
leave the ls-files functionality the same and fix the bug
in status.

Signed-off-by: Jeff King <peff@peff.net>
---
On Thu, Oct 16, 2008 at 10:18:13AM -0400, Jeff King wrote:

> But the right solution is to refactor this so the code isn't duplicated.
> And I'll post a patch for that in a second.

Here it is.

> I do have to wonder, though, whether an even better solution would be to
> more explicitly tell read_directory "I'm interested only in 'other'
> files" rather than relying on guessing based on
> dir.show_other_directories. Then we could just avoid ever passing these
> gitlinks back to ls-files and status in the first place.

I was thinking such a change would get rid of the need for my patch,
since all files returned _would_ be "others". However, we would still
need to deal with the "unmerged entries" problem, so I think this
refactoring is worth it regardless.

 builtin-ls-files.c |   33 ++-------------------------------
 cache.h            |    2 ++
 read-cache.c       |   27 +++++++++++++++++++++++++++
 t/t7502-status.sh  |    6 ++++++
 wt-status.c        |   15 ++-------------
 5 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 068f424..b48327d 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -91,39 +91,10 @@ static void show_other_files(struct dir_struct *dir)
 {
 	int i;
 
-
-	/*
-	 * Skip matching and unmerged entries for the paths,
-	 * since we want just "others".
-	 *
-	 * (Matching entries are normally pruned during
-	 * the directory tree walk, but will show up for
-	 * gitlinks because we don't necessarily have
-	 * dir->show_other_directories set to suppress
-	 * them).
-	 */
 	for (i = 0; i < dir->nr; i++) {
 		struct dir_entry *ent = dir->entries[i];
-		int len, pos;
-		struct cache_entry *ce;
-
-		/*
-		 * Remove the '/' at the end that directory
-		 * walking adds for directory entries.
-		 */
-		len = ent->len;
-		if (len && ent->name[len-1] == '/')
-			len--;
-		pos = cache_name_pos(ent->name, len);
-		if (0 <= pos)
-			continue;	/* exact match */
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == len &&
-			    !memcmp(ce->name, ent->name, len))
-				continue; /* Yup, this one exists unmerged */
-		}
+		if (!cache_name_is_other(ent->name, ent->len))
+			continue;
 		show_dir_entry(tag_other, ent);
 	}
 }
diff --git a/cache.h b/cache.h
index 991544c..ae6647e 100644
--- a/cache.h
+++ b/cache.h
@@ -277,6 +277,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
+#define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
 #endif
 
 enum object_type {
@@ -393,6 +394,7 @@ extern int add_to_index(struct index_state *, const char *path, struct stat *, i
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
+extern int index_name_is_other(const struct index_state *, const char *, int);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index c229fd4..780f2c7 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1565,3 +1565,30 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
 	return !!data.add_errors;
 }
 
+/*
+ * Returns 1 if the path is an "other" path with respect to
+ * the index; that is, the path is not mentioned in the index at all,
+ * either as a file, a directory with some files in the index,
+ * or as an unmerged entry.
+ *
+ * We helpfully remove a trailing "/" from directories so that
+ * the output of read_directory can be used as-is.
+ */
+int index_name_is_other(const struct index_state *istate, const char *name,
+		int namelen)
+{
+	int pos;
+	if (namelen && name[namelen - 1] == '/')
+		namelen--;
+	pos = index_name_pos(istate, name, namelen);
+	if (0 <= pos)
+		return 0;	/* exact match */
+	pos = -pos - 1;
+	if (pos < istate->cache_nr) {
+		struct cache_entry *ce = istate->cache[pos];
+		if (ce_namelen(ce) == namelen &&
+		    !memcmp(ce->name, name, namelen))
+			return 0; /* Yup, this one exists unmerged */
+	}
+	return 1;
+}
diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index 1905fb3..93f875f 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -292,6 +292,12 @@ test_expect_success 'status submodule summary is disabled by default' '
 	test_cmp expect output
 '
 
+# we expect the same as the previous test
+test_expect_success 'status --untracked-files=all does not show submodule' '
+	git status --untracked-files=all >output &&
+	test_cmp expect output
+'
+
 head=$(cd sm && git rev-parse --short=7 --verify HEAD)
 
 cat >expect <<EOF
diff --git a/wt-status.c b/wt-status.c
index d2eac36..c3a9cab 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -277,20 +277,9 @@ static void wt_status_print_untracked(struct wt_status *s)
 
 	read_directory(&dir, ".", "", 0, NULL);
 	for(i = 0; i < dir.nr; i++) {
-		/* check for matching entry, which is unmerged; lifted from
-		 * builtin-ls-files:show_other_files */
 		struct dir_entry *ent = dir.entries[i];
-		int pos = cache_name_pos(ent->name, ent->len);
-		struct cache_entry *ce;
-		if (0 <= pos)
-			die("bug in wt_status_print_untracked");
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == ent->len &&
-			    !memcmp(ce->name, ent->name, ent->len))
-				continue;
-		}
+		if (!cache_name_is_other(ent->name, ent->len))
+			continue;
 		if (!shown_header) {
 			s->workdir_untracked = 1;
 			wt_status_print_untracked_header(s);
-- 
1.6.0.2.710.gc3f34.dirty

Re: [PATCH] refactor handling of "other" files in ls-files and status

[Jeff King]

On Thu, Oct 16, 2008 at 10:59:16AM -0400, Jeff King wrote:

> This patch pulls the ls-files code into a new function,
> cache_name_is_other, and uses it in both places. This should
> leave the ls-files functionality the same and fix the bug
> in status.

BTW, this is probably maint-worthy, but I prepared it on 'master'.
Unfortunately, there is a purely textual conflict when applying to
maint. For your convenience, here's a rebase to maint:

-- >8 --
refactor handling of "other" files in ls-files and status

When the "git status" display code was originally converted
to C, we copied the code from ls-files to discover whether a
pathname returned by read_directory was an "other", or
untracked, file.

Much later, 5698454e updated the code in ls-files to handle
some new cases caused by gitlinks.  This left the code in
wt-status.c broken: it would display submodule directories
as untracked directories. Nobody noticed until now, however,
because unless status.showUntrackedFiles was set to "all",
submodule directories were not actually reported by
read_directory. So the bug was only triggered in the
presence of a submodule _and_ this config option.

This patch pulls the ls-files code into a new function,
cache_name_is_other, and uses it in both places. This should
leave the ls-files functionality the same and fix the bug
in status.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-ls-files.c |   33 ++-------------------------------
 cache.h            |    2 ++
 read-cache.c       |   28 ++++++++++++++++++++++++++++
 t/t7502-status.sh  |    6 ++++++
 wt-status.c        |   15 ++-------------
 5 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 068f424..b48327d 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -91,39 +91,10 @@ static void show_other_files(struct dir_struct *dir)
 {
 	int i;
 
-
-	/*
-	 * Skip matching and unmerged entries for the paths,
-	 * since we want just "others".
-	 *
-	 * (Matching entries are normally pruned during
-	 * the directory tree walk, but will show up for
-	 * gitlinks because we don't necessarily have
-	 * dir->show_other_directories set to suppress
-	 * them).
-	 */
 	for (i = 0; i < dir->nr; i++) {
 		struct dir_entry *ent = dir->entries[i];
-		int len, pos;
-		struct cache_entry *ce;
-
-		/*
-		 * Remove the '/' at the end that directory
-		 * walking adds for directory entries.
-		 */
-		len = ent->len;
-		if (len && ent->name[len-1] == '/')
-			len--;
-		pos = cache_name_pos(ent->name, len);
-		if (0 <= pos)
-			continue;	/* exact match */
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == len &&
-			    !memcmp(ce->name, ent->name, len))
-				continue; /* Yup, this one exists unmerged */
-		}
+		if (!cache_name_is_other(ent->name, ent->len))
+			continue;
 		show_dir_entry(tag_other, ent);
 	}
 }
diff --git a/cache.h b/cache.h
index 884fae8..76730ce 100644
--- a/cache.h
+++ b/cache.h
@@ -270,6 +270,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
+#define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
 #endif
 
 enum object_type {
@@ -382,6 +383,7 @@ extern int add_to_index(struct index_state *, const char *path, struct stat *, i
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
+extern int index_name_is_other(const struct index_state *, const char *, int);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index 8f96fd1..4e067e4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1480,3 +1480,31 @@ int read_index_unmerged(struct index_state *istate)
 	istate->cache_nr = dst - istate->cache;
 	return !!last;
 }
+
+/*
+ * Returns 1 if the path is an "other" path with respect to
+ * the index; that is, the path is not mentioned in the index at all,
+ * either as a file, a directory with some files in the index,
+ * or as an unmerged entry.
+ *
+ * We helpfully remove a trailing "/" from directories so that
+ * the output of read_directory can be used as-is.
+ */
+int index_name_is_other(const struct index_state *istate, const char *name,
+		int namelen)
+{
+	int pos;
+	if (namelen && name[namelen - 1] == '/')
+		namelen--;
+	pos = index_name_pos(istate, name, namelen);
+	if (0 <= pos)
+		return 0;	/* exact match */
+	pos = -pos - 1;
+	if (pos < istate->cache_nr) {
+		struct cache_entry *ce = istate->cache[pos];
+		if (ce_namelen(ce) == namelen &&
+		    !memcmp(ce->name, name, namelen))
+			return 0; /* Yup, this one exists unmerged */
+	}
+	return 1;
+}
diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index c8e4c2e..187a13e 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -285,6 +285,12 @@ test_expect_success 'status submodule summary is disabled by default' '
 	test_cmp expect output
 '
 
+# we expect the same as the previous test
+test_expect_success 'status --untracked-files=all does not show submodule' '
+	git status --untracked-files=all >output &&
+	test_cmp expect output
+'
+
 head=$(cd sm && git rev-parse --short=7 --verify HEAD)
 
 cat >expect <<EOF
diff --git a/wt-status.c b/wt-status.c
index 889e50f..64cedfc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -275,20 +275,9 @@ static void wt_status_print_untracked(struct wt_status *s)
 
 	read_directory(&dir, ".", "", 0, NULL);
 	for(i = 0; i < dir.nr; i++) {
-		/* check for matching entry, which is unmerged; lifted from
-		 * builtin-ls-files:show_other_files */
 		struct dir_entry *ent = dir.entries[i];
-		int pos = cache_name_pos(ent->name, ent->len);
-		struct cache_entry *ce;
-		if (0 <= pos)
-			die("bug in wt_status_print_untracked");
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == ent->len &&
-			    !memcmp(ce->name, ent->name, ent->len))
-				continue;
-		}
+		if (!cache_name_is_other(ent->name, ent->len))
+			continue;
 		if (!shown_header) {
 			s->workdir_untracked = 1;
 			wt_status_print_header(s, "Untracked files",
-- 
1.6.0.2.710.gc3f34.dirty

Re: [PATCH] refactor handling of "other" files in ls-files and status

[Junio C Hamano]

Thanks.