Re: [PATCH v2 2/7] branch: add branch_checked_out() helper

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2022, 04:52 UTC
Message-ID: <xmqqleu7sqnt.fsf@gitster.g>
In-Reply-To: <74d5f74b-c95f-6892-6a66-3e646b1f9e53@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 48 quoted lines

>> Yes, that's a good plan. I'll take a look.
>
> Here is a fixup for this patch that should work. I'll put it in v3.
>
> diff --git a/branch.c b/branch.c
> index 2e6419cdfa5..514212f5619 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -10,6 +10,7 @@
>  #include "worktree.h"
>  #include "submodule-config.h"
>  #include "run-command.h"
> +#include "strmap.h"
>  
>  struct tracking {
>  	struct refspec_item spec;
> @@ -369,17 +370,44 @@ int validate_branchname(const char *name, struct strbuf *ref)
>  	return ref_exists(ref->buf);
>  }
>  
> -int branch_checked_out(const char *refname, char **path)
> +static int initialized_checked_out_branches;
> +static struct strmap current_checked_out_branches = STRMAP_INIT;
> +
> +static void prepare_checked_out_branches(void)
>  {
> +	int i = 0;
> +	struct worktree **worktrees;
> +
> +	if (initialized_checked_out_branches)
> +		return;
> +	initialized_checked_out_branches = 1;
> +
> +	worktrees = get_worktrees();
> +
> +	while (worktrees[i]) {
> +		struct worktree *wt = worktrees[i];
>  
> +		if (!wt->is_bare && wt->head_ref)
> +			strmap_put(&current_checked_out_branches,
> +				   wt->head_ref,
> +				   wt->path);
> +
> +		i++;
> +	}
>  	free_worktrees(worktrees);
> -	return result;
> +}

Yeah, the above illustrates what I had in mind pretty closely. The above outline only checks where the HEAD symref points at, and it does not protect a branch that is in the middle of getting bisected or rebased, which find_shared_symref() tries to do, IIRC, though.

It should not be too hard to add that to the above to allow us to achieve feature parity with the original.

Show 12 quoted lines

> +
> +int branch_checked_out(const char *refname, char **path)
> +{
> +	const char *path_in_set;
> +	prepare_checked_out_branches();
> +
> +	path_in_set = strmap_get(&current_checked_out_branches, refname);
> +	if (path_in_set && path)
> +		*path = xstrdup(path_in_set);
> +
> +	return !!path_in_set;
>  }

This one looks quite straight-forward.

Show 24 quoted lines

> And there is another use of find_shared_symref() in the same file, allowing
> us to do the following:
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index ac29c2b1ae3..3933c482839 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -885,7 +885,7 @@ static int update_local_ref(struct ref *ref,
>  			    struct worktree **worktrees)
>  {
>  	struct commit *current = NULL, *updated;
> -	const struct worktree *wt;
> +	char *path = NULL;
>  	const char *pretty_ref = prettify_refname(ref->name);
>  	int fast_forward = 0;
>  
> @@ -900,17 +900,17 @@ static int update_local_ref(struct ref *ref,
>  	}
>  
>  	if (!update_head_ok &&
> -	    (wt = find_shared_symref(worktrees, "HEAD", ref->name)) &&
> -	    !wt->is_bare && !is_null_oid(&ref->old_oid)) {
> +	    !is_null_oid(&ref->old_oid) &&
> +	    branch_checked_out(ref->name, &path)) {

Yes.  It looks like a good direction to go in.

Thanks.