git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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: Fri, 8 Nov 2019 13:45:36 +0100	[thread overview]
Message-ID: <20191108134536.43d6e476@ingpc3.intern.lauterbach.com> (raw)
In-Reply-To: <xmqq36ezoz6i.fsf@gitster-ct.c.googlers.com>

Hello Junio,

> > 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.  
> 
> Hmph, I thought this was a solved problem, but maybe I am still
> misunderstanding you.

You are right, this is partly solved: 
Deleting such a reference to a local branch is always possible.
But these kind of problems are not solved fully.

To explain my issue more concisely:
I start in any "regular" git repository.
   Regular ==  Repository was cloned from somewhere so a remote
   named "origin" and a remote-tracking branch named "origin/master"
   do exist. 

I fire off the following four commands
(Yes: This is malicious and stupid ;-))

   git checkout master
   git branch origin/master
   git branch remotes/origin/master
   git branch refs/remotes/origin/master

git does not complain at all here.

Then I try the following three commands.

   git branch somework origin/master
      warning: refname 'origin/master' is ambiguous.
      fatal: Ambiguous object name: 'origin/master'.

   git branch somework remotes/origin/master
      warning: refname 'remotes/origin/master' is ambiguous.
      fatal: Ambiguous object name: 'remotes/origin/master'.

   git branch somework refs/remotes/origin/master
      warning: refname 'refs/remotes/origin/master' is ambiguous.
      fatal: Ambiguous object name: 'refs/remotes/origin/master'.


QUESTION: 
I think I am lost now. That's where I might have overlooked something ?



I might continue with (this is the solved case):

   git branch -d refs/remotes/origin/master
      Deleted branch refs/remotes/origin/master (was 3454f30).

Sounds rather scary (because this sounds like you deleted a 
remote-tracking branch), but actually does the right thing I guess.
(The command deletes refs/heads/refs/remotes/origin/master)


After which this succeeds:

   git branch somework refs/remotes/origin/master
      Branch 'somework' set up to track remote branch 'master' from 'origin'.




PATCH:
Make this fail:

   git branch refs/remotes/origin/master
      fatal: Invalid new branch name: 'refs/remotes/origin/master'

This avoids the failure for 

   git branch somework refs/remotes/origin/master


and to avoid very similar issues make these fail too:

   git tag -m "a tag" refs/remotes/origin/master
      fatal: Invalid new tag name: 'refs/remotes/origin/master'

   git remote add refs/heads ssh://ds1/home/irohloff/git/gcc_build.git
      fatal: Invalid new remote name: 'refs/heads'


All of these examples use really pathological names for tags/remotes/branches.
I cannot believe that anyone wants to do this intentionally.

QUESTION:
Are there more user created, command line specified refnames 
in addition to tags/remotes/branches ?




If you have time:

Some more background.
The whole idea behind the patch: 
Make sure "refs/" is a "unique" prefix, which only
appears as ".git/refs/".
This should ensure that "refs/" only matches
to the very first entry from:
	static const char *ref_rev_parse_rules[] = {
		"%.*s",
		"refs/%.*s",
		"refs/tags/%.*s",
		"refs/heads/%.*s",
		"refs/remotes/%.*s",
		"refs/remotes/%.*s/HEAD",
		NULL
	};  

So goal: Make sure
  refs/refs/*          does not exist
  refs/tags/refs/*     does not exist
  refs/heads/refs/*    does not exist
  refs/remotes/refs/*  does not exist

To avoid the existence of refs/remotes/refs/* it is necessary to 
also prohibit a standalone "refs" as remote name (not just "refs/*");
and to handle that more easily I also prohibit a standalone "refs"
for tags and branches.

Of course you might still create all these nasty subdirs with plumbing.
I try to avoid that this is done with porcelain.
(At least that's as far as I understand git terminology.)
Of course future git extensions might try to create something like
  .git/refs/refs/*
but since these extensions are reviewed, I guess it is easy to nudge
authors of extensions (like git-svn, git-bisect, ...) to NOT do this.

so long
  Ingo


PS: 
I really think per default more prefixes than just "refs/" should
be forbidden when creating tags/remotes/branches.
But I also agree with you that this is much less straight forward
(Which prefixes to forbid ? Config option ? How much does this break ? ...).
As far as I can tell tags/remotes/branches, which 
are called "refs/*" or "refs" are completely pathological; 
I think unconditionally forbidding to create these kind of 
refnames for tags/remotes/branches with porcelain is OK.

BTW: This is also quite confusing 
(but does not really hurt and is consistent with what you described)

   git branch -r -d refs/remotes/origin/master
      error: remote-tracking branch 'refs/remotes/origin/master' not found.

What is meant here is I think

   remote-tracking branch 'refs/remotes/refs/remotes/origin/master' not found.

  reply	other threads:[~2019-11-08 12:45 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
2019-11-08  2:54     ` Junio C Hamano
2019-11-08 12:45       ` Ingo Rohloff [this message]
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191108134536.43d6e476@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).