Re: [PATCH v5] describe: refresh the index when 'broken' flag is used
Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:
Show 82 quoted lines
> When describe is run with 'dirty' flag, we refresh the index
> to make sure it is in sync with the filesystem before
> determining if the working tree is dirty. However, this is
> not done for the codepath where the 'broken' flag is used.
>
> This causes `git describe --broken --dirty` to false
> positively report the worktree being dirty if a file has
> different stat info than what is recorded in the index.
> Running `git update-index -q --refresh` to refresh the index
> before running diff-index fixes the problem.
>
> Also add tests to deliberately update stat info of a
> file before running describe to verify it behaves correctly.
>
> Reported-by: Paul Millar <paul.millar@desy.de>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>
> ---
> builtin/describe.c | 11 +++++++++++
> t/t6120-describe.sh | 24 ++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index e5287eddf2..7cb9d50b36 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -53,6 +53,10 @@ static const char *diff_index_args[] = {
> "diff-index", "--quiet", "HEAD", "--", NULL
> };
>
> +static const char *update_index_args[] = {
> + "update-index", "--unmerged", "-q", "--refresh", NULL
> +};
> +
> struct commit_name {
> struct hashmap_entry entry;
> struct object_id peeled;
> @@ -645,6 +649,13 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> if (argc == 0) {
> if (broken) {
> struct child_process cp = CHILD_PROCESS_INIT;
> + strvec_pushv(&cp.args, update_index_args);
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.no_stdout = 1;
> + if (run_command(&cp))
> + child_process_clear(&cp);
> +
> strvec_pushv(&cp.args, diff_index_args);
> cp.git_cmd = 1;
> cp.no_stdin = 1;
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index e78315d23d..6c396e7abc 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -671,4 +671,28 @@ test_expect_success 'setup misleading taggerdates' '
>
> check_describe newer-tag-older-commit~1 --contains unique-file~2
>
> +test_expect_success 'describe --dirty with a file with changed stat' '
> + git init stat-dirty &&
> + (
> + cd stat-dirty &&
> +
> + echo A >file &&
> + git add file &&
> + git commit -m A &&
> + git tag A -a -m A &&
> +
> + cat file >file.new &&
> + mv file.new file &&
> + git describe --dirty >actual &&
> + echo "A" >expected &&
> + test_cmp expected actual &&
> +
> + cat file >file.new &&
> + mv file.new file &&
> + git describe --dirty --broken >actual &&
> + echo "A" >expected &&
> + test_cmp expected actualNot worth a reroll, but you don't have to create file.new twice.
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 6c396e7abc..6c4b20fec7 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -683,14 +683,12 @@ test_expect_success 'describe --dirty with a
file with changed stat' '
cat file >file.new &&
mv file.new file &&
- git describe --dirty >actual &&
echo "A" >expected &&
+
+ git describe --dirty >actual &&
test_cmp expected actual &&
- cat file >file.new &&
- mv file.new file &&
git describe --dirty --broken >actual &&
- echo "A" >expected &&
test_cmp expected actual
)
'
> + )
> +'
> +
> test_done
>
> Range-diff against v4:
> 1: 1da5fa48d9 ! 1: 52f590b70f describe: refresh the index when 'broken' flag is used
> @@ builtin/describe.c: int cmd_describe(int argc, const char **argv, const char *pr
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.no_stdout = 1;
> -+ run_command(&cp);
> -+ strvec_clear(&cp.args);
> ++ if (run_command(&cp))
> ++ child_process_clear(&cp);
> +
> strvec_pushv(&cp.args, diff_index_args);
> cp.git_cmd = 1;
> --
> 2.45.2.606.g9005149a4a.dirty
Other than this, this looks good to me.
Thanks!
Karthik Nayak <karthik.188@gmail.com>