Sebastian Staudt <koraktor@xxxxxxxxx> writes: > We don't use NEED_WORK_TREE when running the git-describe builtin, > since you should be able to describe a commit even in a bare repository. > However, the --dirty flag does need a working tree. Since we don't call > setup_work_tree(), it uses whatever directory we happen to be in. That's > unlikely to match our index, meaning we'd say "dirty" even when the real > working tree is clean. > > We can fix that by calling setup_work_tree() once we know that the user > has asked for --dirty. > > The --broken option also needs a working tree. But because its > implementation calls git-diff-index we don‘t have to setup the working > tree in the git-describe process. Very nicely described. > > Signed-off-by: Sebastian Staudt <koraktor@xxxxxxxxx> > Helped-by: Jeff King <peff@xxxxxxxx> > --- > builtin/describe.c | 1 + > t/t6120-describe.sh | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/builtin/describe.c b/builtin/describe.c > index cc118448ee..b5b7abdc8f 100644 > --- a/builtin/describe.c > +++ b/builtin/describe.c > @@ -629,6 +629,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix) > struct argv_array args = ARGV_ARRAY_INIT; > int fd, result; > > + setup_work_tree(); > read_cache(); > refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, > NULL, NULL, NULL); And the implementation is as promised in the proposed log message. Quite straight-forward and obviously right ;-) > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh > index d639d94696..7cfed77c52 100755 > --- a/t/t6120-describe.sh > +++ b/t/t6120-describe.sh > @@ -145,14 +145,38 @@ check_describe A-* HEAD > > check_describe "A-*[0-9a-f]" --dirty > > +test_expect_success 'describe --dirty with --work-tree' ' > + ( > + cd "$TEST_DIRECTORY" && > + git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty >"$TRASH_DIRECTORY/out" > + ) && > + grep "^A-[1-9][0-9]\?-g[0-9a-f]\+$" out > +' > + Without the fix to the code, this test piece fails with "-dirty" suffix in 'out'. Good. > +test_expect_success 'describe --dirty with --work-tree' ' > + ( > + cd "$TEST_DIRECTORY" && > + git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty >"$TRASH_DIRECTORY/out" > + ) && > + grep "^A-[1-9][0-9]\?-g[0-9a-f]\+-dirty$" out > +' This succeeds with or without the fix to the code; the buggy behaviour was to claim "-dirty"-ness when the working tree files are clean. This new test is not about that buggy behaviour. It is rather about the updated code does not mistakenly claim cleanness in a dirty working tree. Good. > check_describe "A-*[0-9a-f].mod" --dirty=.mod > > +test_expect_success 'describe --dirty=.mod with --work-tree' ' > + ( > + cd "$TEST_DIRECTORY" && > + git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty=.mod >"$TRASH_DIRECTORY/out" > + ) && > + grep "^A-[1-9][0-9]\?-g[0-9a-f]\+.mod$" out > +' > + Likewise. > test_expect_success 'describe --dirty HEAD' ' > test_must_fail git describe --dirty HEAD > ' > @@ -303,8 +327,17 @@ test_expect_success 'describe chokes on severely broken submodules' ' > mv .git/modules/sub1/ .git/modules/sub_moved && > test_must_fail git describe --dirty > ' > + > test_expect_success 'describe ignoring a broken submodule' ' > git describe --broken >out && > + grep broken out > +' > + > +test_expect_success 'describe with --work-tree ignoring a broken submodule' ' > + ( > + cd "$TEST_DIRECTORY" && > + git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --broken >"$TRASH_DIRECTORY/out" > + ) && OK, this checks the same repository as the existing test does, but does so from outside the repository. Looks good. > test_when_finished "mv .git/modules/sub_moved .git/modules/sub1" && > grep broken out > '
