* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:03 ` Junio C Hamano
@ 2014-02-18 19:35 ` John Keeping
2014-02-18 19:51 ` Junio C Hamano
2014-02-18 19:37 ` David Kastrup
2016-03-23 11:17 ` Duy Nguyen
2 siblings, 1 reply; 21+ messages in thread
From: John Keeping @ 2014-02-18 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
On Tue, Feb 18, 2014 at 11:03:10AM -0800, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > Prevent is a strong word. I meant we only do it if they force
> > it. Something like this..
> >
> > -- 8< --
> > diff --git a/branch.c b/branch.c
> > index 723a36b..3f0540f 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -251,6 +251,11 @@ void create_branch(const char *head,
> > forcing = 1;
> > }
> >
> > + if (!force && dwim_ref(name, strlen(name), sha1, &real_ref))
> > + die(_("creating ref refs/heads/%s makes %s ambiguous.\n"
> > + "Use -f to create it anyway."),
> > + name, name);
>
> Does this check still allow you to create a branch "refs/heads/next"
> and then later create a branch "next"? The latter will introduce an
> ambiguity without any prevention, even though the prevention would
> trigger if the order in which these two branches are created is
> swapped--- the end result has ambiguity but the safety covers only
> one avenue to the confusing situation.
>
> And the only way I can think of to avoid that kind of confusion is
> to forbid creation of a subset of possible names by reserving a set
> of known (but arbitrary) prefixes---which I am not sure is a good
> way to go. At least not yet.
There's already the arbitrary set of prefixes in
refs.c::prettify_refname() and refs.c::ref_rev_parse_rules(). I can see
how a user might think that since "git log refs/heads/name" is
equivalent to "git log master" then "git branch refs/heads/name" should
be equivalent to "git branch name".
I don't think requiring "--force" for these branch names that overlap
with these special namespaces is a big leap from supporting them for
inspection commands. Although I'm not sure how sensible it is to
examine every remote name to catch something like "git branch
origin/master". Perhaps Duy's ambiguity check is the best thing for
that case.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:35 ` John Keeping
@ 2014-02-18 19:51 ` Junio C Hamano
2014-02-18 20:01 ` John Keeping
2014-02-20 4:17 ` Michael Haggerty
0 siblings, 2 replies; 21+ messages in thread
From: Junio C Hamano @ 2014-02-18 19:51 UTC (permalink / raw)
To: John Keeping; +Cc: Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
John Keeping <john@keeping.me.uk> writes:
> There's already the arbitrary set of prefixes in
> refs.c::prettify_refname() and refs.c::ref_rev_parse_rules(). I can see
> how a user might think that since "git log refs/heads/name" is
> equivalent to "git log master" then "git branch refs/heads/name" should
> be equivalent to "git branch name".
Not quite, I am afraid. Branch names used for "git branch <name>"
and "git checkout <name>" are like the Lvalue of an assignment, as
opposed to extended SHA-1 expressions to express any commit
(e.g. 'master^0', 'refs/heads/master', or 'master') that correspond
to the Rvalues used in an expression. Because "git checkout" can
take a branch name or an arbitrary commit object name, there needs a
way for the users to disambiguate.
Saying that "git checkout refs/heads/name" must be equivalent to
"git checkout name" is like arguing that assignment "value+0 = x"
should be valid because "value+0" is a valid value.
For the first parameter to "git branch", there is no ambiguity---it
must be the name of a branch and cannot be an arbitrary commit
object name, so special casing "git branch refs/heads/master" to
mean "git branch master" may not be too bad. But then we need to
either start rejecting "git branch refs/tags/v1.0" or keep allowing
it to create a ref refs/heads/refs/tags/v1.0 to denote the branch of
that exact name given---neither of which is more consistent, so...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:51 ` Junio C Hamano
@ 2014-02-18 20:01 ` John Keeping
2014-02-20 4:17 ` Michael Haggerty
1 sibling, 0 replies; 21+ messages in thread
From: John Keeping @ 2014-02-18 20:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
On Tue, Feb 18, 2014 at 11:51:05AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > There's already the arbitrary set of prefixes in
> > refs.c::prettify_refname() and refs.c::ref_rev_parse_rules(). I can see
> > how a user might think that since "git log refs/heads/name" is
> > equivalent to "git log master" then "git branch refs/heads/name" should
> > be equivalent to "git branch name".
>
> Not quite, I am afraid. Branch names used for "git branch <name>"
> and "git checkout <name>" are like the Lvalue of an assignment, as
> opposed to extended SHA-1 expressions to express any commit
> (e.g. 'master^0', 'refs/heads/master', or 'master') that correspond
> to the Rvalues used in an expression. Because "git checkout" can
> take a branch name or an arbitrary commit object name, there needs a
> way for the users to disambiguate.
>
> Saying that "git checkout refs/heads/name" must be equivalent to
> "git checkout name" is like arguing that assignment "value+0 = x"
> should be valid because "value+0" is a valid value.
That isn't my argument. I agree that the "create" and "view" operations
are distinct. The problem appears to be that not all users expect this
and it caused them confusion because they are able to create refs like
refs/heads/refs/heads/master.
I don't think we should apply the mapping, but I also do not think that
using a set of known (but arbitrary) constraints to prevent the creation
of potentially confusing branch names is a particularly big leap from
applying a similar set of rules in prettify_ref() and dwim_ref().
Especially if there's an escape hatch via '--force' or 'git update-ref'
for those who know what they're doing.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:51 ` Junio C Hamano
2014-02-18 20:01 ` John Keeping
@ 2014-02-20 4:17 ` Michael Haggerty
2014-02-20 18:22 ` Junio C Hamano
1 sibling, 1 reply; 21+ messages in thread
From: Michael Haggerty @ 2014-02-20 4:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: John Keeping, Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
On 02/18/2014 08:51 PM, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
>> There's already the arbitrary set of prefixes in
>> refs.c::prettify_refname() and refs.c::ref_rev_parse_rules(). I can see
>> how a user might think that since "git log refs/heads/name" is
>> equivalent to "git log master" then "git branch refs/heads/name" should
>> be equivalent to "git branch name".
>
> Not quite, I am afraid. Branch names used for "git branch <name>"
> and "git checkout <name>" are like the Lvalue of an assignment, as
> opposed to extended SHA-1 expressions to express any commit
> (e.g. 'master^0', 'refs/heads/master', or 'master') that correspond
> to the Rvalues used in an expression. Because "git checkout" can
> take a branch name or an arbitrary commit object name, there needs a
> way for the users to disambiguate.
>
> Saying that "git checkout refs/heads/name" must be equivalent to
> "git checkout name" is like arguing that assignment "value+0 = x"
> should be valid because "value+0" is a valid value.
Your logic is impeccable...and yet the user's logic is also quite
reasonable. I fell into this trap when I started using Git, and so did
most (all?) of my colleagues.
I think the problem is partly caused by the visual and semantic
similarity between references and Unix pathnames. For pathnames, the
file that is called "filename.txt" in my current context has an
unambiguous, canonical name that might be "/home/mhagger/filename.txt".
My first mental model of Git references was that "branch" and
"refs/heads/branch" are synonyms, and that the latter is somehow the
"unambiguous" and "canonical" way to write it. I think this mental
model is what led me to make the universal beginner's mistake
git branch refs/heads/mybranch
It took me a while to figure out how to fix the situation, and the whole
experience was very frustrating.
I wonder whether we could give a way to specify a reference in an
unambiguous, canonical fashion like I expected, for example by using a
leading slash: "/refs/heads/mybranch". This could be a way for the user
to ask for DWIMming to be turned off without having to resort to
plumbing commands like update-ref. This wouldn't necessarily solve the
problem, but it would at least lead the new user to type
git branch /refs/heads/mybranch
instead of the ambiguous command above, which Git could either accept or
reject in good conscience rather than having to speculate about what the
user *really* meant. I think that supporting absolute reference names
like this would also be useful for scripts, which otherwise probably
often have subtle failure modes if the user has defined reference names
that are ambiguous, modulo DWIM, with the reference that the script
intended.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: error: src refspec refs/heads/master matches more than one.
2014-02-20 4:17 ` Michael Haggerty
@ 2014-02-20 18:22 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2014-02-20 18:22 UTC (permalink / raw)
To: Michael Haggerty
Cc: John Keeping, Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I wonder whether we could give a way to specify a reference in an
> unambiguous, canonical fashion like I expected, for example by using a
> leading slash: "/refs/heads/mybranch". This could be a way for the user
> to ask for DWIMming to be turned off without having to resort to
> plumbing commands like update-ref. This wouldn't necessarily solve the
> problem, but it would at least lead the new user to type
>
> git branch /refs/heads/mybranch
>
> instead of the ambiguous command above, which Git could either accept or
> reject in good conscience rather than having to speculate about what the
> user *really* meant. I think that supporting absolute reference names
> like this would also be useful for scripts, which otherwise probably
> often have subtle failure modes if the user has defined reference names
> that are ambiguous, modulo DWIM, with the reference that the script
> intended.
I do agree that things start to become confusing to the end users
when we tell refnames and object names apart and behave differently,
e.g. "git checkout master" vs "git checkout master^0" (this example
uses a disambiguation syntax that is related to but different from
what you brought up).
For the <name> in "git branch <name> [<commit>]" (but not <commit>),
I do not see much value in allowing the users to say "refs/heads/"
in the first place---all the local branch refs are to be created in
refs/heads/ anyway and "git branch /refs/tags/bar" (if we were to
allow your notation to name an absolute ref) will have to be checked
and signaled as an error.
Even though there is no reason to forbid a ref to be named in such a
way at the lowest machinery level (read: at the sha1_name.c layer)
[*1*], I would say it would be better to at least warn users when
they create such a ref with Porcelain commands like "branch",
"checkout -b", etc., or even outright forbid.
In other contexts, however, it _might_ make sense, but I am somewhat
skeptical. For example, if you have a branch 'foo' (whose ref being
refs/heads/foo) and a branch 'refs/heads/foo' (whose ref being
refs/heads/refs/heads/foo) at the same time, you need some way to
clarify that you mean the former, and one way to do so may be
git branch newfoo /refs/heads/foo
and removing the latter would be
git branch -D /refs/heads/refs/heads/foo
perhaps. But this starts to sound like a workaround to a problem
that the user ended up having such a strangely named branch in the
first place, not a useful feature.
[Footnote]
*1* The way refs are used and the specific meanings given to some of
the hierarchies under refs/ by the core-git Porcelain is not
fundamental to the data model of Git. Git the SCM by convention
uses refs/heads/ for branches, and it is perfectly fine for Git
the SCM to enforce its own policy like that to its end users and
forbid creating and using any ref outside that hierarchy as a
local branch (e.g. checking it out), but I'd prefer it if we can
keep the lower level "a general filesystem to build SCM on top"
layer as separate from such policy decision as possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:03 ` Junio C Hamano
2014-02-18 19:35 ` John Keeping
@ 2014-02-18 19:37 ` David Kastrup
2014-02-18 21:47 ` Junio C Hamano
2016-03-23 11:17 ` Duy Nguyen
2 siblings, 1 reply; 21+ messages in thread
From: David Kastrup @ 2014-02-18 19:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
Junio C Hamano <gitster@pobox.com> writes:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> + if (!force && dwim_ref(name, strlen(name), sha1, &real_ref))
>> + die(_("creating ref refs/heads/%s makes %s ambiguous.\n"
>> + "Use -f to create it anyway."),
>> + name, name);
>
> Does this check still allow you to create a branch "refs/heads/next"
> and then later create a branch "next"? The latter will introduce an
> ambiguity without any prevention, even though the prevention would
> trigger if the order in which these two branches are created is
> swapped--- the end result has ambiguity but the safety covers only one
> avenue to the confusing situation.
>
> And the only way I can think of to avoid that kind of confusion is
> to forbid creation of a subset of possible names by reserving a set
> of known (but arbitrary) prefixes---which I am not sure is a good
> way to go. At least not yet.
Just for the record: after seeing the respective arguments, I consider
it the sanest way.
It's conceivable to give a configuration option for augmenting the set
of reserved prefixes. That would allow to adapt the arbitrariness to
match the policies or ref name choices of a particular project while
keeping the set of references addressed by the standard git commands in
check automagically.
--
David Kastrup
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:37 ` David Kastrup
@ 2014-02-18 21:47 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2014-02-18 21:47 UTC (permalink / raw)
To: David Kastrup; +Cc: Duy Nguyen, Andreas Schwab, Josef Wolf, Git Mailing List
David Kastrup <dak@gnu.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> + if (!force && dwim_ref(name, strlen(name), sha1, &real_ref))
>>> + die(_("creating ref refs/heads/%s makes %s ambiguous.\n"
>>> + "Use -f to create it anyway."),
>>> + name, name);
>>
>> Does this check still allow you to create a branch "refs/heads/next"
>> and then later create a branch "next"? The latter will introduce an
>> ambiguity without any prevention, even though the prevention would
>> trigger if the order in which these two branches are created is
>> swapped--- the end result has ambiguity but the safety covers only one
>> avenue to the confusing situation.
>>
>> And the only way I can think of to avoid that kind of confusion is
>> to forbid creation of a subset of possible names by reserving a set
>> of known (but arbitrary) prefixes---which I am not sure is a good
>> way to go. At least not yet.
>
> Just for the record: after seeing the respective arguments, I consider
> it the sanest way.
>
> It's conceivable to give a configuration option for augmenting the set
> of reserved prefixes. That would allow to adapt the arbitrariness to
> match the policies or ref name choices of a particular project while
> keeping the set of references addressed by the standard git commands in
> check automagically.
I am inclined to say that we should start by just giving a warning
whenever "git branch", "git checkout -b", etc. tries to create a
branch whose name begins with "refs/" and other potentially
ambiguous ones that match ref_rev_parse_rules[]. I personally do
not think people name their branch with a name that begins with
"refs/" on purpose; I am not sure about other ones, like "heads" or
"tags". Personally I think it also is unlikely to want to have these
words immediately followed by a slash in the branch name, so it may
not even be necessary to give them any way to turn off the warning,
which in turn would mean we can promote that warning to an die()
that can be overridable with "--force", but by going that "first
warn and see if people are annoyed" route, we would hopefully find
out soon enough that there may be some people who do want to name
their branches in a funny way ;-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: error: src refspec refs/heads/master matches more than one.
2014-02-18 19:03 ` Junio C Hamano
2014-02-18 19:35 ` John Keeping
2014-02-18 19:37 ` David Kastrup
@ 2016-03-23 11:17 ` Duy Nguyen
2 siblings, 0 replies; 21+ messages in thread
From: Duy Nguyen @ 2016-03-23 11:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Andreas Schwab, Josef Wolf, Git Mailing List, Michael Haggerty,
John Keeping
I'm digging this topic up from the ground because if it was fixed, I
would not have spent many confusing (and a little bit panicking)
minutes wondering how the hell I managed to push to "origin/master"
which I did not have push access to begin with, which turned out to be
the local branch, refs/heads/origin/master, not the remote branch
refs/remotes/origin/master.
Summary until this point in the old thread, you can accidentally do
"git branch refs/heads/next", which happily creates
refs/heads/refs/heads/next. If you also have refs/heads/next, things
can get confusing for commands that accepts both branch and non-branch
refs. There were some more discussion about unambiguously specifying
full ref in git-branch but it was getting nowhere.
On Wed, Feb 19, 2014 at 2:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Prevent is a strong word. I meant we only do it if they force
>> it. Something like this..
>>
>> -- 8< --
>> diff --git a/branch.c b/branch.c
>> index 723a36b..3f0540f 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -251,6 +251,11 @@ void create_branch(const char *head,
>> forcing = 1;
>> }
>>
>> + if (!force && dwim_ref(name, strlen(name), sha1, &real_ref))
>> + die(_("creating ref refs/heads/%s makes %s ambiguous.\n"
>> + "Use -f to create it anyway."),
>> + name, name);
>
> Does this check still allow you to create a branch "refs/heads/next"
> and then later create a branch "next"? The latter will introduce an
> ambiguity without any prevention, even though the prevention would
> trigger if the order in which these two branches are created is
> swapped--- the end result has ambiguity but the safety covers only
> one avenue to the confusing situation.
It could be fixed by checking all existing refs if any of them becomes
ambiguous after refs/heads/next comes to life. We can filter and check
only branch that shares the same base name (or starts with "refs/",
"heads/", "tags/" or "remotes/"), so it's still expensive but not as
much as checking all refs.
Even with covering only one avenue, it would definitely help my case
(refs/remotes/origin/master exists and refs/heads/origin/master asked
to be created).
Also in my case, if refs/heads/origin/master already exists then I
think I should reject creating refs/remotes/origin/master, or warn
loudly.
Sounds promising enough to start making patches?
> And the only way I can think of to avoid that kind of confusion is
> to forbid creation of a subset of possible names by reserving a set
> of known (but arbitrary) prefixes---which I am not sure is a good
> way to go. At least not yet.
--
Duy
^ permalink raw reply [flat|nested] 21+ messages in thread