{"thread":{"id":"24828","subject":"[PATCH] disallow branch names that start with a hyphen","startedAt":"2010-08-22T14:08:02Z","lastAt":"2010-09-15T07:34:14Z","messageCount":6,"participants":["Clemens Buchacher","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"148708","messageId":"20100822140801.GA6574@localhost","threadId":"24828","inReplyTo":null,"subject":"[PATCH] disallow branch names that start with a hyphen","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-22T14:08:02Z","receivedAt":"2010-08-22T14:08:02Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"\nBranch names such as \"-\", \"--\" or even \"---\" do\nnot work with git checkout. Anything that starts\nwith a hyphen is also potentially ambiguous with a\ncommand option.\n\nIn order to avoid mistakes, do not allow such\nbranch names.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n refs.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex b540067..6884dff 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -742,7 +742,7 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n  * Make sure \"ref\" is something reasonable to have under \".git/refs/\";\n  * We do not like it if:\n  *\n- * - any path component of it begins with \".\", or\n+ * - any path component of it begins with \".\" or \"-\", or\n  * - it has double dots \"..\", or\n  * - it has ASCII control character, \"~\", \"^\", \":\" or SP, anywhere, or\n  * - it ends with a \"/\".\n@@ -778,7 +778,7 @@ int check_ref_format(const char *ref)\n \t\t\treturn CHECK_REF_FORMAT_ERROR;\n \n \t\t/* we are at the beginning of the path component */\n-\t\tif (ch == '.')\n+\t\tif (ch == '.' || ch == '-')\n \t\t\treturn CHECK_REF_FORMAT_ERROR;\n \t\tbad_type = bad_ref_char(ch);\n \t\tif (bad_type) {\n-- \n1.7.2.1.1.g202c\n\n"},{"id":"148720","messageId":"7vsk262vla.fsf@alter.siamese.dyndns.org","threadId":"24828","inReplyTo":"20100822140801.GA6574@localhost","subject":"Re: [PATCH] disallow branch names that start with a hyphen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-22T21:20:17Z","receivedAt":"2010-08-22T21:20:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> Branch names such as \"-\", \"--\" or even \"---\" do\n> not work with git checkout. Anything that starts\n> with a hyphen is also potentially ambiguous with a\n> command option.\n>\n> In order to avoid mistakes, do not allow such\n> branch names.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n> ---\n>  refs.c |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n\nWhile the inconsistency between \"git branch -- -foo\" (which succeeds and\ncreates refs/heads/-foo) and \"git checkout -foo\" (whcih does not work) is\na valid issue to address, I think this patch does so at a wrong level of\nabstraction.  The abstractions implemented by refs.c and friends are at\nthe plumbing layer that is accessible by Porcelain scripts other people\nwrite, and we should try not to break them with policy decisions the\nPorcelains (e.g. \"git checkout\") we ship as part of the core git (e.g. we\ndo not like a branch whose name begins with a dash).\n\nCurrently these succeed:\n\n    git update-ref refs/mine/-foo HEAD\n    git update-ref -d refs/mine/-foo\n\nso it is reasonable to expect that somebody has a Porcelain that uses a\nconvention to use its own refs namespace \"mine\" with leading dash to mean\nsomething to it.  Your patch would break such a Porcelain rather badly,\nbut I do not think we have to break this to fix your problem.\n\nTwo valid approaches would be: (1) teach a syntax to checkout to allow\nchecking out such a branch; or (2) forbid \"checkout -b\" and \"branch\" from\ncreating such a branch.\n\nI do not think in the context of the core git Porcelain it is necessary\nto start allowing checking out \"-foo\" which we didn't before, so how about\nfixing this by tightening the command line parsing rules for Porcelain\ncommands that create a new branch?\n\nNote that either way update-ref should be kept as is, to allow third party\nPorcelains to keep doing what they have been doing, _and_ to allow\nunfortunate users who ran a broken \"git branch\" command with \"-- -foo\" as\nits arguments to recover from it.\n\nHmm?\n"},{"id":"148729","messageId":"20100823043701.GA11822@localhost","threadId":"24828","inReplyTo":"7vsk262vla.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow branch names that start with a hyphen","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-08-23T04:37:01Z","receivedAt":"2010-08-23T04:37:01Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Aug 22, 2010 at 02:20:17PM -0700, Junio C Hamano wrote:\n> \n> forbid \"checkout -b\" and \"branch\" from creating such a branch.\n\nSounds good to me. I will have limited access to email this week.\nI'll revisit this when I am back.\n\nClemens\n"},{"id":"150707","messageId":"7vtyls6qa7.fsf@alter.siamese.dyndns.org","threadId":"24828","inReplyTo":"20100823043701.GA11822@localhost","subject":"Re: [PATCH] disallow branch names that start with a hyphen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T20:09:20Z","receivedAt":"2010-09-14T20:09:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Sun, Aug 22, 2010 at 02:20:17PM -0700, Junio C Hamano wrote:\n>> \n>> forbid \"checkout -b\" and \"branch\" from creating such a branch.\n>\n> Sounds good to me. I will have limited access to email this week.\n> I'll revisit this when I am back.\n\nHeh, it turns out that we have a perfect place to hook this into.\n\n-- >8 --\nSubject: disallow branch names that start with a hyphen\n\nThe current command line parser overly lax in places and allows a branch\nwhose name begins with a hyphen e.g. \"-foo\" to be created, but the\nparseopt infrastructure in general do not like to parse anything that\nbegin with a dash as a short-hand refname.  \"git checkout -foo\" won't\nwork, nor \"git branch -d -foo\" (even though \"git branch -d -- -foo\" works,\nit does so by mistake; we should not be taking anything but pathspecs\nafter double-dash).\n\nAll the codepath that creates a new branch ref, including the destination\nof \"branch -m src dst\", use strbuf_check_branch_ref() to validate if the\ngiven name is suitable as a branch name.  Tighten it to disallow such a\nname.\n\nYou can still get rid of historical mistake with\n\n  $ git update-ref -d refs/heads/-foo\n\nand third-party Porcelains are free to keep using update-ref to create\nrefs with path component that begins with \"-\".\n\nIssue originally raised by Clemens Buchacher.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\ndiff --git a/strbuf.c b/strbuf.c\nindex bc3a080..65b4cf4 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -399,6 +399,8 @@ int strbuf_branchname(struct strbuf *sb, const char *name)\n int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n {\n \tstrbuf_branchname(sb, name);\n+\tif (name[0] == '-')\n+\t\treturn CHECK_REF_FORMAT_ERROR;\n \tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n \treturn check_ref_format(sb->buf);\n }\n"},{"id":"150710","messageId":"m3eicwkqog.fsf@localhost.localdomain","threadId":"24828","inReplyTo":"7vtyls6qa7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow branch names that start with a hyphen","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-14T20:38:06Z","receivedAt":"2010-09-14T20:38:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Subject: disallow branch names that start with a hyphen\n> \n> The current command line parser overly lax in places and allows a branch\n> whose name begins with a hyphen e.g. \"-foo\" to be created, but the\n> parseopt infrastructure in general do not like to parse anything that\n> begin with a dash as a short-hand refname.  \"git checkout -foo\" won't\n> work, nor \"git branch -d -foo\" (even though \"git branch -d -- -foo\" works,\n> it does so by mistake; we should not be taking anything but pathspecs\n> after double-dash).\n> \n> All the codepath that creates a new branch ref, including the destination\n> of \"branch -m src dst\", use strbuf_check_branch_ref() to validate if the\n> given name is suitable as a branch name.  Tighten it to disallow such a\n> name.\n> \n> You can still get rid of historical mistake with\n> \n>   $ git update-ref -d refs/heads/-foo\n> \n> and third-party Porcelains are free to keep using update-ref to create\n> refs with path component that begins with \"-\".\n> \n> Issue originally raised by Clemens Buchacher.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n> diff --git a/strbuf.c b/strbuf.c\n> index bc3a080..65b4cf4 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -399,6 +399,8 @@ int strbuf_branchname(struct strbuf *sb, const char *name)\n>  int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n>  {\n>  \tstrbuf_branchname(sb, name);\n> +\tif (name[0] == '-')\n> +\t\treturn CHECK_REF_FORMAT_ERROR;\n>  \tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n>  \treturn check_ref_format(sb->buf);\n>  }\n\nShouldn't it include update to Documentation/git-check-ref-format.txt?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"150725","messageId":"20100915073414.GA28927@localhost","threadId":"24828","inReplyTo":"7vtyls6qa7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow branch names that start with a hyphen","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-09-15T07:34:14Z","receivedAt":"2010-09-15T07:34:14Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Tue, Sep 14, 2010 at 01:09:20PM -0700, Junio C Hamano wrote:\n> >\n> > Sounds good to me. I will have limited access to email this week.\n> > I'll revisit this when I am back.\n> \n> Heh, it turns out that we have a perfect place to hook this into.\n\nThank you for taking care of this! Unfortunately, I haven't had the\ntime to do any work at all.\n\nClemens\n"}]}