Re: [PATCH] describe: refresh the index when 'broken' flag is used

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Abhijeet and Karthik

On 24/06/2024 11:56, Karthik Nayak wrote:
Abhijeet Sonar <abhijeet.nkt@xxxxxxxxx> writes:

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.  Refreshing the
index before running diff-index fixes the problem.

This is a good description of the problem the patch fixes.

Signed-off-by: Abhijeet Sonar <abhijeet.nkt@xxxxxxxxx>
Reported-by: Paul Millar <paul.millar@xxxxxxx>
Suggested-by: Junio C Hamano <gitster@xxxxxxxxx>
---
  builtin/describe.c | 14 ++++++++++++++
  1 file changed, 14 insertions(+)

diff --git a/builtin/describe.c b/builtin/describe.c
index e5287eddf2..2b443c155e 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -645,6 +645,20 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
  	if (argc == 0) {
  		if (broken) {
  			struct child_process cp = CHILD_PROCESS_INIT;
+			struct lock_file index_lock = LOCK_INIT;
+			int fd;
+
+			setup_work_tree();
+			prepare_repo_settings(the_repository);
+			repo_read_index(the_repository);
+			refresh_index(the_repository->index, REFRESH_QUIET|REFRESH_UNMERGED,
+				      NULL, NULL, NULL);
+			fd = repo_hold_locked_index(the_repository,
+						    &index_lock, 0);
+			if (0 <= fd)
+				repo_update_index_if_able(the_repository, &index_lock);
+

As we're dealing with a repository that might be broken I suspect we'd be better to run "git update-index --unmerged -q --refresh" as a subprocess in the same way that we run "git diff-index" so that "git describe --broken" does not die if the index cannot be refreshed.

I'm wondering why this needs to be done, as I can see, when we use the
'--broken' flag, we create a child process to run `git diff-index
--quiet HEAD`. As such, we shouldn't have to refresh the index here.

"git diff-index" and "git diff-files" do not refresh the index. This is by design so that a script can refresh the index once and run "git diff-index" several times without wasting time updating the index each time.

Also apart from that, we should add a test to capture the changes.

That would be nice

Best Wishes

Phillip


  			cp.git_cmd = 1;
  			cp.no_stdin = 1;
--
2.45.GIT




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux