Re: [PATCH] Show submodules as modified when they contain a dirty work tree
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 13, 2010, 22:10 UTC
- Message-ID
- <7v6375lkpj.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4B4E1817.1070202@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 9 quoted lines
> Thanks for your review, here is the updated patch. Changes to the RFC > version are: > > - Removed check for a dangling HEAD (now the testsuite runs fine) > - Reworded the commit message > - Inlined is_submodule_working_directory_dirty() into > is_submodule_modified() > - The new code will only be called when refs did match (when they > didn't the submodule will already show up as modified)
Looking good.
I had to squash in '#include "submodule.h"' in diff-lib.c just after it includes "refs.h", though.
And a patch to add:
Show 11 quoted lines
>> * It doesn't give detailed output when doing a "git diff* -p" with or >> without the --submodule option. It should show something like >> >> diff --git a/sub b/sub >> index 5431f52..3f35670 160000 >> --- a/sub >> +++ b/sub >> @@ -1 +1 @@ >> -Subproject commit 5431f529197f3831cdfbba1354a819a79f948f6f >> +Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty >>
would look like the attached.
I think a reasonable next step would be
- Move the check for your condition (c) that we dropped from this round to wt-status.c;
- Add wt_status_print_dangling_submodules() to wt-status.c, and use the above logic to produce a section "Submodules with Dangling HEAD" or something.
- Call it in wt_status_print(), immediately before we check s->verbose and show the patch text under -v option. "git status" now will warn about the condition (c).
- Add a similar wt_shortstatus_print_dangling_submodules() and call it at the end of wt_shortstatus_print().
- Update is_submodule_modified() in your patch thats reads the output from "status --porcelain", to *ignore* information about dangling submodules. As we discussed, dangling submodules may be something the user cares about, but that is not something "diff" should.
-- >8 -- Subject: Teach diff that modified submodule directory is dirty
A diff run in superproject only compares the name of the commit object bound at the submodule paths. When we compare with a work tree and the checked out submodule directory is dirty (e.g. has either staged or unstaged changes, or has new files the user forgot to add to the index), show the work tree side as "dirty".
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- diff.c | 9 ++++++- t/t4027-diff-submodule.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c index 04beb26..750c066 100644 --- a/diff.c +++ b/diff.c @@ -2029,9 +2029,14 @@ static int populate_from_stdin(struct diff_filespec *s) static int diff_populate_gitlink(struct diff_filespec *s, int size_only) { int len; - char *data = xmalloc(100); + char *data = xmalloc(100), *dirty = ""; + + /* Are we looking at the work tree? */ + if (!s->sha1_valid && is_submodule_modified(s->path)) + dirty = "-dirty"; + len = snprintf(data, 100, - "Subproject commit %s\n", sha1_to_hex(s->sha1)); + "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty); s->data = data; s->size = len; s->should_free = 1; diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh index 5cf8924..bf8c980 100755 --- a/t/t4027-diff-submodule.sh +++ b/t/t4027-diff-submodule.sh @@ -32,7 +32,8 @@ test_expect_success setup ' cd sub && git rev-list HEAD ) && - echo ":160000 160000 $3 $_z40 M sub" >expect + echo ":160000 160000 $3 $_z40 M sub" >expect && + subtip=$3 subprev=$2 ' test_expect_success 'git diff --raw HEAD' ' @@ -50,6 +51,52 @@ test_expect_success 'git diff-files --raw' ' test_cmp expect actual.files ' +expect_from_to () { + printf "%sSubproject commit %s\n+Subproject commit %s\n" \ + "-" "$1" "$2" +} + +test_expect_success 'git diff HEAD' ' + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (work tree)' ' + echo >>sub/world && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (index)' ' + ( + cd sub && + git reset --hard && + echo >>world && + git add world + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (untracked)' ' + ( + cd sub && + git reset --hard && + git clean -qfdx && + >cruft + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + test_expect_success 'git diff (empty submodule dir)' ' : >empty && rm -rf sub/* sub/.git &&