{"thread":{"id":"21185","subject":"[PATCH] disallow refs containing successive slashes","startedAt":"2009-10-10T17:49:48Z","lastAt":"2009-10-13T04:49:30Z","messageCount":17,"participants":["Jens Lehmann","Junio C Hamano","Jonathan Nieder","Nanako Shiraishi","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"124579","messageId":"4AD0C93C.6050306@web.de","threadId":"21185","inReplyTo":null,"subject":"[PATCH] disallow refs containing successive slashes","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2009-10-10T17:49:48Z","receivedAt":"2009-10-10T17:49:48Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When creating branches using names starting with '/' or containing a '//',\na leading slash would just vanish and successive slashes were 'compressed'\ninto just one slash.\n\nThis behaviour comes from the refs being stored as files in subdirectories\nwhere the filesystem compresses each '//' in a pathname to a '/' . So\ndisallowing refs to contain '//' fixes these problems and prohibits\nbranch names to start with a '/' or to contain '//'.\n\nSigned-off-by: Jens Lehmann \n---\n\nI became aware of this issue while looking into problems occuring\nwhen a user created a branch starting with a '/' in git gui (e.g.\n\"/foo\"). Strange things happen, while git gui shows the current\nbranch as \"/foo\" under the hood a branch \"foo\" (without the slash)\nhad been created. But then you can't delete \"/foo\" from git gui,\nbecause a branch of that name doesn't exist.\n\nIMHO branch names should not be allowed to start with a '/' or\ncontain a '//' (branches with trailing slashes are not allowed since\ncommit 03feddd6). So this implies that refs should never contain a\n'//' in the first place.\n\nThis behaviour has been changed the last time by commit 03feddd.\nThe version before that commit didn't allow '//' to be part of a\nref (but still allowed a trailing '/'). The newer version explicitly\nstates \"/* tolerate duplicated slashes */\" in the added code. But i\ncouldn't find any hints on the list or on the interweb *why* these\nwere allowed. So this patch might break something i can't see right\nnow, even though the test suite runs fine ...\n\nThoughts? Objections?\n\n\n Documentation/git-check-ref-format.txt | 2 ++\n refs.c | 7 +++++--\n t/t1600-check-ref-format.sh | 19 +++++++++++++++++++\n 3 files changed, 26 insertions(+), 2 deletions(-)\n create mode 100755 t/t1600-check-ref-format.sh\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex 0b7982e..180ffbb 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -38,6 +38,8 @@ imposes the following rules on how references are named:\n\n . They cannot end with a slash `/` nor a dot `.`.\n\n+. They cannot contain two successive slashes `//`.\n+\n . They cannot end with the sequence `.lock`.\n\n . They cannot contain a sequence `@{`.\ndiff --git a/refs.c b/refs.c\nindex 808f56b..cedd768 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -687,6 +687,7 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n * - it has double dots \"..\", or\n * - it has ASCII control character, \"~\", \"^\", \":\" or SP, anywhere, or\n * - it ends with a \"/\".\n+ * - it contains '//'\n * - it ends with \".lock\"\n * - it contains a \"\\\" (backslash)\n */\n@@ -712,8 +713,10 @@ int check_ref_format(const char *ref)\n\n \tlevel = 0;\n \twhile (1) {\n-\t\twhile ((ch = *cp++) == '/')\n-\t\t\t; /* tolerate duplicated slashes */\n+\t\tif ((ch = *cp++) == '/')\n+\t\t\t/* Don't tolerate successive slashes */\n+\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\n \t\tif (!ch)\n \t\t\t/* should not end with slashes */\n \t\t\treturn CHECK_REF_FORMAT_ERROR;\ndiff --git a/t/t1600-check-ref-format.sh b/t/t1600-check-ref-format.sh\nnew file mode 100755\nindex 0000000..05fb9f9\n--- /dev/null\n+++ b/t/t1600-check-ref-format.sh\n@@ -0,0 +1,19 @@\n+#!/bin/sh\n+\n+test_description='git check-ref-format'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'refs/heads/master is ok' '\n+\tgit check-ref-format refs/heads/master\n+'\n+\n+test_expect_success 'no successive slashes allowed' '\n+\ttest_must_fail git check-ref-format a//b\n+'\n+\n+test_expect_success 'no trailing slashes allowed' '\n+\ttest_must_fail git check-ref-format a/b/\n+'\n+\n+test_done\n-- \n1.6.5.rc2.205.g1185a\n"},{"id":"124583","messageId":"7vws327wbp.fsf@alter.siamese.dyndns.org","threadId":"21185","inReplyTo":"4AD0C93C.6050306@web.de","subject":"Re: [PATCH] disallow refs containing successive slashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-10T21:50:18Z","receivedAt":"2009-10-10T21:50:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> When creating branches using names starting with '/' or containing a '//',\n> a leading slash would just vanish and successive slashes were 'compressed'\n> into just one slash.\n\nHmm. We already do that without your patch.\n\n $ git branch /foo//bar\n $ git for-each-ref --format='%(refname)'\n refs/heads/foo/bar\n refs/heads/master\n $ git branch -d /foo//bar\n Deleted branch /foo//bar (was deadbeef)\n $ git for-each-ref --format='%(refname)'\n refs/heads/master\n\n> Signed-off-by: Jens Lehmann \n> ---\n>\n> I became aware of this issue while looking into problems occuring\n> when a user created a branch starting with a '/' in git gui (e.g.\n> \"/foo\"). Strange things happen, while git gui shows the current\n> branch as \"/foo\" under the hood a branch \"foo\" (without the slash)\n> had been created. But then you can't delete \"/foo\" from git gui,\n> because a branch of that name doesn't exist.\n\nPerhaps an interface to give a cleaned-up version, e.g.\n\n $ git check-ref-format --print refs/heads//foo/bar\n refs/heads/foo/bar\n\nis what you want in order to fix git-gui? I dunno.\n"},{"id":"124600","messageId":"4AD1B6A4.8060405@web.de","threadId":"21185","inReplyTo":"7vws327wbp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow refs containing successive slashes","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2009-10-11T10:42:44Z","receivedAt":"2009-10-11T10:42:44Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Junio C Hamano schrieb:\n> Jens Lehmann writes:\n> \n>> When creating branches using names starting with '/' or containing a '//',\n>> a leading slash would just vanish and successive slashes were 'compressed'\n>> into just one slash.\n> \n> Hmm. We already do that without your patch.\n> \n> $ git branch /foo//bar\n> $ git for-each-ref --format='%(refname)'\n> refs/heads/foo/bar\n> refs/heads/master\n> $ git branch -d /foo//bar\n> Deleted branch /foo//bar (was deadbeef)\n> $ git for-each-ref --format='%(refname)'\n> refs/heads/master\n\nYes, but git changes the name silently from '/foo//bar' to 'foo/bar'!\n\nThe automagical removal of some slashes leads to strange behaviour when\nusing such names:\n\n $ git checkout -b foo/bar\n Switched to a new branch 'foo/bar'\n $ git checkout -b /foo//bar\n fatal: A branch named '/foo//bar' already exists.\n $ git for-each-ref --format='%(refname)'\n refs/heads/foo/bar\n refs/heads/master\n\ngit claims wrongly that \"A branch named '/foo//bar' already exists.\".\n\nAnd that same happens to tag and repo names, leading to other strange\neffects:\n\n $ git tag /foo//bar\n $ git tag\n foo/bar\n\nThat is not the tagname the user provided.\n\n $ git remote add /foo//bar git://git.kernel.org/pub/scm/git/git.git\n $ git remote show\n warning: Config remote shorthand cannot begin with '/': /foo//bar.url\n warning: Config remote shorthand cannot begin with '/': /foo//bar.fetch\n $ git fetch /foo//bar\n warning: Config remote shorthand cannot begin with '/': /foo//bar.url\n warning: Config remote shorthand cannot begin with '/': /foo//bar.fetch\n fatal: 'foo/bar' does not appear to be a git repository\n fatal: The remote end hung up unexpectedly\n\nNote: git fetch uses both versions of the remote name in its output.\n\n\n>> I became aware of this issue while looking into problems occuring\n>> when a user created a branch starting with a '/' in git gui (e.g.\n>> \"/foo\"). Strange things happen, while git gui shows the current\n>> branch as \"/foo\" under the hood a branch \"foo\" (without the slash)\n>> had been created. But then you can't delete \"/foo\" from git gui,\n>> because a branch of that name doesn't exist.\n> \n> Perhaps an interface to give a cleaned-up version, e.g.\n> \n> $ git check-ref-format --print refs/heads//foo/bar\n> refs/heads/foo/bar\n> \n> is what you want in order to fix git-gui? I dunno.\n\nYes, one solution could be to fix every application handling branch, tag\nor repo names to mimic the namechange done in the bowels of git. But i\nthink it is not worth the hassle. Every application i tested (git gui,\ngitk and a handful of git commands) just assumes - and i think rightfully\nso - that refnames will not change while they are being created (And they\ndo call \"git check-ref-format\" or strbuf_check_branch_ref() and friends\nto make sure they have a valid refname, but it gets changed nonetheless).\n\nWith this patch you get an error every time you try to create such a\nrefname in the first place and the strange effects and the ambiguity\nof refnames ('/foo//bar' is a synonym for 'foo/bar' right now) just\ngo away.\n\nThe motivation for this patch was a confused user at my dayjob. IMHO\nmost people don't use '/foo//bar' on purpose but the extra slashes get\nthere through typos, copy & paste errors and such. Then generating an\nerror might just be the Right Thing to do to avoid the problems with\nthe changed refname afterwards.\n"},{"id":"124629","messageId":"7vbpkdwyo2.fsf@alter.siamese.dyndns.org","threadId":"21185","inReplyTo":"4AD1B6A4.8060405@web.de","subject":"Re: [PATCH] disallow refs containing successive slashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-11T18:52:45Z","receivedAt":"2009-10-11T18:52:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n>>> I became aware of this issue while looking into problems occuring\n>>> when a user created a branch starting with a '/' in git gui (e.g.\n>>> \"/foo\"). Strange things happen, while git gui shows the current\n>>> branch as \"/foo\" under the hood a branch \"foo\" (without the slash)\n>>> had been created. But then you can't delete \"/foo\" from git gui,\n>>> because a branch of that name doesn't exist.\n>> \n>> Perhaps an interface to give a cleaned-up version, e.g.\n>> \n>> $ git check-ref-format --print refs/heads//foo/bar\n>> refs/heads/foo/bar\n>> \n>> is what you want in order to fix git-gui? I dunno.\n>\n> Yes, one solution could be to fix every application handling branch, tag\n> or repo names to mimic the namechange done in the bowels of git. But i\n> think it is not worth the hassle.\n\nThat cuts both ways.\n\nWhen the users make typoes (e.g. /foo//bar) you can accept the only sane\ncorrection (e.g. foo/bar) instead of rejecting, since the only thing the\nuser can do after getting such a rejection is to correct it to that\ncorrected name (e.g. foo/bar) himself and re-issue the command anyway.\nYou can push the \"hassle\" down to the user, or you can fix the tool to\nremove the hassle from the user.\n\nBesides, by rejecting what we used to accept you are breaking people's\nexpectations. So I am moderately negative, unless you can say your \"every\napplication\" is literally _tons_.\n"},{"id":"124651","messageId":"20091012003115.GA5782@progeny.tock","threadId":"21185","inReplyTo":"7vbpkdwyo2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow refs containing successive slashes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T00:31:15Z","receivedAt":"2009-10-12T00:31:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n \n> When the users make typoes (e.g. /foo//bar) you can accept the only sane\n> correction (e.g. foo/bar) instead of rejecting, since the only thing the\n> user can do after getting such a rejection is to correct it to that\n> corrected name (e.g. foo/bar) himself and re-issue the command anyway.\n> You can push the \"hassle\" down to the user, or you can fix the tool to\n> remove the hassle from the user.\n\nYes, making check-ref-format stricter without changing its users to\nmassage their input would be a regression.\n\nThe problem Jens described is a git-gui bug. In lib/branch_delete.tcl,\nline 57, the list of deletable branches is populated as follows:\n\n\tforeach h [load_all_heads] {\n\t\tif {$h ne $current_branch} {\n\t\t\t$w_heads insert end $h\n\t\t}\n\t}\n\nSince slashes coalesce, a user-supplied new branch name is not canonical\nand checking \"$h eq $current_branch\" does not actually check if they are\nthe same branch. git-gui should be using the branch name as output by\n\"git check-ref-format --branch\" after the branch is created.\n\nBut what about other scripts that assume each branch has only one\npossible name? Maybe they could be forced to fix up the name early on\nby making check-ref-format reject names with \"//\" in them and providing\na \"git check-ref-format --print\" to help.\n\nUpside: scripts would complain loudly instead of failing subtly on input\nwith extra slashes if they forget to use \"git check-ref-format --print\"\nwhen appropriate. Downside: users, including the 22 callers of\ncheck_ref_format() in git, would have to be checked and probably changed\nto avoid regressions.\n\nNever having made that typo, I find it hard to consider such a change\nworth the trouble, but it sounds doable.\n\nRegards,\nJonathan\n"},{"id":"124653","messageId":"20091012114700.6117@nanako3.lavabit.com","threadId":"21185","inReplyTo":"7vbpkdwyo2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] disallow refs containing successive slashes","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-10-12T02:47:00Z","receivedAt":"2009-10-12T02:47:00Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano writes:\n\n> Jens Lehmann writes:\n>> Yes, one solution could be to fix every application handling branch, tag\n>> or repo names to mimic the namechange done in the bowels of git. But i\n>> think it is not worth the hassle.\n> Besides, by rejecting what we used to accept you are breaking people's\n> expectations. So I am moderately negative, unless you can say your \"every\n> application\" is literally _tons_.\n\nIsn't 1.7.0 a good release to break such people's expectations?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"124664","messageId":"20091012052536.GA11106@progeny.tock","threadId":"21185","inReplyTo":"7vws327wbp.fsf@alter.siamese.dyndns.org","subject":"[PATCH/RFC 0/4] plumbing to help fix git-gui","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:25:36Z","receivedAt":"2009-10-12T05:25:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jens Lehmann writes:\n\n>> I became aware of this issue while looking into problems occuring\n>> when a user created a branch starting with a '/' in git gui (e.g.\n>> \"/foo\"). Strange things happen, while git gui shows the current\n>> branch as \"/foo\" under the hood a branch \"foo\" (without the slash)\n>> had been created. But then you can't delete \"/foo\" from git gui,\n>> because a branch of that name doesn't exist.\n> \n> Perhaps an interface to give a cleaned-up version, e.g.\n> \n> $ git check-ref-format --print refs/heads//foo/bar\n> refs/heads/foo/bar\n> \n> is what you want in order to fix git-gui? I dunno.\n\nThe following packages do exactly that. I tried to use it to fix\ngit gui, but my Tcl-fu was not up to it, at least for tonight.\n\nIt is not obvious to me that this interface is the one that would be\nmost helpful for git gui: probably a command to just munge the branch\nname would be more convenient, though less general.\n\nIs the --print option useful? Right now, it seems that its main use is\ndocumentation. But in the future, it would be very nice if this command\ncould be used to transform Unicode ref names to some appropriate\nnormalization form, to make Unicode ref names usable in Mac OS X and\nless confusing everywhere.\n\nI look forward to your thoughts.\n\nJonathan Nieder (4):\n Add tests for git check-ref-format\n Documentation: describe check-ref-format --branch in more detail\n check-ref-format: add option to print canonical name\n check-ref-format: Simplify --print implementation\n\n Documentation/git-check-ref-format.txt | 32 ++++++++++++----\n builtin-check-ref-format.c | 31 ++++++++++++++++\n t/t1402-check-ref-format.sh | 61 ++++++++++++++++++++++++++++++++\n 3 files changed, 116 insertions(+), 8 deletions(-)\n create mode 100644 t/t1402-check-ref-format.sh\n"},{"id":"124666","messageId":"20091012052704.GB11106@progeny.tock","threadId":"21185","inReplyTo":"20091012052536.GA11106@progeny.tock","subject":"[PATCH 1/4] Add tests for git check-ref-format","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:27:04Z","receivedAt":"2009-10-12T05:27:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The \"git check-ref-format\" command is a basic command various\nporcelains rely on. Test its functionality to make sure it does\nnot unintentionally change.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t1402-check-ref-format.sh | 44 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 44 insertions(+), 0 deletions(-)\n create mode 100644 t/t1402-check-ref-format.sh\n\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nnew file mode 100644\nindex 0000000..382bc6e\n--- /dev/null\n+++ b/t/t1402-check-ref-format.sh\n@@ -0,0 +1,44 @@\n+#!/bin/sh\n+\n+test_description='Test git check-ref-format'\n+\n+. ./test-lib.sh\n+\n+valid_ref() {\n+\ttest_expect_success \"ref name '$1' is valid\" \\\n+\t\t\"git check-ref-format '$1'\"\n+}\n+invalid_ref() {\n+\ttest_expect_success \"ref name '$1' is not valid\" \\\n+\t\t\"test_must_fail git check-ref-format '$1'\"\n+}\n+\n+valid_ref 'heads/foo'\n+invalid_ref 'foo'\n+valid_ref 'foo/bar/baz'\n+valid_ref 'refs///heads/foo'\n+invalid_ref 'heads/foo/'\n+invalid_ref './foo'\n+invalid_ref '.refs/foo'\n+invalid_ref 'heads/foo..bar'\n+invalid_ref 'heads/foo?bar'\n+valid_ref 'foo./bar'\n+invalid_ref 'heads/foo.lock'\n+valid_ref 'heads/foo@bar'\n+invalid_ref 'heads/v@{ation'\n+invalid_ref 'heads/foo\\bar'\n+\n+test_expect_success \"check-ref-format --branch @{-1}\" '\n+\tT=$(git write-tree) &&\n+\tsha1=$(echo A | git commit-tree $T) &&\n+\tgit update-ref refs/heads/master $sha1 &&\n+\tgit update-ref refs/remotes/origin/master $sha1\n+\tgit checkout master &&\n+\tgit checkout origin/master &&\n+\tgit checkout master &&\n+\trefname=$(git check-ref-format --branch @{-1}) &&\n+\ttest \"$refname\" = \"$sha1\" &&\n+\trefname2=$(git check-ref-format --branch @{-2}) &&\n+\ttest \"$refname2\" = master'\n+\n+test_done\n-- \n1.6.5.rc1.199.g596ec\n"},{"id":"124665","messageId":"20091012052823.GC11106@progeny.tock","threadId":"21185","inReplyTo":"20091012052536.GA11106@progeny.tock","subject":"[PATCH 2/4] Documentation: describe check-ref-format --branch","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:28:23Z","receivedAt":"2009-10-12T05:28:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Unless one already knew, it was not obvious what sort of shorthand\n\"git check-ref-format --branch\" expands. Explain it.\n\nThe --branch argument is not optional.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-check-ref-format.txt | 9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex 0b7982e..e9b3b40 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git check-ref-format' \n-'git check-ref-format' [--branch] \n+'git check-ref-format' --branch \n \n DESCRIPTION\n -----------\n@@ -63,8 +63,11 @@ reference name expressions (see linkgit:git-rev-parse[1]):\n \n . at-open-brace `@{` is used as a notation to access a reflog entry.\n \n-With the `--branch` option, it expands a branch name shorthand and\n-prints the name of the branch the shorthand refers to.\n+With the `--branch` option, it expands the ``previous branch syntax''\n+`@{-n}`. For example, `@{-1}` is a way to refer the last branch you\n+were on. This option should be used by porcelains to accept this\n+syntax anywhere a branch name is expected, so they can act as if you\n+typed the branch name.\n \n EXAMPLE\n -------\n-- \n1.6.5.rc1.199.g596ec\n"},{"id":"124667","messageId":"20091012053141.GD11106@progeny.tock","threadId":"21185","inReplyTo":"20091012052536.GA11106@progeny.tock","subject":"[PATCH/RFC 3/4] git check-ref-format --print","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:31:42Z","receivedAt":"2009-10-12T05:31:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tolerating empty path components in ref names means each ref does\nnot have a unique name. This creates difficulty for porcelains\nthat want to see if two branches are equal. Add a helper associating\nto each ref a canonical name.\n\nIf a user asks a porcelain to create a ref \"refs/heads//master\",\nthe porcelain can run \"git check-ref-format --print refs/heads//master\"\nand only deal with \"refs/heads/master\" from then on.\n\nIn the future, it would be very nice if this command could be\nmodified to transform Unicode ref names to some appropriate\nnormalization form, to make Unicode ref names usable in Mac OS X,\ntoo, and less confusing everywhere.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-check-ref-format.txt | 25 +++++++++++++++++++------\n builtin-check-ref-format.c | 10 ++++++++++\n t/t1402-check-ref-format.sh | 17 +++++++++++++++++\n 3 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex e9b3b40..0aeef24 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -9,6 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git check-ref-format' \n+'git check-ref-format' --print \n 'git check-ref-format' --branch \n \n DESCRIPTION\n@@ -63,19 +64,31 @@ reference name expressions (see linkgit:git-rev-parse[1]):\n \n . at-open-brace `@{` is used as a notation to access a reflog entry.\n \n+With the `--print` option, if 'refname' is acceptable, it prints the\n+canonicalized name of a hypothetical reference with that name. That is,\n+it prints 'refname' with any extra `/` characters removed.\n+\n With the `--branch` option, it expands the ``previous branch syntax''\n `@{-n}`. For example, `@{-1}` is a way to refer the last branch you\n were on. This option should be used by porcelains to accept this\n syntax anywhere a branch name is expected, so they can act as if you\n typed the branch name.\n \n-EXAMPLE\n--------\n-\n-git check-ref-format --branch @{-1}::\n-\n-Print the name of the previous branch.\n+EXAMPLES\n+--------\n \n+* Print the name of the previous branch:\n++\n+------------\n+$ git check-ref-format --branch @{-1}\n+------------\n+\n+* Determine the reference name to use for a new branch:\n++\n+------------\n+$ ref=$(git check-ref-format --print \"refs/heads/$newbranch\") ||\n+die \"we do not like '$newbranch' as a branch name.\"\n+------------\n \n GIT\n ---\ndiff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c\nindex f9381e0..b97b61a 100644\n--- a/builtin-check-ref-format.c\n+++ b/builtin-check-ref-format.c\n@@ -17,6 +17,16 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"%s\\n\", sb.buf + 11);\n \t\texit(0);\n \t}\n+\tif (argc == 3 && !strcmp(argv[1], \"--print\")) {\n+\t\tchar *refname = xmalloc(strlen(argv[2]) + 1);\n+\n+\t\tif (check_ref_format(argv[2]))\n+\t\t\texit(1);\n+\t\tif (normalize_path_copy(refname, argv[2]))\n+\t\t\tdie(\"Could not normalize ref name '%s'\", argv[2]);\n+\t\tprintf(\"%s\\n\", refname);\n+\t\texit(0);\n+\t}\n \tif (argc != 2)\n \t\tusage(\"git check-ref-format refname\");\n \treturn !!check_ref_format(argv[1]);\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 382bc6e..eb45afb 100644\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -41,4 +41,21 @@ test_expect_success \"check-ref-format --branch @{-1}\" '\n \trefname2=$(git check-ref-format --branch @{-2}) &&\n \ttest \"$refname2\" = master'\n \n+valid_ref_normalized() {\n+\ttest_expect_success \"ref name '$1' simplifies to '$2'\" \"\n+\t\trefname=\\$(git check-ref-format --print '$1') &&\n+\t\ttest \\\"\\$refname\\\" = '$2'\"\n+}\n+invalid_ref_normalized() {\n+\ttest_expect_success \"check-ref-format --print rejects '$1'\" \"\n+\t\ttest_must_fail git check-ref-format --print '$1'\"\n+}\n+\n+valid_ref_normalized 'heads/foo' 'heads/foo'\n+valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'\n+invalid_ref_normalized 'foo'\n+invalid_ref_normalized 'heads/foo/../bar'\n+invalid_ref_normalized 'heads/./foo'\n+invalid_ref_normalized 'heads\\foo'\n+\n test_done\n-- \n1.6.5.rc1.199.g596ec\n"},{"id":"124668","messageId":"20091012053301.GE11106@progeny.tock","threadId":"21185","inReplyTo":"20091012052536.GA11106@progeny.tock","subject":"[PATCH/RFC 4/4] check-ref-format: simplify --print implementation","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:33:01Z","receivedAt":"2009-10-12T05:33:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"normalize_path_copy() is a complicated function, but most of its\nfunctionality will never apply to a ref name that has been checked\nwith check_ref_format().\n\nSigned-off-by: Jonathan Nieder \n---\n builtin-check-ref-format.c | 25 +++++++++++++++++++++++--\n 1 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c\nindex b97b61a..e439136 100644\n--- a/builtin-check-ref-format.c\n+++ b/builtin-check-ref-format.c\n@@ -7,6 +7,28 @@\n #include \"builtin.h\"\n #include \"strbuf.h\"\n \n+/*\n+ * Replace each run of adjacent slashes in src with a single slash,\n+ * and write the result to dst.\n+ *\n+ * This function is similar to normalize_path_copy(), but stripped down\n+ * to meet check_ref_format's simpler needs.\n+ */\n+static void collapse_slashes(char *dst, const char *src)\n+{\n+\tchar ch;\n+\tchar prev = '\\0';\n+\n+\twhile (ch = *src++) {\n+\t\tif (prev == '/' && ch == prev)\n+\t\t\tcontinue;\n+\n+\t\t*dst++ = ch;\n+\t\tprev = ch;\n+\t}\n+\t*dst = '\\0';\n+}\n+\n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"--branch\")) {\n@@ -22,8 +44,7 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \n \t\tif (check_ref_format(argv[2]))\n \t\t\texit(1);\n-\t\tif (normalize_path_copy(refname, argv[2]))\n-\t\t\tdie(\"Could not normalize ref name '%s'\", argv[2]);\n+\t\tcollapse_slashes(refname, argv[2]);\n \t\tprintf(\"%s\\n\", refname);\n \t\texit(0);\n \t}\n-- \n1.6.5.rc1.199.g596ec\n"},{"id":"124670","messageId":"20091012054546.GF11106@progeny.tock","threadId":"21185","inReplyTo":"20091012052536.GA11106@progeny.tock","subject":"Re: [PATCH/RFC 0/4] plumbing to help fix git-gui","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-12T05:45:47Z","receivedAt":"2009-10-12T05:45:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Junio C Hamano wrote:\n\n>> Perhaps an interface to give a cleaned-up version, e.g.\n>> \n>> $ git check-ref-format --print refs/heads//foo/bar\n>> refs/heads/foo/bar\n>> \n>> is what you want in order to fix git-gui? I dunno.\n> \n> The following packages do exactly that.\n\ns/packages/patches/\n\nI better sleep...\n\nSorry for the confusion,\nJonathan\n"},{"id":"124708","messageId":"20091012143922.GL9261@spearce.org","threadId":"21185","inReplyTo":"20091012053141.GD11106@progeny.tock","subject":"Re: [PATCH/RFC 3/4] git check-ref-format --print","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-10-12T14:39:22Z","receivedAt":"2009-10-12T14:39:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jonathan Nieder wrote:\n> +valid_ref_normalized 'heads/foo' 'heads/foo'\n> +valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'\n> +invalid_ref_normalized 'foo'\n> +invalid_ref_normalized 'heads/foo/../bar'\n> +invalid_ref_normalized 'heads/./foo'\n> +invalid_ref_normalized 'heads\\foo'\n\nWhat about '/refs/heads/foo'? Shouldn't that drop the leading /?\n\nI actually had someone enter that into Gerrit Code Review once,\nexposing a bug I have yet to fix that permits that as a valid\nbranch name. *sigh*\n\nFWIW, I think this is heading in the right direction. Rather\nthan teaching the UIs how clean up a name, give us a tool to\ndo the normalization and validation, and let us call it when\nwe get user input.\n\n-- \nShawn.\n"},{"id":"124753","messageId":"7veip8e302.fsf@alter.siamese.dyndns.org","threadId":"21185","inReplyTo":"20091012143922.GL9261@spearce.org","subject":"Re: [PATCH/RFC 3/4] git check-ref-format --print","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-12T21:06:21Z","receivedAt":"2009-10-12T21:06:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Jonathan Nieder wrote:\n>> +valid_ref_normalized 'heads/foo' 'heads/foo'\n>> +valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'\n>> +invalid_ref_normalized 'foo'\n>> +invalid_ref_normalized 'heads/foo/../bar'\n>> +invalid_ref_normalized 'heads/./foo'\n>> +invalid_ref_normalized 'heads\\foo'\n>\n> What about '/refs/heads/foo'? Shouldn't that drop the leading /?\n>\n> I actually had someone enter that into Gerrit Code Review once,\n> exposing a bug I have yet to fix that permits that as a valid\n> branch name. *sigh*\n>\n> FWIW, I think this is heading in the right direction. Rather\n> than teaching the UIs how clean up a name, give us a tool to\n> do the normalization and validation, and let us call it when\n> we get user input.\n\nI understand that you prefer the latter between \"there is no tool; the\ncaller is responsibile to make sure it feeds us canonical representation\"\nand \"there is a tool that makes a slightly malformed string into canonical\nform for the callers to use before calling us.\" And that would be my\npreference between these two as well.\n\nBut that is based on the current behaviour of accepting slightly malformed\nand silently making it canonical. If we throw a third alternative,\nJonathan's original patch, that did \"we reject malformed string\", in the\nmix, what would be your preference?\n\nI moderately favour the \"tool to canonicalize is given, and it would be a\nbug for the caller not to use it\" approach this series takes primarily\nbecause that approach won't break scripts that do something like this to\nmake a new branch, optionally grouped by the owner (e.g. 'sp/smart-http'\nor 'next'):\n\n\towner=$1 topic=$2\n\tbranch=$owner/$topic\n git branch \"$branch\"\n\nThis currently works as expected as long as it does not later try to\ncompare for-each-ref output with \"refs/heads/$branch\" and expects a match.\nWith the third approach, the optional username grouping becomes mandatory\nfor such a script, as 'git branch' will error out. To keep the grouping\nstill optinal, such a script needs to be written perhaps like:\n\n\tif test -z \"$2\"\n then\n\t\tbranch=\"$1\"\n\telse\n \tbranch=\"$1/$2\"\n\tfi\n\nand that would be a hassle for the scripters, but this _could_ be a kind\nof backward incompatible tightening we might want to consider for 1.7.0,\nas somebody suggested earlier.\n\nBut now I have spelled this out, I do not see much upside for rejecting,\nand more importantly, I think it would be an independent issue. We can\nreject or just keep normalizing silently, and a tool to show the\nnormalized name would be useful and necessary regardless of that.\n"},{"id":"124775","messageId":"20091012232653.GQ9261@spearce.org","threadId":"21185","inReplyTo":"7veip8e302.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 3/4] git check-ref-format --print","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-10-12T23:26:54Z","receivedAt":"2009-10-12T23:26:54Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> I understand that you prefer the latter between \"there is no tool; the\n> caller is responsibile to make sure it feeds us canonical representation\"\n> and \"there is a tool that makes a slightly malformed string into canonical\n> form for the callers to use before calling us.\" And that would be my\n> preference between these two as well.\n...\n> But now I have spelled this out, I do not see much upside for rejecting,\n> and more importantly, I think it would be an independent issue. We can\n> reject or just keep normalizing silently, and a tool to show the\n> normalized name would be useful and necessary regardless of that.\n\nI agree with the last paragraph here, we shouldn't reject, but\ninstead keep the current state but encourage tools to use the new\ncanonical print tool to clean up a name if they want to hang onto the\nstring the user entered and it needs to exactly match for-each-ref\nsort of output.\n\n-- \nShawn.\n"},{"id":"124776","messageId":"7vococ6v73.fsf@alter.siamese.dyndns.org","threadId":"21185","inReplyTo":"20091012053141.GD11106@progeny.tock","subject":"Re: [PATCH/RFC 3/4] git check-ref-format --print","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-12T23:36:48Z","receivedAt":"2009-10-12T23:36:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> In the future, it would be very nice if this command could be\n> modified to transform Unicode ref names to some appropriate\n> normalization form, to make Unicode ref names usable in Mac OS X,\n> too, and less confusing everywhere.\n\nI do not disagree with a desire to help fixing the unicode insanity on\nthat platform, but I suspect that check-ref-format is a wrong place to\ntackle the issue. You would need a similar filter for outputs from the\nlikes of ls-files and \"diff --name-only\", iow, anything that deal with\npathnames, no?\n\nIt would have be something like \"check-ref-format --print | iconv ...\"\npipeline (conceptually, if not forcing the pipeline to the end users, that\nis).\n\nAlso are people happy with \"--print\"? I was waiting for others to reword\nit to \"--normalize\" or something like that. In your documentation you\nexplain it to \"canonicalize\", and in your tests you name the output\n\"normalized\". I am fine with either wording, but would like to see us\nusing only one, not both.\n\nThanks.\n"},{"id":"124810","messageId":"20091013044929.GA30342@progeny.tock","threadId":"21185","inReplyTo":"7vococ6v73.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 3/4] git check-ref-format --print","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-13T04:49:30Z","receivedAt":"2009-10-13T04:49:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I do not disagree with a desire to help fixing the unicode insanity on\n> that platform, but I suspect that check-ref-format is a wrong place to\n> tackle the issue. You would need a similar filter for outputs from the\n> likes of ls-files and \"diff --name-only\", iow, anything that deal with\n> pathnames, no?\n> \n> It would have be something like \"check-ref-format --print | iconv ...\"\n> pipeline (conceptually, if not forcing the pipeline to the end users, that\n> is).\n\nGNU iconv does not write the various Unicode normalization forms, so it\nwould have to be something like \"check-ref-format --print | charlint ...\"\ninstead. Regardless of the filesystem, it seems reasonable to consider é\n(U+00e9) and é (U+0065 + U+0301) the same character when it appears in a\nref name, and one way to achieve this would be to pick one normalization\nform and stick to it. This does not seem so different from stripping out\nempty path components.\n\nAs a side effect, that would deal with OS X’s strange handling of unicode\nfilenames for .git/refs/*. Now that I think about it, if fighting OS X\nwere the only problem that needed to be solved, I don’t think I’d like\nthis solution so much. The analogous solution to the also unsolved issue\nof case insensitive filesystems is to force all ref names to lowercase.\nDo we want to do that? (The case insensitivity issue might not be as bad,\nsince the relevant filesystems will at least _preserve_ the case of\nfilenames in .git/refs. Should we copy them and smudge new ref names to\nmatch known ones that differ only in case? Just thinking about the\nproblem makes me cringe.)\n\nCoping with the unicode filename craziness in the working tree is a\nseparate issue, though probably a more important one. I think Linus set\nup a framework for solving it in\n.\n\nRegards,\nJonathan\n"}]}