Junio C Hamano <gitster@xxxxxxxxx> writes:
> "Olga Pilipenco via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> +/*
>> +* When in a secondary worktree, and when extensions.worktreeConfig
>> +* is true, only $commondir/config and $commondir/worktrees/<id>/
>> +* config.worktree are consulted, hence any core.bare=true setting in
>> +* $commondir/config.worktree gets overlooked. Thus, check it manually
>> +* to determine if the repository is bare.
>> +*/
>> +static int is_main_worktree_bare(struct repository *repo)
>> +{
>> + int bare = 0;
>> + struct config_set cs = {0};
>> + char *worktree_config = xstrfmt("%s/config.worktree", repo_get_common_dir(repo));
>> +
>> + git_configset_init(&cs);
>> + git_configset_add_file(&cs, worktree_config);
>> + git_configset_get_bool(&cs, "core.bare", &bare);
>> +
>> + git_configset_clear(&cs);
>> + free(worktree_config);
>> + return bare;
>> +}
>
> That is nicely described.
>
>> /**
>> * get the main worktree
>> */
>> @@ -79,16 +101,11 @@ static struct worktree *get_main_worktree(int skip_reading_head)
>> CALLOC_ARRAY(worktree, 1);
>> worktree->repo = the_repository;
>> worktree->path = strbuf_detach(&worktree_path, NULL);
>> - /*
>> - * NEEDSWORK: If this function is called from a secondary worktree and
>> - * config.worktree is present, is_bare_repository_cfg will reflect the
>> - * contents of config.worktree, not the contents of the main worktree.
>> - * This means that worktree->is_bare may be set to 0 even if the main
>> - * worktree is configured to be bare.
>> - */
>> - worktree->is_bare = (is_bare_repository_cfg == 1) ||
>> - is_bare_repository();
>> worktree->is_current = is_current_worktree(worktree);
>> + worktree->is_bare = (is_bare_repository_cfg == 1) ||
>> + is_bare_repository() ||
>> + (!worktree->is_current && is_main_worktree_bare(the_repository));
>
> Is "this worktree does not have is_current bit set" equivalent to
> "this worktree is the main one, so is_main_worktree_bare() needs to
> be consulted"? That linkage between "the is_current bit unset" and
> "is the main worktree" is not obvious to me.
Does the thinking behind it go like this?
We grabbed the "main" worktree object and stored it in worktree;
it is either our current worktree (in which case is_current is
true), or it is not (in which case, is_current is false). We
know that the old logic failed when asking the "is it bare"
question from a secondary worktree. !worktree->is_current tells
us that we _are_ asking the question from a secondary worktree,
so we need to make the extra call to check config.worktree file
as well in that case.
Perhaps the logic is clear to those who diagnosed the problem, wrote
the patch, and reviewed it, in which case there is no reason to
reroll. Perhaps it was just me to whom it was not obvious that
the purpose of "is_current" check was not about "are we looking at
the main worktree" but was about "if we are not in the main worktree,
we need this extra check".
Thanks.
