Skip to content

Commit

Permalink
git status: Fix false positive "new commits" output for dirty submodules
Browse files Browse the repository at this point in the history 
Testing if the output "new commits" should appear in the long format of
"git status" is done by comparing the hashes of the diffpair. This always
resulted in printing "new commits" for submodules that contained untracked
or modified content, even if they did not contain new commits. The reason
was that match_stat_with_submodule() did set the "changed" flag for dirty
submodules, resulting in two->sha1 being set to the null_sha1 at the call
sites, which indicates that new commits are present. This is changed so
that when no new commits are present, the same object names are in the
sha1 field for both sides of the filepair, and the working tree side will
have the "dirty_submodule" flag set when appropriate. For a submodule to
be seen as modified even when it just has a dirty work tree, some
conditions had to be extended to also check for the "dirty_submodule"
flag.

Unfortunately the test case that should have found this bug had been
changed incorrectly too. It is fixed and extended to test for other
combinations too.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
@jlehmann @gitster
jlehmann authored and gitster committed Mar 13, 2010
1 parent ae6d5c1 commit 85adbf2
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 9 deletions.
6 changes: 2 additions & 4 deletions diff-lib.c
View file
Expand Up @@ -72,8 +72,6 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
&& !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
&& (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
*dirty_submodule = is_submodule_modified(ce->name);
if (*dirty_submodule)
changed = 1;
}
return changed;
}
Expand Down Expand Up @@ -202,7 +200,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
}
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
ce_option, &dirty_submodule);
if (!changed) {
if (!changed && !dirty_submodule) {
ce_mark_uptodate(ce);
if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
continue;
Expand Down Expand Up @@ -333,7 +331,7 @@ static int show_modified(struct rev_info *revs,
}

oldmode = old->ce_mode;
if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
if (mode == oldmode && !hashcmp(sha1, old->sha1) && !dirty_submodule &&
!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
return 0;

Expand Down
7 changes: 5 additions & 2 deletions diff.c
View file
Expand Up @@ -2032,7 +2032,7 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
char *data = xmalloc(100), *dirty = "";

/* Are we looking at the work tree? */
if (!s->sha1_valid && s->dirty_submodule)
if (s->dirty_submodule)
dirty = "-dirty";

len = snprintf(data, 100,
Expand Down Expand Up @@ -3081,7 +3081,8 @@ int diff_unmodified_pair(struct diff_filepair *p)
* dealing with a change.
*/
if (one->sha1_valid && two->sha1_valid &&
!hashcmp(one->sha1, two->sha1))
!hashcmp(one->sha1, two->sha1) &&
!one->dirty_submodule && !two->dirty_submodule)
return 1; /* no change */
if (!one->sha1_valid && !two->sha1_valid)
return 1; /* both look at the same file on the filesystem. */
Expand Down Expand Up @@ -3216,6 +3217,8 @@ static void diff_resolve_rename_copy(void)
}
else if (hashcmp(p->one->sha1, p->two->sha1) ||
p->one->mode != p->two->mode ||
p->one->dirty_submodule ||
p->two->dirty_submodule ||
is_null_sha1(p->one->sha1))
p->status = DIFF_STATUS_MODIFIED;
else {
Expand Down
84 changes: 81 additions & 3 deletions t/t7506-status-submodule.sh
View file
Expand Up @@ -34,7 +34,7 @@ test_expect_success 'status with modified file in submodule' '
(cd sub && git reset --hard) &&
echo "changed" >sub/foo &&
git status >output &&
grep "modified: sub (new commits, modified content)" output
grep "modified: sub (modified content)" output
'

test_expect_success 'status with modified file in submodule (porcelain)' '
Expand All @@ -49,7 +49,7 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
test_expect_success 'status with added file in submodule' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
git status >output &&
grep "modified: sub (new commits, modified content)" output
grep "modified: sub (modified content)" output
'

test_expect_success 'status with added file in submodule (porcelain)' '
Expand All @@ -64,7 +64,7 @@ test_expect_success 'status with untracked file in submodule' '
(cd sub && git reset --hard) &&
echo "content" >sub/new-file &&
git status >output &&
grep "modified: sub (new commits, untracked content)" output
grep "modified: sub (untracked content)" output
'

test_expect_success 'status with untracked file in submodule (porcelain)' '
Expand All @@ -74,6 +74,84 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
EOF
'

test_expect_success 'status with added and untracked file in submodule' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
echo "content" >sub/new-file &&
git status >output &&
grep "modified: sub (modified content, untracked content)" output
'

test_expect_success 'status with added and untracked file in submodule (porcelain)' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
echo "content" >sub/new-file &&
git status --porcelain >output &&
diff output - <<-\EOF
M sub
EOF
'

test_expect_success 'status with modified file in modified submodule' '
(cd sub && git reset --hard) &&
rm sub/new-file &&
(cd sub && echo "next change" >foo && git commit -m "next change" foo) &&
echo "changed" >sub/foo &&
git status >output &&
grep "modified: sub (new commits, modified content)" output
'

test_expect_success 'status with modified file in modified submodule (porcelain)' '
(cd sub && git reset --hard) &&
echo "changed" >sub/foo &&
git status --porcelain >output &&
diff output - <<-\EOF
M sub
EOF
'

test_expect_success 'status with added file in modified submodule' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
git status >output &&
grep "modified: sub (new commits, modified content)" output
'

test_expect_success 'status with added file in modified submodule (porcelain)' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
git status --porcelain >output &&
diff output - <<-\EOF
M sub
EOF
'

test_expect_success 'status with untracked file in modified submodule' '
(cd sub && git reset --hard) &&
echo "content" >sub/new-file &&
git status >output &&
grep "modified: sub (new commits, untracked content)" output
'

test_expect_success 'status with untracked file in modified submodule (porcelain)' '
git status --porcelain >output &&
diff output - <<-\EOF
M sub
EOF
'

test_expect_success 'status with added and untracked file in modified submodule' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
echo "content" >sub/new-file &&
git status >output &&
grep "modified: sub (new commits, modified content, untracked content)" output
'

test_expect_success 'status with added and untracked file in modified submodule (porcelain)' '
(cd sub && git reset --hard && echo >foo && git add foo) &&
echo "content" >sub/new-file &&
git status --porcelain >output &&
diff output - <<-\EOF
M sub
EOF
'

test_expect_success 'rm submodule contents' '
rm -rf sub/* sub/.git
'
Expand Down

0 comments on commit 85adbf2

Please sign in to comment.