Re: [PATCH v3 2/8] branch: consider refs under 'update-refs'

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

 



On 28/06/2022 14:25, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <derrickstolee@xxxxxxxxxx>

The branch_checked_out() helper helps commands like 'git branch' and
'git fetch' from overwriting refs that are currently checked out in
other worktrees.

A future update to 'git rebase' will introduce a new '--update-refs'
option which will update the local refs that point to commits that are
being rebased. To avoid collisions as the rebase completes, we want to
make the future data store for these refs to be considered by
branch_checked_out().

The data store is a plaintext file inside the 'rebase-merge' directory
for that worktree. The file alternates refnames and OIDs. The OIDs will
be used to store the to-be-written values as the rebase progresses, but
can be ignored at the moment.

Create a new sequencer_get_update_refs_state() method that parses this
file and populates a struct string_list with the ref-OID pairs. We can
then use this list to add to the current_checked_out_branches strmap
used by branch_checked_out().

To properly navigate to the rebase directory for a given worktree,
extract the static strbuf_worktree_gitdir() method to a public API
method.

We can test that this works without having Git write this file by
artificially creating one in our test script.

This looks good apart from a couple of small nits

Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
---
  branch.c                  | 13 ++++++++++++
  sequencer.c               | 42 +++++++++++++++++++++++++++++++++++++++
  sequencer.h               |  9 +++++++++
  t/t2407-worktree-heads.sh | 22 ++++++++++++++++----
  4 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/branch.c b/branch.c
index 526e8237673..f252c4eef6c 100644
--- a/branch.c
+++ b/branch.c
@@ -365,6 +365,7 @@ static void prepare_checked_out_branches(void)
  		char *old;
  		struct wt_status_state state = { 0 };
  		struct worktree *wt = worktrees[i++];
+		struct string_list update_refs = STRING_LIST_INIT_DUP;
if (wt->is_bare)
  			continue;
@@ -400,6 +401,18 @@ static void prepare_checked_out_branches(void)
  			strbuf_release(&ref);
  		}
  		wt_status_state_free_buffers(&state);
+
+		if (!sequencer_get_update_refs_state(get_worktree_git_dir(wt),
+						     &update_refs)) {
+			struct string_list_item *item;
+			for_each_string_list_item(item, &update_refs) {
+				old = strmap_put(&current_checked_out_branches,
+						 item->string,
+						 xstrdup(wt->path));
+				free(old);
+			}
+			string_list_clear(&update_refs, 1);
+		}
  	}
free_worktrees(worktrees);
diff --git a/sequencer.c b/sequencer.c
index 8c3ed3532ac..1094e146b99 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5901,3 +5901,45 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
return 0;
  }
+
+int sequencer_get_update_refs_state(const char *wt_dir,
+				    struct string_list *refs)
+{
+	int result = 0;
+	struct stat st;
+	FILE *fp = NULL;
+	struct strbuf ref = STRBUF_INIT;
+	struct strbuf hash = STRBUF_INIT;
+	char *path = xstrfmt("%s/rebase-merge/update-refs", wt_dir);

I think it would make sense to introduce rebase_path_update_refs() in this patch rather than later in the series

+
+	if (stat(path, &st))
+		goto cleanup;

What's the reason for stating the file first, rather than just letting fopen() fail if it does not exist?

Best Wishes

Phillip



[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