{"thread":{"id":"38289","subject":"[PATCH v2 1/2] notes: accept any ref","startedAt":"2015-01-06T08:03:51Z","lastAt":"2015-01-08T10:31:58Z","messageCount":16,"participants":["Kyle J. McKay","Junio C Hamano","Johan Herland","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"254319","messageId":"d4509363c8f670483dacdd2a5070f5a@74d39fa044aa309eaea14b9f57fe79c","threadId":"38289","inReplyTo":null,"subject":"[PATCH v2 0/2] Accept any notes ref","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T08:03:51Z","receivedAt":"2015-01-06T08:03:51Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"This is a resend of the original patch by Scott Chacon combined with the\nsuggested patch by Johan Herland with a summary of the discussion so far\nin the hope to restart discussion about including this change.\n\nThe original thread can be found at [1].\n\nThe history so far:\n\nThe Patch 1/2 was originally submitted to allow using refs outside of\nthe refs/notes/ namespace as notes refs for the purpose of merging notes.\nHowever, that patch actually just relaxes the restriction on notes refs so\nthat a fully-qualified ref will be used as-is in the notes code and affects\nmore than just notes merging.\n\nThe concern was then raised that this is a \"stealth-enabler\" patch and that\neven if notes refs are allowed outside of refs/notes/ then they should still\nbe restricted to a some subset (to be determined) of the the refs/ hierarchy\nand not just allowed anywhere.\n\nBut, I feel that the rest of Git does not restrict the user in this way -- if\nthe user really wants to do something that is \"not recommended\" there is\nalmost always a mechanism and/or option to leave the user in control and allow\nit despite any recommendation(s) to the contrary.\n\nSo I am resending this summary with attached patches (both of which are\nvery trivial) to allow using any ref for notes as long as it's fully qualified\n(i.e. starts with \"refs/\"). Patch 1/2 does that and patch 2/2 fixes the test\nthat breaks because of it.\n\nIn the hope of restarting discussion towards enabling use of refs outside of\nrefs/notes/ with the notes machinery I conclude by including Peff's final\nreply to the original thread which I think contains the most compelling\naregument for inclusion:\n\nOn Dec 4, 2014, at 02:26, Jeff King wrote:\n\n> On Sat, Nov 22, 2014 at 10:04:57AM -0800, Kyle J. McKay wrote:\n>\n>> On Sep 19, 2014, at 11:22, Junio C Hamano wrote:\n>>\n>>> By \"stealth enabler\" I mean the removal of refs/notes/ restriction\n>>> that was originally done as a safety measure to avoid mistakes of\n>>> storing notes outside. The refs/remote-notes/ future direction\n>>> declares that it is no longer a mistake to store notes outside\n>>> refs/notes/, but that does not necessarily have to mean that\n>>> anywhere under refs/ is fine. It may make more sense to be explicit\n>>> with the code touched here to allow traditional refs/notes/ and the\n>>> new hierarchy only. That way, we will still keep the \"avoid\n>>> mistakes\" safety and enable the new layout at the same time.\n>>\n>> This is the part where I want to lobby for inclusion of this change.\n>> I do not believe it is consistent with existing Git practice to\n>> enforce restrictions like this (you can only display/edit/etc. notes\n>> under refs/notes/*).\n>\n> Yeah, this is the compelling part to me. There is literally no way to\n> utilize the notes codes for any ref outside of refs/notes currently.\n> We don't know if refs/remote-notes is the future, or refs/remotes/\n> origin/notes, or something else. But we can't even experiment with it in a\n> meaningful way because the plumbing layer is so restrictive.\n>\n> The notes feature has stagnated. Not many people use it because it \n> requires so much magic to set up and share notes. I think it makes sense to \n> remove a safety feature that is making it harder to experiment with. If the \n> worst case is that people start actually _using_ notes and we get \n> proliferation of places that people are sticking them in the refs hierarchy,\n> that is vastly preferable IMHO to the current state, in which few people use\n> them and there is little support for sharing them at all.\n\n-Kyle\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/257281\n\nKyle J. McKay (1):\n t/t3308-notes-merge.sh: succeed with relaxed notes refs\n\nScott Chacon (1):\n notes: accept any ref\n\n notes.c | 2 +-\n t/t3308-notes-merge.sh | 1 -\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\n-- \n2.1.4\n"},{"id":"254317","messageId":"4066d4aa9a52b58b40b3abed21eaf35@74d39fa044aa309eaea14b9f57fe79c","threadId":"38289","inReplyTo":"d4509363c8f670483dacdd2a5070f5a@74d39fa044aa309eaea14b9f57fe79c","subject":"[PATCH v2 1/2] notes: accept any ref","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T08:03:52Z","receivedAt":"2015-01-06T08:03:52Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"From: Scott Chacon \n\nCurrently if you try to merge notes, the notes code ensures that the\nreference is under the 'refs/notes' namespace. In order to do any sort\nof collaborative workflow, this doesn't work well as you can't easily\nhave local notes refs seperate from remote notes refs.\n\nThis patch changes the expand_notes_ref function to check for simply a\nleading refs/ instead of refs/notes to check if we're being passed an\nexpanded notes reference. This would allow us to set up\nrefs/remotes-notes or otherwise keep mergeable notes references outside\nof what would be contained in the notes push refspec.\n\nSigned-off-by: Scott Chacon \n---\n notes.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/notes.c b/notes.c\nindex c763a21e..7165a33f 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1292,7 +1292,7 @@ int copy_note(struct notes_tree *t,\n \n void expand_notes_ref(struct strbuf *sb)\n {\n-\tif (starts_with(sb->buf, \"refs/notes/\"))\n+\tif (starts_with(sb->buf, \"refs/\"))\n \t\treturn; /* we're happy */\n \telse if (starts_with(sb->buf, \"notes/\"))\n \t\tstrbuf_insert(sb, 0, \"refs/\", 5);\n-- \n2.1.4\n"},{"id":"254318","messageId":"e514c72c481c66f9808f1dd09117a16@74d39fa044aa309eaea14b9f57fe79c","threadId":"38289","inReplyTo":"d4509363c8f670483dacdd2a5070f5a@74d39fa044aa309eaea14b9f57fe79c","subject":"[PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T08:03:53Z","receivedAt":"2015-01-06T08:03:53Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"With the recent change to allow notes refs to be located in\nthe refs hierarchy in locations other than refs/notes/ the\n'git notes merge refs/heads/master' test started succeeding.\n\nPreviously refs/heads/master would have been expanded to\na non-existing, ref refs/notes/refs/heads/master, and the\nmerge would have failed (as expected).\n\nNow, however, since refs/heads/master exists and the new,\nmore relaxed notes refs rules leave it unchanged, the merge\nsucceeds. This has a follow-on effect which makes the\nnext two tests fail as well.\n\nThe refs/heads/master ref could just be replaced with\nanother ref name that does not exist such as refs/heads/xmaster,\nbut there are already several tests using non-existant refs\nso instead just remove the refs/heads/master line.\n\nSuggested-by: Johan Herland \nSigned-off-by: Kyle J. McKay \n---\n t/t3308-notes-merge.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh\nindex 24d82b49..f0feb64b 100755\n--- a/t/t3308-notes-merge.sh\n+++ b/t/t3308-notes-merge.sh\n@@ -90,7 +90,6 @@ test_expect_success 'fail to merge various non-note-trees' '\n \ttest_must_fail git notes merge refs/notes/ &&\n \ttest_must_fail git notes merge refs/notes/dir &&\n \ttest_must_fail git notes merge refs/notes/dir/ &&\n-\ttest_must_fail git notes merge refs/heads/master &&\n \ttest_must_fail git notes merge x: &&\n \ttest_must_fail git notes merge x:foo &&\n \ttest_must_fail git notes merge foo^{bar\n-- \n2.1.4\n"},{"id":"254321","messageId":"xmqq8uhgqkge.fsf@gitster.dls.corp.google.com","threadId":"38289","inReplyTo":"e514c72c481c66f9808f1dd09117a16@74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T10:20:33Z","receivedAt":"2015-01-06T10:20:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" writes:\n\n> Now, however, since refs/heads/master exists and the new,\n> more relaxed notes refs rules leave it unchanged, the merge\n> succeeds. ...\n> ...\n> diff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh\n> index 24d82b49..f0feb64b 100755\n> --- a/t/t3308-notes-merge.sh\n> +++ b/t/t3308-notes-merge.sh\n> @@ -90,7 +90,6 @@ test_expect_success 'fail to merge various non-note-trees' '\n> \ttest_must_fail git notes merge refs/notes/ &&\n> \ttest_must_fail git notes merge refs/notes/dir &&\n> \ttest_must_fail git notes merge refs/notes/dir/ &&\n> -\ttest_must_fail git notes merge refs/heads/master &&\n> \ttest_must_fail git notes merge x: &&\n> \ttest_must_fail git notes merge x:foo &&\n> \ttest_must_fail git notes merge foo^{bar\n\nThe test title reads \"fail to merge non-note trees\", and I am\nassuming that the tree-ish refs/heads/master (aka 'master' branch)\nrepresents does not look anything like a typical note tree where\npathnames are 40-hex with fan-out.\n\nThe fact that \"git notes merge refs/heads/master\" fails is a very\ngood prevention of end-user mistakes, and this removal of test\ndemonstrates that we are dropping a valuable safety.\n\nArguably, not being able to save notes tree anywhere outside of\nrefs/notes/ hierarchy may be too high a price to pay in order to\nprevent refs/heads/master from being considered (hence to avoid such\nend-user mistakes), but at the same time, losing this safetly may\nalso be too high a price to pay in order to allow people to store\ntheir notes in somewhere outside e.g. refs/remote-notes/origin/foo.\n\"Somewhere outside\" does not mean \"Including other hierarchies like\nrefs/heads and refs/tags that have long established meaning\".\n\nAlthough I am not fundamentally against allowing to store notes\noutside refs/notes/, it is different from \"anywhere is fine\".\nCan't we do this widening in a less damaging way?\n"},{"id":"254331","messageId":"F6072C48-FA50-4F9D-AD26-0B4C4DD64B91@gmail.com","threadId":"38289","inReplyTo":"xmqq8uhgqkge.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T12:27:34Z","receivedAt":"2015-01-06T12:27:34Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 02:20, Junio C Hamano wrote:\n\n> \"Kyle J. McKay\" writes:\n>\n>> Now, however, since refs/heads/master exists and the new,\n>> more relaxed notes refs rules leave it unchanged, the merge\n>> succeeds. ...\n>> ...\n>> diff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh\n>> index 24d82b49..f0feb64b 100755\n>> --- a/t/t3308-notes-merge.sh\n>> +++ b/t/t3308-notes-merge.sh\n>> @@ -90,7 +90,6 @@ test_expect_success 'fail to merge various non- \n>> note-trees' '\n>> \ttest_must_fail git notes merge refs/notes/ &&\n>> \ttest_must_fail git notes merge refs/notes/dir &&\n>> \ttest_must_fail git notes merge refs/notes/dir/ &&\n>> -\ttest_must_fail git notes merge refs/heads/master &&\n>> \ttest_must_fail git notes merge x: &&\n>> \ttest_must_fail git notes merge x:foo &&\n>> \ttest_must_fail git notes merge foo^{bar\n>\n> The test title reads \"fail to merge non-note trees\", and I am\n> assuming that the tree-ish refs/heads/master (aka 'master' branch)\n> represents does not look anything like a typical note tree where\n> pathnames are 40-hex with fan-out.\n\nIn fact it looks like this:\n\n100644 blob 2a5d0158a25a97e8ebf4158d9187acb124da50ea\t1st.t\n100644 blob 3f514b8c0d8e9345bda64de1664eb43d7d38d12a\t2nd.t\n100644 blob e5404b81e697da4f0c99aac167b5e63bcce4b78b\t3rd.t\n100644 blob c950fbad52232390031696035ad79c670ee3bd7b\t4th.t\n100644 blob ba96e617c4d2741ac7693ca7eb20f9dddf4754f6\t5th.t\n\nso you are correct.\n\n> The fact that \"git notes merge refs/heads/master\" fails is a very\n> good prevention of end-user mistakes, and this removal of test\n> demonstrates that we are dropping a valuable safety.\n\nAt the point the dropped line runs, core.notesRef has been set to refs/ \nnotes/y which does not exist.\n\nAll of the tests in the 'fail to merge various non-note-trees' section \nfail with one of these errors:\n\n 1) Failed to resolve remote notes ref ''\n\n 2) Cannot merge empty notes ref () into empty \nnotes ref (refs/notes/y)\n\n 3) error: object 6c99d48c9905deea5d59d723468862362918626a is a \ntree, not a commit\n\nThe 3rd error comes from the \"git notes merge x:\" attempt.\n\nSo despite the name of the test, the actual tree contents do not seem \nto be examined.\n\nWhen the notes ref checking is relaxed to leave refs/heads/master \nalone rather than turning it into refs/notes/refs/heads/master, the \nprevious error (#2 in this case) goes away and since refs/notes/y does \nnot exist, it is simply updated to the value of refs/heads/master \nwithout any checks. Of course that refs/heads/master tree doesn't \nlook like a notes tree.\n\nAnd if we do this:\n\n git update-ref refs/notes/refs/heads/master master\n\nThen \"git notes merge refs/heads/master\" also succeeds without \ncomplaining that the refs/notes/refs/heads/master tree does not look \nlike a notes tree and we didn't need to relax the refs/notes \nrestriction and, as you point out, the name of the test seems to imply \nthat would be rejected.\n\nInterestingly, if we then attempt to merge refs/notes/x into this non- \nnotes refs/notes/y tree, it also succeeds and even keeps the things \nthat do not look like notes. The reverse merge (y into x) succeeds as \nwell, but the non-notes stuff in y is not merged in in that case.\n\n> Arguably, not being able to save notes tree anywhere outside of\n> refs/notes/ hierarchy may be too high a price to pay in order to\n> prevent refs/heads/master from being considered (hence to avoid such\n> end-user mistakes), but at the same time, losing this safetly may\n> also be too high a price to pay in order to allow people to store\n> their notes in somewhere outside e.g. refs/remote-notes/origin/foo.\n> \"Somewhere outside\" does not mean \"Including other hierarchies like\n> refs/heads and refs/tags that have long established meaning\".\n\nIf we relax the refs/notes restriction, putting a notes ref in refs/ \nheads/ doesn't necessarily seem like that's a terrible thing \nas long as it's really a notes tree if used with the notes machinery. \nAIUI, the refs/heads convention only requires the ref to point to the \ntip of a commit chain which all of the refs under refs/notes satisfy. \nThe refs/heads convention AIUI does not impose any requirement about \nthe contents of the tree(s) those commits in the chain refer to. But \nat the same time I can't think of any particular reason I'd want to \nstore notes refs in there either.\n\n> Although I am not fundamentally against allowing to store notes\n> outside refs/notes/, it is different from \"anywhere is fine\".\n> Can't we do this widening in a less damaging way?\n\nWithout arbitrarily restricting where notes can be stored it seems to \nme the only option would be to have the notes machinery actually \ninspect the tree of any existing notes ref it's passed. That would \nalso catch the case where \"git update-ref refs/notes/refs/heads/master \nmaster\" was run as well. It also seems like a good check to have in \nplace to help catch user errors.\n\nI'm not all that familiar with the notes code, perhaps there's already \na function that does the tree check to make sure the tree actually \nlooks like a notes tree that can easily be called? Maybe Johan has \nsome thoughts on this?\n\n-Kyle\n"},{"id":"254352","messageId":"xmqqtx03pxzf.fsf@gitster.dls.corp.google.com","threadId":"38289","inReplyTo":"F6072C48-FA50-4F9D-AD26-0B4C4DD64B91@gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T18:25:56Z","receivedAt":"2015-01-06T18:25:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" writes:\n\n> So despite the name of the test, the actual tree contents do not seem\n> to be examined.\n\nYes, but the thing is, thanks to refs/notes restriction, there is no\nneed to do such examination [*1*].\n\nNote that it is an entirely different matter when you deliberately\nviolate the expectation using plumbing (e.g. update-ref). Users of\nplumbing commands are expected to know what they are doing, so the\nlevel of safety we need to give them can be much lower than Porcelain\ncommands such as 'git notes'. \n\nBut when you stick to Porcelain commands, it is very hard to mix\nrefs/notes/* with refs/heads/* and refs/remotes/* by mistake. You\nhave to really work on it by doing something unusual to have a non\ncommit in refs/heads/*, a commit in refs/notes/*, or a regular\nnon-note commit in refs/notes/*.\n\nOnce you lift the existing restriction, that easy safety goes away,\nso the burden of giving a reasonable safety in some other way falls\non the one who is dropping refs/notes restriction.\n\nThat is exactly what I meant by that the existing safety pays price\nof not being able to store notes outside refs/notes, which may be\ntoo high a price to pay.\n\n>> Although I am not fundamentally against allowing to store notes\n>> outside refs/notes/, it is different from \"anywhere is fine\".\n>> Can't we do this widening in a less damaging way?\n>\n> Without arbitrarily restricting where notes can be stored it seems to\n> me the only option would be to have the notes machinery actually\n> inspect the tree of any existing notes ref it's passed.\n\nAs I said earlier (assuming you read footnotes before you read a new\nparagraph), the ship has already sailed.\n\nObvious two sensible ways forward are to do a blacklist (i.e. allow\nanywhere except for known non-notes namespaces like refs/heads/) or\ndo a whitelist (i.e. allow refs/ in addition to\nrefs/notes) of the namespace, and the latter is vastly preferrable\nthan the former, because you can practically *never* crawl back a\nnamespace once you give it to the general public, even if you later\nfind it a grave error to open it too widely and need a more\ncontrolled access [*2*]. And the name of the game for a software\nthat can be relied for a long haul is to avoid painting ourselves in\na corner that we cannot get out of.\n\nIf we add refs/remote-notes/* to the whitelist now, and if later it\nturns out to be not so ideal and we would prefer another layout for\nremotes, e.g. refs/remotesNew/*/{heads,notes,tags}/, we can add\nrefs/remotesNew/*/notes/ to the whitelist _without_ breaking those\nwho have already started using refs/remote-notes/*. That is why\nI said whitelist is preferrable than blacklist.\n\n\n[Footnote]\n\n*1* I actually do not think a tree examination would help very much\n here. IIRC, somebody decided long time ago that it would be a\n good idea to be able to store a path that is not a fanned-out\n 40-hex in a notes tree and 'git notes merge' would accept such a\n notes-tree. Although I doubt that the resulting notes-tree\n produced by 'notes merge' is carefully designed one (as opposed\n to whatever the implementation happens to do) with sensible\n semantics, people may already be relying on it.\n\n*2* The above 'notes-tree can store non fanned-out 40-hex' is a good\n example why you need to start strict and loosen only when it\n becomes necessary. Despite that even Git itself does not use\n that \"facility\" to do anything useful AFAIK, only because we\n started with a loose variant that allows arbitrary garbage, we\n cannot retroactively tighten the definition of what a notes-tree\n should look like without risking to break practice of other\n people.\n"},{"id":"254380","messageId":"A5A16FEC-D72C-4B21-8BE1-3A41696DFF49@gmail.com","threadId":"38289","inReplyTo":"xmqqtx03pxzf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T23:29:08Z","receivedAt":"2015-01-06T23:29:08Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 10:25, Junio C Hamano wrote:\n\n> \"Kyle J. McKay\" writes:\n>\n>> So despite the name of the test, the actual tree contents do not seem\n>> to be examined.\n>\n> Yes, but the thing is, thanks to refs/notes restriction, there is no\n> need to do such examination [*1*].\n>\n> Note that it is an entirely different matter when you deliberately\n> violate the expectation using plumbing (e.g. update-ref). Users of\n> plumbing commands are expected to know what they are doing, so the\n> level of safety we need to give them can be much lower than Porcelain\n> commands such as 'git notes'.\n>\n> But when you stick to Porcelain commands, it is very hard to mix\n> refs/notes/* with refs/heads/* and refs/remotes/* by mistake. You\n> have to really work on it by doing something unusual to have a non\n> commit in refs/heads/*, a commit in refs/notes/*, or a regular\n> non-note commit in refs/notes/*.\n\nPerhaps that is the crux of the issue. There is no git notes-plumbing \ncommand where the git notes command continues to apply restrictions \nbut the vaporware notes-plumbing command allows anything just like \nupdate-ref does.\n\nI think there are two issues here:\n\n1) There's no simple way to store remote notes refs outside the refs/ \nnotes namespace in such a way that they can be used with git notes \ncommands.\n\n2) People who want to experiment with using git notes storage as a \nbasis for building some new feature are forced to store their refs \nunder the refs/notes namespace even if that does not make sense for \nthe feature when what's stored in the notes tree is not intended to \nprovide any content that is directly meaningful to the user.\n\n> That is exactly what I meant by that the existing safety pays price\n> of not being able to store notes outside refs/notes, which may be\n> too high a price to pay.\n>\n>>> Although I am not fundamentally against allowing to store notes\n>>> outside refs/notes/, it is different from \"anywhere is fine\".\n>>> Can't we do this widening in a less damaging way?\n>>\n>> Without arbitrarily restricting where notes can be stored it seems to\n>> me the only option would be to have the notes machinery actually\n>> inspect the tree of any existing notes ref it's passed.\n>\n> As I said earlier (assuming you read footnotes before you read a new\n> paragraph), the ship has already sailed.\n\nHmpf. So the only possible safety check is refname-based. But, as \nyou say, it's no use crying over spilled milk [3].\n\n> Obvious two sensible ways forward are to do a blacklist (i.e. allow\n> anywhere except for known non-notes namespaces like refs/heads/) or\n> do a whitelist (i.e. allow refs/ in addition to\n> refs/notes) of the namespace, and the latter is vastly preferrable\n> than the former, because you can practically *never* crawl back a\n> namespace once you give it to the general public, even if you later\n> find it a grave error to open it too widely and need a more\n> controlled access [*2*]. And the name of the game for a software\n> that can be relied for a long haul is to avoid painting ourselves in\n> a corner that we cannot get out of.\n>\n> If we add refs/remote-notes/* to the whitelist now, and if later it\n> turns out to be not so ideal and we would prefer another layout for\n> remotes, e.g. refs/remotesNew/*/{heads,notes,tags}/, we can add\n> refs/remotesNew/*/notes/ to the whitelist _without_ breaking those\n> who have already started using refs/remote-notes/*. That is why\n> I said whitelist is preferrable than blacklist.\n\nA whitelist solves issue (1) but is no help for issue (2) unless some \nadditional additional part of the refs namespace were to be also \nwhitelisted. Perhaps something like refs/x-/... in the same \nvein as the various IETF standards for experimental names.\n\n> [Footnote]\n>\n> *1* I actually do not think a tree examination would help very much\n> here. IIRC, somebody decided long time ago that it would be a\n> good idea to be able to store a path that is not a fanned-out\n> 40-hex in a notes tree and 'git notes merge' would accept such a\n> notes-tree. Although I doubt that the resulting notes-tree\n> produced by 'notes merge' is carefully designed one (as opposed\n> to whatever the implementation happens to do) with sensible\n> semantics, people may already be relying on it.\n>\n> *2* The above 'notes-tree can store non fanned-out 40-hex' is a good\n> example why you need to start strict and loosen only when it\n> becomes necessary. Despite that even Git itself does not use\n> that \"facility\" to do anything useful AFAIK, only because we\n> started with a loose variant that allows arbitrary garbage, we\n> cannot retroactively tighten the definition of what a notes-tree\n> should look like without risking to break practice of other\n> people.\n\n\n[3] http://cheezburger.com/6423972864\n\n-Kyle\n"},{"id":"254383","messageId":"xmqqoaqbmpmj.fsf@gitster.dls.corp.google.com","threadId":"38289","inReplyTo":"A5A16FEC-D72C-4B21-8BE1-3A41696DFF49@gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T23:54:44Z","receivedAt":"2015-01-06T23:54:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" writes:\n\n> A whitelist solves issue (1) but is no help for issue (2) unless some\n> additional additional part of the refs namespace were to be also\n> whitelisted. Perhaps something like refs/x-/... in the same\n> vein as the various IETF standards for experimental names.\n\nYour (2) is about people who are _experimenting_, no?\n\nWhy can't they use refs/notes/x-/* while doing so, and\nwhen that matures and proves useful to wider public we can make it\nmore official by whitelisting refs/ in addition to\nrefs/notes/, and the problem is solved, no?\n"},{"id":"254386","messageId":"CALKQrge3yEfmi26HMbckyw_ONRXUWtZGvz=SqqYaL2sqJaHbUg@mail.gmail.com","threadId":"38289","inReplyTo":"A5A16FEC-D72C-4B21-8BE1-3A41696DFF49@gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-01-07T00:28:48Z","receivedAt":"2015-01-07T00:28:48Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jan 7, 2015 at 12:29 AM, Kyle J. McKay wrote:\n> On Jan 6, 2015, at 10:25, Junio C Hamano wrote:\n>> \"Kyle J. McKay\" writes:\n>>> So despite the name of the test, the actual tree contents do not seem\n>>> to be examined.\n>>\n>> Yes, but the thing is, thanks to refs/notes restriction, there is no\n>> need to do such examination [*1*].\n>>\n>> Note that it is an entirely different matter when you deliberately\n>> violate the expectation using plumbing (e.g. update-ref). Users of\n>> plumbing commands are expected to know what they are doing, so the\n>> level of safety we need to give them can be much lower than Porcelain\n>> commands such as 'git notes'.\n>>\n>> But when you stick to Porcelain commands, it is very hard to mix\n>> refs/notes/* with refs/heads/* and refs/remotes/* by mistake. You\n>> have to really work on it by doing something unusual to have a non\n>> commit in refs/heads/*, a commit in refs/notes/*, or a regular\n>> non-note commit in refs/notes/*.\n\nJust injecting my own (probably faulty) memory here: I agree with Junio's\nanalysis (including his footnote *1*). We figured that it was not possible\nto outright forbid non-notes in notes trees (a sufficiently determined user\ncan always use plumbing commands to create arbitrarily non-well-formed\n\"notes trees\"), so instead we implemented the notes plumbing\n(notes.{h,c}) to deal with non-notes somewhat sanely instead (ignoring\nthem in notes queries, and preserving them across notes manipulations).\n\nAs a consequence, the safety features were pushed up into the porcelain\nlayer (builtin/notes.{h,c} and builtin/notes-merge.{h,c}), where they apply\nto ref names instead of actual notes tree contents.\n\n> Perhaps that is the crux of the issue. There is no git notes-plumbing\n> command where the git notes command continues to apply restrictions but the\n> vaporware notes-plumbing command allows anything just like update-ref does.\n\nGood observation. From my POV, as people start creating tools that use\nnotes in a more structured manner (than as free-form plain-text appendages\nto commit messages), the sharp and pointy bits (and bugs) of the interface\ncome into view. This applies to the safety features being discussed in this\nthread, but also IMHO to the other things being currently discussed/fixed\nin the notes code:\n\n - adding empty notes\n - adding notes to non-commits in fast-import\n - suboptimal fanout when mixing fast-import\n and git-notes manipulations of notes trees\n\nSo far I've approached some of these issues by making/suggesting fairly\nmodest fixes/extensions to the notes porcelain, to make it more friendly\nto these users. I haven't considered that we might at some point be\nbetter off splitting out a separate plumbing command for manipulating\nnotes. However, I'm not sure we're there, yet. The problems raised so\nfar do not IMHO warrant the creation of a whole new plumbing command.\nInstead, we can still keep fixing and improving 'git notes'.\n\n> I think there are two issues here:\n>\n> 1) There's no simple way to store remote notes refs outside the refs/notes\n> namespace in such a way that they can be used with git notes commands.\n>\n> 2) People who want to experiment with using git notes storage as a basis for\n> building some new feature are forced to store their refs under the\n> refs/notes namespace even if that does not make sense for the feature when\n> what's stored in the notes tree is not intended to provide any content that\n> is directly meaningful to the user.\n\nAgreed.\n\n>> That is exactly what I meant by that the existing safety pays price\n>> of not being able to store notes outside refs/notes, which may be\n>> too high a price to pay.\n>>\n>>>> Although I am not fundamentally against allowing to store notes\n>>>> outside refs/notes/, it is different from \"anywhere is fine\".\n>>>> Can't we do this widening in a less damaging way?\n>>>\n>>>\n>>> Without arbitrarily restricting where notes can be stored it seems to\n>>> me the only option would be to have the notes machinery actually\n>>> inspect the tree of any existing notes ref it's passed.\n>>\n>>\n>> As I said earlier (assuming you read footnotes before you read a new\n>> paragraph), the ship has already sailed.\n>\n>\n> Hmpf. So the only possible safety check is refname-based. But, as you say,\n> it's no use crying over spilled milk [3].\n>\n>> Obvious two sensible ways forward are to do a blacklist (i.e. allow\n>> anywhere except for known non-notes namespaces like refs/heads/) or\n>> do a whitelist (i.e. allow refs/ in addition to\n>> refs/notes) of the namespace, and the latter is vastly preferrable\n>> than the former, because you can practically *never* crawl back a\n>> namespace once you give it to the general public, even if you later\n>> find it a grave error to open it too widely and need a more\n>> controlled access [*2*]. And the name of the game for a software\n>> that can be relied for a long haul is to avoid painting ourselves in\n>> a corner that we cannot get out of.\n>>\n>> If we add refs/remote-notes/* to the whitelist now, and if later it\n>> turns out to be not so ideal and we would prefer another layout for\n>> remotes, e.g. refs/remotesNew/*/{heads,notes,tags}/, we can add\n>> refs/remotesNew/*/notes/ to the whitelist _without_ breaking those\n>> who have already started using refs/remote-notes/*. That is why\n>> I said whitelist is preferrable than blacklist.\n\nI agree that a whitelist is preferable to a blacklist.\n\n> A whitelist solves issue (1) but is no help for issue (2) unless some\n> additional additional part of the refs namespace were to be also\n> whitelisted. Perhaps something like refs/x-/... in the same vein\n> as the various IETF standards for experimental names.\n\nAlternatively (or additionally), for issue (2), we could add a\n--disable-ref-safety option to 'git notes', to explicitly disable the\nsafety checks for \"experimental\" use.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"254389","messageId":"CALKQrgcAVpcA1MiwC=XYMyXeKTvOYQwe8v4hpfJBEP4+ycic9g@mail.gmail.com","threadId":"38289","inReplyTo":"F6072C48-FA50-4F9D-AD26-0B4C4DD64B91@gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-01-07T01:19:20Z","receivedAt":"2015-01-07T01:19:20Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"(This addresses some smaller specific questions from Kyle, and is not an\nattempt to take focus away from the main direction of this thread)\n\nOn Jan 6, 2015 1:27 PM, \"Kyle J. McKay\" wrote:\n> On Jan 6, 2015, at 02:20, Junio C Hamano wrote:\n\n[...]\n\n> At the point the dropped line runs, core.notesRef has been set to\n> refs/notes/y which does not exist.\n\nMerging into an empty or non-existing notes ref is not in itself an error.\n\n> All of the tests in the 'fail to merge various non-note-trees' section\n> fail with one of these errors:\n>\n> 1) Failed to resolve remote notes ref ''\n>\n> 2) Cannot merge empty notes ref () into empty notes\n> ref (refs/notes/y)\n>\n> 3) error: object 6c99d48c... is a tree, not a commit\n>\n> The 3rd error comes from the \"git notes merge x:\" attempt.\n\nYes. We need an actual commit object which can become the second patent of\nthe notes merge.\n\n> So despite the name of the test, the actual tree contents do not seem to\n> be examined.\n\nCorrect. The test name should probably be improved. I have a bad habit of\nusing the term \"notes tree\" to refer to generic collection of notes, sometimes\nmeaning the notes ref, sometimes the commit referenced by a notes ref, and\nsometimes the actual tree object referenced by the notes commit (or directly\nreferenced by the notes ref). In this case I guess a better name for the test\nwould be: \"fail to merge various things that are not notes commits\"\n\n> When the notes ref checking is relaxed to leave refs/heads/master alone\n> rather than turning it into refs/notes/refs/heads/master, the previous\n> error (#2 in this case) goes away and since refs/notes/y does not exist,\n> it is simply updated to the value of refs/heads/master without any checks.\n\nThis is the expected behaviour of merging an otherwise valid (notes) ref.\n\n> Of course that refs/heads/master tree doesn't look like a notes tree.\n>\n> And if we do this:\n>\n> git update-ref refs/notes/refs/heads/master master\n\n(loading the gun and pointing it at your foot...)\n\n> Then \"git notes merge refs/heads/master\" also succeeds without\n> complaining that the refs/notes/refs/heads/master tree does not look\n> like a notes tree and we didn't need to relax the refs/notes restriction\n> and, as you point out, the name of the test seems to imply that would be\n> rejected.\n\nIn this case, the test name is clearly misleading. As discussed elsewhere\nin the thread, we cannot absolutely enforce a distinction between a \"notes\"\ntree object (i.e. a git tree that contain ONLY notes entries) and a\n\"non-notes\" tree object. There is only a strong convention/expectation that\na notes tree object contains only (or at least mostly) SHA1-named entries\n(possibly with fanout).\n\nThat said, if you stick to the porcelain, you should not be able to create\na notes ref referencing a tree object that contains anything but notes.\n\n> Interestingly, if we then attempt to merge refs/notes/x into this\n> non-notes refs/notes/y tree, it also succeeds and even keeps the things\n> that do not look like notes. The reverse merge (y into x) succeeds as\n> well, but the non-notes stuff in y is not merged in in that case.\n\nThe general policy for handling non-notes in notes code, is to ignore them\nwhen querying, and preserve them when manipulating (t/t3304-notes-mixed.sh\nhas examples). In this case, we preserve the non-notes from the primary\nparent (the notes tree from which the merge was started), and we ignore\nany non-notes coming from subsequent parents. I can't remember if this\nbehaviour was explicitly desired, or if it's just an accident of\nimplementation. However, I will argue that it doesn't make sense for\nnotes-merge to attempt any meaningful merge of non-notes. That is simply\noutside its scope.\n\n>> Arguably, not being able to save notes tree anywhere outside of\n>> refs/notes/ hierarchy may be too high a price to pay in order to\n>> prevent refs/heads/master from being considered (hence to avoid such\n>> end-user mistakes), but at the same time, losing this safetly may\n>> also be too high a price to pay in order to allow people to store\n>> their notes in somewhere outside e.g. refs/remote-notes/origin/foo.\n>> \"Somewhere outside\" does not mean \"Including other hierarchies like\n>> refs/heads and refs/tags that have long established meaning\".\n>\n> If we relax the refs/notes restriction, putting a notes ref in\n> refs/heads/ doesn't necessarily seem like that's a terrible\n> thing as long as it's really a notes tree if used with the notes\n> machinery. AIUI, the refs/heads convention only requires the ref to\n> point to the tip of a commit chain which all of the refs under\n> refs/notes satisfy.\n\nAt some point I remember having a discussion on whether we should\nsupport notes refs pointing directly to a tree objects (to implement\n\"history-less\" notes - sometimes the notes history itself is\nuninteresting, if you're only interested in the current state of the\nnotes tree). However, I can't immediately find any tests exercising\nthis functionality, so it's likely unsupported (the convention here is\ninstead to use a commit object, but make sure to prepare the next notes\ncommit with zero parents; see notes-cache.{h,c} for an example of this.\n\n> The refs/heads convention AIUI does not impose any requirement about\n> the contents of the tree(s) those commits in the chain refer to.\n> But at the same time I can't think of any particular reason I'd want\n> to store notes refs in there either.\n>\n>> Although I am not fundamentally against allowing to store notes\n>> outside refs/notes/, it is different from \"anywhere is fine\".\n>> Can't we do this widening in a less damaging way?\n>\n> Without arbitrarily restricting where notes can be stored it seems to\n> me the only option would be to have the notes machinery actually\n> inspect the tree of any existing notes ref it's passed. That would\n> also catch the case where\n> \"git update-ref refs/notes/refs/heads/master master\" was run as well.\n> It also seems like a good check to have in place to help catch user\n> errors.\n>\n> I'm not all that familiar with the notes code, perhaps there's already\n> a function that does the tree check to make sure the tree actually\n> looks like a notes tree that can easily be called? Maybe Johan has\n> some thoughts on this?\n\nNope, no such check. However, it's worth considering giving the user a\nwarning (maybe even - optionally - an error) when non-notes are found\nin a notes tree. Search for non_note in notes.c to find relevant code.\n\n\nHave fun! :)\n\n...Johan\n"},{"id":"254390","messageId":"20150107011958.GA3536@peff.net","threadId":"38289","inReplyTo":"xmqq8uhgqkge.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-01-07T01:19:58Z","receivedAt":"2015-01-07T01:19:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 06, 2015 at 02:20:33AM -0800, Junio C Hamano wrote:\n\n> The fact that \"git notes merge refs/heads/master\" fails is a very\n> good prevention of end-user mistakes, and this removal of test\n> demonstrates that we are dropping a valuable safety.\n\nIs it really that valuable? If it were:\n\n git notes merge master\n\nI could see somebody running that accidentally. But we are talking about\nsomebody who is already fully-qualifying a ref (and anything unqualified\ncontinues to get looked up under refs/notes). Do people really go to the\nlength of qualifying the ref and then get confused or upset that git did\nexactly what they asked it to do?\n\nI'm worried that the end-user safety here is really a strawman, and we\nare making this more complicated than it needs to be.\n\n-Peff\n"},{"id":"254394","messageId":"27162E6E-742C-455D-A005-220B7A8B38CF@gmail.com","threadId":"38289","inReplyTo":"xmqqoaqbmpmj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-07T01:27:03Z","receivedAt":"2015-01-07T01:27:03Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 15:54, Junio C Hamano wrote:\n\n> \"Kyle J. McKay\" writes:\n>\n>> A whitelist solves issue (1) but is no help for issue (2) unless some\n>> additional additional part of the refs namespace were to be also\n>> whitelisted. Perhaps something like refs/x-/... in the \n>> same\n>> vein as the various IETF standards for experimental names.\n>\n> Your (2) is about people who are _experimenting_, no?\n>\n> Why can't they use refs/notes/x-/* while doing so, and\n> when that matures and proves useful to wider public we can make it\n> more official by whitelisting refs/ in addition to\n> refs/notes/, and the problem is solved, no?\n\nI supposed if that's the best that can be done. The problem is then \nthat if you set notes.displayRef to refs/notes/* to see all relevant \nnotes, don't you end up seeing these experimental, not-meant-for-end- \nuser-consumption notes if they are somewhere under refs/notes/ ? My \nglob specs are a little fuzzy, maybe it's refs/notes/** that gets \neverything under it?\n"},{"id":"254395","messageId":"74EB0B94-0FD1-4AD2-9493-20DB02FC5071@gmail.com","threadId":"38289","inReplyTo":"CALKQrge3yEfmi26HMbckyw_ONRXUWtZGvz=SqqYaL2sqJaHbUg@mail.gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-07T01:51:15Z","receivedAt":"2015-01-07T01:51:15Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 16:28, Johan Herland wrote:\n\n> On Wed, Jan 7, 2015 at 12:29 AM, Kyle J. McKay \n> wrote:\n>> Perhaps that is the crux of the issue. There is no git notes- \n>> plumbing\n>> command where the git notes command continues to apply restrictions \n>> but the\n>> vaporware notes-plumbing command allows anything just like update- \n>> ref does.\n>\n> Good observation. From my POV, as people start creating tools that use\n> notes in a more structured manner (than as free-form plain-text \n> appendages\n> to commit messages), the sharp and pointy bits (and bugs) of the \n> interface\n> come into view. This applies to the safety features being discussed \n> in this\n> thread, but also IMHO to the other things being currently discussed/ \n> fixed\n> in the notes code:\n> [...]\n> I haven't considered that we might at some point be\n> better off splitting out a separate plumbing command for manipulating\n> notes. However, I'm not sure we're there, yet. The problems raised so\n> far do not IMHO warrant the creation of a whole new plumbing command.\n> Instead, we can still keep fixing and improving 'git notes'.\n>\n>> 1) There's no simple way to store remote notes refs outside the \n>> refs/notes\n>> namespace in such a way that they can be used with git notes \n>> commands.\n>>\n>> 2) People who want to experiment with using git notes storage as a \n>> basis for\n>> building some new feature are forced to store their refs under the\n>> refs/notes namespace even if that does not make sense for the \n>> feature when\n>> what's stored in the notes tree is not intended to provide any \n>> content that\n>> is directly meaningful to the user.\n>\n>> A whitelist solves issue (1) but is no help for issue (2) unless some\n>> additional additional part of the refs namespace were to be also\n>> whitelisted. Perhaps something like refs/x-/... in the \n>> same vein\n>> as the various IETF standards for experimental names.\n>\n> Alternatively (or additionally), for issue (2), we could add a\n> --disable-ref-safety option to 'git notes', to explicitly disable the\n> safety checks for \"experimental\" use.\n\nI like that. Sort of a poor-man's git notes-plumbing option. And \nthere is precedence for this sort of thing as recently (v2.2.0) git \nhash-object learned a \"--literally\" option to bypass checks it would \notherwise normally perform.\n\n-Kyle\n"},{"id":"254402","messageId":"xmqqiogilgr5.fsf@gitster.dls.corp.google.com","threadId":"38289","inReplyTo":"20150107011958.GA3536@peff.net","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T16:03:58Z","receivedAt":"2015-01-07T16:03:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Jan 06, 2015 at 02:20:33AM -0800, Junio C Hamano wrote:\n>\n>> The fact that \"git notes merge refs/heads/master\" fails is a very\n>> good prevention of end-user mistakes, and this removal of test\n>> demonstrates that we are dropping a valuable safety.\n>\n> Is it really that valuable? If it were:\n>\n> git notes merge master\n>\n> I could see somebody running that accidentally.\n> ...\n> But we are talking about\n> somebody who is already fully-qualifying a ref (and anything unqualified\n> continues to get looked up under refs/notes).\n\nThat (specifically 'merge') is not my real worry. It's the other\nway around, actually.\n\nBecause expand_notes_ref() makes sure that any given notes ref is\nprefixed appropriately to start with refs/notes/,\n\n git notes --ref=refs/heads/master add ...blah...\n git notes --ref=refs/tag/v1.0 add ...blah...\n\nwould be a sensible way when somebody wants to keep a forest of\nnotes refs, one per real ref. Wouldn't they have already been\ntrained to spell \"refs/heads/master\" when they want to refer to\nrefs/notes/refs/heads/master because of this?\n"},{"id":"254403","messageId":"xmqqegr6lgaf.fsf@gitster.dls.corp.google.com","threadId":"38289","inReplyTo":"CALKQrge3yEfmi26HMbckyw_ONRXUWtZGvz=SqqYaL2sqJaHbUg@mail.gmail.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T16:14:00Z","receivedAt":"2015-01-07T16:14:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> Alternatively (or additionally), for issue (2), we could add a\n> --disable-ref-safety option to 'git notes', to explicitly disable the\n> safety checks for \"experimental\" use.\n\nI actually would rather prefer to see a proper plumbing use\nsupported, either by a new \"git notes-plumb\" subcommand or \"git\nnotes --\" that gives the scriptors a stable interface\nto the low-level machinery without enforcing any Policy that belongs\nto the Porcelain layer. At the very least, the plumbing should:\n\n - disable anything that introduces potential ambiguity, e.g. DWIMs\n done by expand_notes_ref(), but not limited to it.\n\n - lift any restrictions based on policy, e.g. \"where can notes\n trees live\" (again, but not limited to this).\n"},{"id":"254458","messageId":"20150108103158.GB4806@peff.net","threadId":"38289","inReplyTo":"xmqqiogilgr5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-01-08T10:31:58Z","receivedAt":"2015-01-08T10:31:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 07, 2015 at 08:03:58AM -0800, Junio C Hamano wrote:\n\n> > But we are talking about\n> > somebody who is already fully-qualifying a ref (and anything unqualified\n> > continues to get looked up under refs/notes).\n> \n> That (specifically 'merge') is not my real worry. It's the other\n> way around, actually.\n> \n> Because expand_notes_ref() makes sure that any given notes ref is\n> prefixed appropriately to start with refs/notes/,\n> \n> git notes --ref=refs/heads/master add ...blah...\n> git notes --ref=refs/tag/v1.0 add ...blah...\n> \n> would be a sensible way when somebody wants to keep a forest of\n> notes refs, one per real ref. Wouldn't they have already been\n> trained to spell \"refs/heads/master\" when they want to refer to\n> refs/notes/refs/heads/master because of this?\n\nThanks, that is a more interesting case, and I agree that moving to\nallowing fully-qualified refs would technically be a regression. I'm\nstill slightly doubtful that this is something people do in practice,\nbut I guess we have no way to know for sure.\n\n-Peff\n"}]}