I see this patch has not been picked up.
I would like to lobby for inclusion of this patch.
On Sep 19, 2014, at 11:22, Junio C Hamano wrote:
> Johan Herland <johan@xxxxxxxxxxx> writes:
>
>> On Fri, Sep 19, 2014 at 11:39 AM, Jeff King <peff@xxxxxxxx> wrote:
>>> On Fri, Sep 19, 2014 at 09:39:45AM +0200, Scott Chacon wrote:
>>>> This patch changes the expand_notes_ref function to check for
>>>> simply a
>>>> leading refs/ instead of refs/notes to check if we're being
>>>> passed an
>>>> expanded notes reference.
>>>
>>> I think this change affects not just "git notes merge", but all of
>>> the
>>> notes lookups (including just "git notes show")....
>> ...
>
> Is it our future direction to set up refs/remote-notes/<remote>/
> namespace?
When cloning (without --mirror) Git now sets up a fetch spec like:
+refs/heads/*:refs/remotes/origin/*
It's unfortunate that it does not preserve the notion of "heads" and
instead set it up like this:
+refs/heads/*:refs/remotes/origin/heads/*
In which case it would make more sense to then simply clone notes like
so:
+refs/notes/*:refs/remotes/origin/notes/*
That would also clear the way to always fetching all remote tags into
refs/remotes/<remote>/tags/* by default as well even if the local refs/
tags/* do not end up being updated.
It seems clumsy to me to use a new remotes-notes ref namespace. What
happens if Git grows the ability to store some kind of (bug) tracking
ticket in refs/tickets in the future? Does Git then use refs/remote-
tickets/<remote> to store the remote refs rather than simply refs/
remotes/<remote>/tickets?
There are a number of applications that create refs outside of refs/
heads/* and refs/tags/*. GitHub uses refs/pull/*, should Git have a
refs/remote-pull/<remote>/* namespace and for Gerrit refs/remote-
changes/<remote>/* and also refs/remote-cache-automerge/<remote>/*
(for refs/cache-automerge/*)?
> If so, let's not do it piecemeail in an unorganized
> guerrilla fashion by starting with a stealth enabler
>
> By "stealth enabler" I mean the removal of refs/notes/ restriction
> that was originally done as a safety measure to avoid mistakes of
> storing notes outside. The refs/remote-notes/ future direction
> declares that it is no longer a mistake to store notes outside
> refs/notes/, but that does not necessarily have to mean that
> anywhere under refs/ is fine. It may make more sense to be explicit
> with the code touched here to allow traditional refs/notes/ and the
> new hierarchy only. That way, we will still keep the "avoid
> mistakes" safety and enable the new layout at the same time.
This is the part where I want to lobby for inclusion of this change.
I do not believe it is consistent with existing Git practice to
enforce restrictions like this (you can only display/edit/etc. notes
under refs/notes/*).
Already that's not true if you use an ugly symbolic-ref workaround,
but that requires polluting your ref namespace.
With all the other Git "restrictions" they are almost always strong
guidance, not brutally enforced.
Take, for example, the "restriction" that HEAD should be either
detached or a symbolic ref to refs/heads/<something>.
It's not absolutely enforced. If you really want to, you can use git
symbolic-ref and set HEAD to somewhere else (even under refs/taga) --
and it mostly works -- `git branch` gets upset but you can commit new
changes, view the log, etc.
How about the "guidance" that pushing does not update tags even if the
change would be a fast-forward? Again, not enforced, use the -f
option or add an explicit refspec to the appropriate remote config.
What about the restriction that `git config --get user.name` cannot
end in "."? (It gets magically stripped off.) That's a toughie, but
if you really, really, really want to you can always `git cat-file
commit HEAD > temp`, add the trailing dot and then git update-ref HEAD
$(git hash-object -t commit -w temp)`. Not recommended but possible.
So anyway, my point is that arbitrarily forcefully restricting the
operation of the various notes commands to refs/notes/* does not seem
consistent with the way everything else works.
> The most important first step for that to happen is to make sure we
> are on the same page on that future direction. I personally think
> refs/remote-notes/<remote> that runs parallel to the remote tracking
> branch hierarchy refs/remotes/<remote> is a reasonable way to do
> this, but my words are no way final.
I'd prefer refs/remotes/<remote>/notes for the reasons stated above.
Having a refs/remote-notes/<remote>/* hierarchy opens the door to a
proliferation of refs/remote-<whatever>/<remote>/* items in the refs
namespace in the future.
So in the vein of providing guidance to the user but, in the end,
allowing the user to remain in control, I have gussied up Johan's
suggested fix for the failing notes test into the following patch.
--Kyle
-- 8< --
Subject: [PATCH] t/t3308-notes-merge.sh: succeed with relaxed notes refs
With the recent change to allow notes refs to be located in
the refs hierarchy in locations other than refs/notes/ the
'git notes merge refs/heads/master' test started succeeding.
Previously refs/heads/master would have been expanded to
a non-existing, ref refs/notes/refs/heads/master, and the
merge would have failed (as expected).
Now, however, since refs/heads/master exists and the new,
more relaxed notes refs rules leave it unchanged, the merge
succeeds. This has a follow-on effect which makes the
next two tests fail as well.
The refs/heads/master ref could just be replaced with
another ref name that does not exist such as refs/heads/xmaster,
but there are already several tests using non-existant refs
so instead just remove the refs/heads/master line.
Suggested-by: Johan Herland <johan@xxxxxxxxxxx>
Signed-off-by: Kyle J. McKay <mackyle@xxxxxxxxx>
---
t/t3308-notes-merge.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh
index 24d82b49..f0feb64b 100755
--- a/t/t3308-notes-merge.sh
+++ b/t/t3308-notes-merge.sh
@@ -90,7 +90,6 @@ test_expect_success 'fail to merge various non-note-trees' '
test_must_fail git notes merge refs/notes/ &&
test_must_fail git notes merge refs/notes/dir &&
test_must_fail git notes merge refs/notes/dir/ &&
- test_must_fail git notes merge refs/heads/master &&
test_must_fail git notes merge x: &&
test_must_fail git notes merge x:foo &&
test_must_fail git notes merge foo^{bar
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html