threads / bug / 15912
Subject: [BUG] git status doesn't handle submodules properly on OSX
Lars HossGreetings 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
Jeff KingRe: [BUG] git status doesn't handle submodules properly on OSX
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
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
> 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
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
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
> 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
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?
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
> 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
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
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
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 >
Pedro MeloRe: [BUG] git status doesn't handle submodules properly on OSX
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!
Pedro MeloRe: [BUG] git status doesn't handle submodules properly on OSX
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!
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
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! > > >
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
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! > > >
Johannes SixtRe: [BUG] git status doesn't handle submodules properly on OSX
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
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
>> 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.
Lars HossRe: [BUG] git status doesn't handle submodules properly on OSX
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
Jeff KingRe: [BUG] git status doesn't handle submodules properly on OSX
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
Jeff King[PATCH] 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> --- 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
Jeff KingRe: [PATCH] refactor handling of "other" files in ls-files and status
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
Pedro MeloRe: [BUG] git status doesn't handle submodules properly on OSX
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!
