On Jan 6, 2015, at 10:25, Junio C Hamano wrote:
"Kyle J. McKay" <mackyle@xxxxxxxxx> writes:
So despite the name of the test, the actual tree contents do not seem
to be examined.
Yes, but the thing is, thanks to refs/notes restriction, there is no
need to do such examination [*1*].
Note that it is an entirely different matter when you deliberately
violate the expectation using plumbing (e.g. update-ref). Users of
plumbing commands are expected to know what they are doing, so the
level of safety we need to give them can be much lower than Porcelain
commands such as 'git notes'.
But when you stick to Porcelain commands, it is very hard to mix
refs/notes/* with refs/heads/* and refs/remotes/* by mistake. You
have to really work on it by doing something unusual to have a non
commit in refs/heads/*, a commit in refs/notes/*, or a regular
non-note commit in refs/notes/*.
Perhaps that is the crux of the issue. There is no git notes-plumbing
command where the git notes command continues to apply restrictions
but the vaporware notes-plumbing command allows anything just like
update-ref does.
I think there are two issues here:
1) There's no simple way to store remote notes refs outside the refs/
notes namespace in such a way that they can be used with git notes
commands.
2) People who want to experiment with using git notes storage as a
basis for building some new feature are forced to store their refs
under the refs/notes namespace even if that does not make sense for
the feature when what's stored in the notes tree is not intended to
provide any content that is directly meaningful to the user.
That is exactly what I meant by that the existing safety pays price
of not being able to store notes outside refs/notes, which may be
too high a price to pay.
Although I am not fundamentally against allowing to store notes
outside refs/notes/, it is different from "anywhere is fine".
Can't we do this widening in a less damaging way?
Without arbitrarily restricting where notes can be stored it seems to
me the only option would be to have the notes machinery actually
inspect the tree of any existing notes ref it's passed.
As I said earlier (assuming you read footnotes before you read a new
paragraph), the ship has already sailed.
Hmpf. So the only possible safety check is refname-based. But, as
you say, it's no use crying over spilled milk [3].
Obvious two sensible ways forward are to do a blacklist (i.e. allow
anywhere except for known non-notes namespaces like refs/heads/) or
do a whitelist (i.e. allow refs/<some-known-space> in addition to
refs/notes) of the namespace, and the latter is vastly preferrable
than the former, because you can practically *never* crawl back a
namespace once you give it to the general public, even if you later
find it a grave error to open it too widely and need a more
controlled access [*2*]. And the name of the game for a software
that can be relied for a long haul is to avoid painting ourselves in
a corner that we cannot get out of.
If we add refs/remote-notes/* to the whitelist now, and if later it
turns out to be not so ideal and we would prefer another layout for
remotes, e.g. refs/remotesNew/*/{heads,notes,tags}/, we can add
refs/remotesNew/*/notes/ to the whitelist _without_ breaking those
who have already started using refs/remote-notes/*. That is why
I said whitelist is preferrable than blacklist.
A whitelist solves issue (1) but is no help for issue (2) unless some
additional additional part of the refs namespace were to be also
whitelisted. Perhaps something like refs/x-<anything>/... in the same
vein as the various IETF standards for experimental names.
[Footnote]
*1* I actually do not think a tree examination would help very much
here. IIRC, somebody decided long time ago that it would be a
good idea to be able to store a path that is not a fanned-out
40-hex in a notes tree and 'git notes merge' would accept such a
notes-tree. Although I doubt that the resulting notes-tree
produced by 'notes merge' is carefully designed one (as opposed
to whatever the implementation happens to do) with sensible
semantics, people may already be relying on it.
*2* The above 'notes-tree can store non fanned-out 40-hex' is a good
example why you need to start strict and loosen only when it
becomes necessary. Despite that even Git itself does not use
that "facility" to do anything useful AFAIK, only because we
started with a loose variant that allows arbitrary garbage, we
cannot retroactively tighten the definition of what a notes-tree
should look like without risking to break practice of other
people.
[3] http://cheezburger.com/6423972864
-Kyle
--
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