Re: [PATCH v6 3/3] worktree add: improve message for ambiguous remote branch name

>
> Yoichi Nakayama <yoichi.nakayama@gmail.com> writes:
>
> > No. The exit codes of the command 'git worktree add ../topic-branch'
> > are the same (== 0). but the results are different.
> >
> > If there is a unique match found in dwim_branch(), it creates a local
> > branch named topic-branch which tracks <remote>/topic-branch.
> > In case of no match or multiple matches, it creates a local branch
> > named topic-branch from HEAD.
> >
> > Since Git treats both cases as successful, either can be considered
> > the intended behavior.
> > (Although, if there are multiple matches, there is a fair chance the
> > result might not be what was intended.)
> >
> > I am confident that it is appropriate to provide a hint when a command
> > fails, but it is difficult to decide what to do when a command succeeds.
>
> I actually think it falls into the same class of bug you are fixing
> in this topic, which was caused by not considering the possibility
> that there can be any case other than 0-match and 1-match, and not
> thinking through the ramifications of treating 2-match and 0-match
> the same way.
>
> It is of course OK to fix one bug and leave the other one
> unaddressed, to be fixed in a later follow-up effort.
>
> The rest of this message is only for those who will tackle the
> "later follow-up effort" part after the dust settles once the
> current topic lands (aka #leftoverbits).
>
> In the beginning, before Thomas Gummerer started his topic in
> November 2017 [*1*], 'git worktree add <path> [<branch>]' created a
> new branch from the checked-out HEAD, without looking at any
> remote.
>
>  - 'git worktree add <path> <branch>' before Thomas's effort errored
>    out if <branch> did not exist.  It was safe to add DWIM from
>    remote-tracking branches without requiring any option.
>
>  - 'git worktree add <path>' used to create a new branch whose name
>    is derived from basename(path) that points at the current HEAD,
>    without erroring out.  Enabling DWIM from remote-tracking
>    branches unconditionally would have meant a silent behavior
>    change.  So DWIM was added to this case to require the
>    '--guess-remote' option to enable [*2*].
>
> Back then, unique_tracking_name() did not let the callers
> distinguish between 0-match and multiple-match cases, so when you
> had multiple matches, 'git worktree add <path> [<branch>]' triggered
> the same code path as 0-matches.  When the DWIM feature was
> designed, handling the multiple-match case correctly was on nobody's
> radar.
>
> Even when Ævar Arnfjörð Bjarmason updated unique_tracking_name() in
> 3c87aa946a (checkout: pass the "num_matches" up to callers,
> 2018-06-05), in a topic that ends at 8d7b558bae (checkout &
> worktree: introduce checkout.defaultRemote, 2018-06-05), to allow
> callers to distinguish between 0-match and ambiguous multi-match
> cases, this work unfortunately concentrated on improving "git
> checkout", and callers of unique_tracking_name() in "git worktree"
> were updated to pass NULL, i.e., teaching them to count how many
> matches they got was postponed.
>
> We know that the update to unique_tracking_name() in this work back
> then was not complete on the "git worktree" side.  After all, that
> is how this topic arose to fix one of the two code paths that call
> the function so that we react differently between 0-match and
> multiple-match cases.
>
> Now that we are aware of the issue, I think the code should error
> out, instead of creating the new branch out of HEAD, when there are
> multiple remotes with the name of the branch.  In other words, the
> existing code that behaves the same way in 0-match and 2-match cases
> is buggy, and we should eventually fix it.
>
>
> [Footnotes]
>
>  *1* https://lore.kernel.org/git/20171112134305.3949-1-t.gummerer@gmail.com/
>  *2* https://lore.kernel.org/git/20171126194356.16187-1-t.gummerer@gmail.com/