Re: [PATCH] notes: do not accept non-blobs as new notes

[Jeff King <peff@peff.net>]

On Wed, May 09, 2012 at 03:19:14PM +0700, Nguyen Thai Ngoc Duy wrote:

> > At the same time, is there any reason not to allow experimentation in
> > this area? We don't know what other people might be putting in their
> > private notes trees, and the current interface does allow this.
> >
> > Is this fixing some important problem that justifies making such
> > experimentation harder?
> 
> I was actually thinking about tree notes when I made this patch. I
> decided that if new git supports tree notes while old git does not,
> the old git should refuse to operate on tree notes so it won't cause
> any unintentional damages (e.g. appending a blob note to a tree note).
> It's too late to fix released git though, I don't know what to do in
> that case.

Hmm. I was thinking that we supported arbitrary sha1s as note values via
the command-line interface. But we don't; that is only the internal C
API, and it is quite difficult to do anything useful with non-blob notes
via the command-line interface. As you noticed, you can trick it into
doing so with "-C", but even "-c" has disastrous results (unless you
like dumping the binary tree object into your editor).

So the support from the command-line tool is pretty awful. And your
patches affected only that, not the internal API, which I find a more
likely candidate for people experimenting. So I take back my original
questions; I think using the command-line tool as-is on non-blob notes
is just crazy, and we are better to rigorously enforce that instead of
dumping binary junk on the user's terminal.

The slim possibility that somebody is using "git notes add -C" in
conjunction with parsing the output of "git notes list" to store trees
is probably not worth worrying about (it took me a long time to even
figure out if you _could_ have a usable workflow).

-Peff