From: Ingo Rohloff <ingo.rohloff@lauterbach.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] branch: Forbid to create local branches with confusing names
Date: Thu, 7 Nov 2019 13:54:09 +0100 [thread overview]
Message-ID: <20191107135409.13fa0336@ingpc3.intern.lauterbach.com> (raw)
In-Reply-To: <xmqqftj0qkzm.fsf@gitster-ct.c.googlers.com>
[-- Attachment #1: Type: text/plain, Size: 6337 bytes --]
On Thu, 07 Nov 2019 15:05:49 +0900
Junio C Hamano <gitster@pobox.com> wrote:
> Ingo Rohloff <ingo.rohloff@lauterbach.com> writes:
>
> > ...
> > This patch forbids to create local branches, with names which start
> > with any of
> >
> > refs/
> > heads/
> > remotes/
> > tags/
>
> Is there a reason why notes/ hierarchy is excluded? What about
> pull/? Are we dealing with an unbounded set? Should this list be
> somehow end-user configurable so that say Gerrit users can add for/
> and changes/ to the "forbidden" set?
I think I did not explain the intention that well.
What I basically want to avoid is a situation in which there is
no way at all to refer unambiguously to a particular reference.
Right now this is easily possible.
I use the following sequence of commands to get into this
situation ("gcc_build" is just a test repository with nothing special).
rm -rf gcc_build
git clone ssh://ds1/home/irohloff/git/gcc_build.git
cd gcc_build
git checkout -b work
echo "work..." >> 00_readme.txt
git commit -a -m "work..."
git branch origin/master
git branch remotes/origin/master
git branch refs/remotes/origin/master
git branch -avv
The last "git branch -avv" outputs:
master 520d29e [refs/remotes/origin/master] initial scripts for building cross compiler GCC
origin/master b8efa13 work...
refs/remotes/origin/master b8efa13 work...
remotes/origin/master b8efa13 work...
* work b8efa13 work...
remotes/origin/HEAD -> refs/remotes/origin/master
remotes/origin/master 520d29e initial scripts for building cross compiler GCC
So this already is a major confusion, because git reports there are two references
with the same name "remotes/origin/master" which point to DIFFERENT objects.
What's worse: I cannot figure out a way to unambiguously refer to
the remote tracking branch remotes/origin/master:
git log origin/master
warning: refname 'origin/master' is ambiguous.
True: This refers to both
refs/remotes/origin/master
refs/heads/origin/master
git log remotes/origin/master
warning: refname 'remotes/origin/master' is ambiguous.
True: This refers to both
refs/remotes/origin/master
refs/heads/remotes/origin/master
git branch refs/remotes/origin/master
warning: refname 'remotes/origin/master' is ambiguous.
True: This refers to both
refs/remotes/origin/master
refs/heads/refs/remotes/origin/master
Now I have run out of ideas how to refer to the remote tracking branch.
So what I want to ensure is that there exists at least one way to
address a reference unambiguously.
(The SHA-1 is not really an option, because it changes each time you
update the branch head.)
I do not want to prevent people from creating ambiguities in general
or weird branch names in general, because I think that's a much harder and
maybe unsolvable problem.
I just want to make sure that there is at least one unambiguous way to
refer to a specific reference.
So to answer your questions:
> Is there a reason why notes/ hierarchy is excluded?
> What about pull/?
I basically wanted to exclude the prefixes which git auto-adds when
expanding reference names.
As far as I understand the source code, this means excluding the prefixes
which are a result of the declaration of
ref_rev_parse_rules
So these are
refs/
remotes/
tags/
heads/
Maybe I do not really understand the source code (or my logic is bogus)
and git might somehow expand a reference abbreviation by adding
"notes/" or "pull/" but I do not know how to trigger this expansion.
That is my rationale behind this set of prefixes and why
I did not include things like "notes/", "pull/" ...
Having said this:
I think it might be enough to just forbid any local branch names,
which have a prefix of "refs/".
Rationale behind that:
I said that I want to have at least one way to unambiguously
refer to a reference.
If I forbid "refs/" as prefix, then I think in the above example,
I can always use
git log refs/remotes/origin/master
because
.git/refs/heads/refs/.... does not exist.
Thinking more about this:
Preventing local branches names with a "refs/" prefix
is just the tip of the iceberg.
Someone might use
git tag refs/remotes/origin/master
or
git add remote refs/remotes/origin <URL>
git fetch refs/remotes/origin
After this refs/remotes/origin/master is again ambiguous.
> Should this list be somehow end-user configurable so that say Gerrit users
> can add for/ and changes/ to the "forbidden" set?
This might be interesting in the future, but at least for now the goal was to
prevent a situation in which there is no unambiguous name at all for a reference.
So I was not thinking about a "convenience" feature for users but really preventing
to get the repository into an almost unusable state.
So to answer your question: No; right now I did not think that this should be
end-user configurable, because the set of prefixes is derived from the declaration
of "ref_rev_parse_rules", which contains a non-configurable set of rules.
>
> This is not a change to builtin/branch.c, so other commands that
> call create_branch() would be affected---are they all equipped to
> toggle on the same bit that is affected by the '-f' option you have
> in mind (presumably "git branch -f")? Wouldn't forcing for those
> other command have undesirable side effects?
>
> I didn't check the code, but I think "checkout -b" also calls
> create_branch() so presumably it also is affected. Using "-B"
> instead of "-b" for "checkout" might pass the force bit on, but
> typically that is done only to recreate an existing branch. Is it a
> good idea to change the meaning of "-B" to also mean "do not bother
> checking the sanity of the branch name"?
>
Yes you are completely right:
I was not at all sure where to put the check.
As mentioned above: If the goal is to prevent to get a git repository into
a super confusing state, then you probably also need to add constraints for
remote names and tag names.
Are there more names which are part of reference names ?
so long
Ingo
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3017 bytes --]
next prev parent reply other threads:[~2019-11-07 12:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-06 16:56 [PATCH] branch: Forbid to create local branches with confusing names Ingo Rohloff
2019-11-06 22:15 ` Johannes Schindelin
2019-11-07 19:04 ` Ingo Rohloff
2019-11-07 6:05 ` Junio C Hamano
2019-11-07 12:54 ` Ingo Rohloff [this message]
2019-11-08 2:54 ` Junio C Hamano
2019-11-08 12:45 ` Ingo Rohloff
2019-11-07 19:07 ` [PATCH v2 0/4] Do not create new refnames "refs" or "refs/*" Ingo Rohloff
2019-11-07 19:07 ` [PATCH v2 1/4] refs: new function newname_has_bad_prefix Ingo Rohloff
2019-11-07 19:07 ` [PATCH v2 2/4] branch: Prevent creation of local branches named "refs" or "refs/*" Ingo Rohloff
2019-11-07 19:07 ` [PATCH v2 3/4] remote: Prevent users from creating remotes " Ingo Rohloff
2019-11-07 19:07 ` [PATCH v2 4/4] tag: Prevent users from creating tags " Ingo Rohloff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191107135409.13fa0336@ingpc3.intern.lauterbach.com \
--to=ingo.rohloff@lauterbach.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).