Commit notes workflow

Commit notes workflow

[Yann Dirson]

We have notes merge support since a couple of releases now, but no real example
in the docs of how best to use that.  That is, no suggested mapping of remote notes,
let alone automatic setup of refspecs at clone time.

Trying to setup such refspecs, I find myself puzzled:

* if I store remote notes under refs/notes (eg. refs/notes/*:refs/notes/origin/* as fetch
  refspec), then a refs/notes/*:refs/notes/origin/* push refspec will include
  refs/notes/origin/*, which we obviously don't want

* if I store them outside of refs/notes (eg. refs/notes/*:refs/remote-notes/origin/* ),
  then "git notes" silently ignores them: no output nor any error message from "notes list"
  or "notes merge".

Do we really want to "git notes" to ignore everything not in refs/notes/ ?  I can think of
2 possibilities out of this situation:

* remove that limitation
* decide on a naming convention for remote notes, and teach "git notes" not to ignore it

A (minor) problem with the second possibility is that this naming convention could evolve,
eg. if we end up with something like was proposed in [1] for 1.8.0.  Is there any real drawback
with the first suggestion ?

[1] http://marc.info/?l=git&m=129661334011986&w=4
-- 
Yann Dirson - Bertin Technologies

Re: Commit notes workflow

[Johan Herland]

On Monday 13 June 2011, Yann Dirson wrote:
> We have notes merge support since a couple of releases now, but no real
> example in the docs of how best to use that.  That is, no suggested
> mapping of remote notes, let alone automatic setup of refspecs at clone
> time.

True. I think this has been held up, partly because I (or anyone else) 
haven't found the time to work on this, and partly because we want to add 
some kind of default refspec to easily share notes between repos; the latter 
has been caught up in the discussion you refer to in [1].

> Trying to setup such refspecs, I find myself puzzled:
> 
> * if I store remote notes under refs/notes (eg.
> refs/notes/*:refs/notes/origin/* as fetch refspec), then a
> refs/notes/*:refs/notes/origin/* push refspec will include
> refs/notes/origin/*, which we obviously don't want
> 
> * if I store them outside of refs/notes (eg.
> refs/notes/*:refs/remote-notes/origin/* ), then "git notes" silently
> ignores them: no output nor any error message from "notes list" or
> "notes merge".
> 
> Do we really want to "git notes" to ignore everything not in refs/notes/
> ?  I can think of 2 possibilities out of this situation:
> 
> * remove that limitation
> * decide on a naming convention for remote notes, and teach "git notes"
> not to ignore it

The naming convention I have proposed (in the discussion for [1]) is 

  refs/notes/*:refs/remotes/$remote/notes/*

(but it obviously depends on reorganizing the entire remote refs hierarchy)

> A (minor) problem with the second possibility is that this naming
> convention could evolve, eg. if we end up with something like was
> proposed in [1] for 1.8.0.  Is there any real drawback with the first
> suggestion ?
> 
> [1] http://marc.info/?l=git&m=129661334011986&w=4

My gut feeling is to keep some sort of limit notes refs, and if/when we get 
around to implementing my proposal in [1] (or some variation thereof), we 
will of course extend the limit to put "refs/remotes/$remote/notes/*" (or 
whatever is decided) in the same category as "refs/notes/*".

In the meantime, I'm unsure if it's a good idea to remove the limitation 
altogether (allowing notes refs everywhere), since re-introducing a limit at 
a later point will then be MUCH harder...


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: Commit notes workflow

[ydirson]

> > Do we really want to "git notes" to ignore everything not in  refs/notes/
> > ? I can think of 2 possibilities out of this situation:
> > 
> > * remove that limitation
> > * decide on a naming convention for remote notes, and teach  "git notes"
> > not to ignore it
> 
> The naming convention I have proposed (in the discussion for 
> [1]) is 
> 
> refs/notes/*:refs/remotes/$remote/notes/*
> 
> (but it obviously depends on reorganizing the entire remote refs  hierarchy)
> > A (minor) problem with the second possibility is that this naming
> > convention could evolve, eg. if we end up with something like was
> > proposed in [1] for 1.8.0. Is there any real drawback with  the first
> > suggestion ?
> > 
> > [1] http://marc.info/?l=git&m=129661334011986&w=4
> 
> My gut feeling is to keep some sort of limit notes refs, and  if/when we get 
> around to implementing my proposal in [1] (or some variation  thereof), we 
> will of course extend the limit to put  "refs/remotes/$remote/notes/*" (or 
> whatever is decided) in the same category as "refs/notes/*".
> 
> In the meantime, I'm unsure if it's a good idea to remove the  limitation 
> altogether (allowing notes refs everywhere), since re- introducing a limit at 
> a later point will then be MUCH harder...

So we could introduce something like refs/remote-notes/<remote>/* today to
start working, and eventually phase it out when refs/remotes/ gets restructured.

Then the next point will be how best to provide git-pull-like support for notes refs.
We have a number of alternatives, like:

* having "git pull" run "git notes merge" on all notes refs with a tracking-branch set to
   the repo from which we pull
* do the same for a configured set of notes refs only
* only have "git pull" and "git status" notify about notes refs being not uptodate, and
   add an explicit "git notes pull" command of some sort (maybe just "git notes merge"
   without an argument, which would be consistent with latest "git merge")
* surely others

Re: Commit notes workflow

[Johan Herland]

On Tuesday 14. June 2011, dirson@bertin.fr wrote:
> > > Do we really want to "git notes" to ignore everything not in
> > >  refs/notes/ ? I can think of 2 possibilities out of this
> > > situation:
> > > 
> > > * remove that limitation
> > > * decide on a naming convention for remote notes, and teach  "git
> > > notes" not to ignore it
> > 
> > The naming convention I have proposed (in the discussion for
> > [1]) is
> > 
> > refs/notes/*:refs/remotes/$remote/notes/*
> > 
> > (but it obviously depends on reorganizing the entire remote refs
> >  hierarchy)
> > 
> > > A (minor) problem with the second possibility is that this naming
> > > convention could evolve, eg. if we end up with something like was
> > > proposed in [1] for 1.8.0. Is there any real drawback with  the
> > > first suggestion ?
> > > 
> > > [1] http://marc.info/?l=git&m=129661334011986&w=4
> > 
> > My gut feeling is to keep some sort of limit notes refs, and
> >  if/when we get around to implementing my proposal in [1] (or some
> > variation  thereof), we will of course extend the limit to put
> >  "refs/remotes/$remote/notes/*" (or whatever is decided) in the
> > same category as "refs/notes/*".
> > 
> > In the meantime, I'm unsure if it's a good idea to remove the
> >  limitation altogether (allowing notes refs everywhere), since
> > re- introducing a limit at a later point will then be MUCH
> > harder...
> 
> So we could introduce something like refs/remote-notes/<remote>/*
> today to start working, and eventually phase it out when
> refs/remotes/ gets restructured.

Yes, if you can't wait for the refs/remotes/ restructuring, then I guess 
you'll have to do that.

> Then the next point will be how best to provide git-pull-like support
> for notes refs. We have a number of alternatives, like:
> 
> * having "git pull" run "git notes merge" on all notes refs with a
> tracking-branch set to the repo from which we pull
> * do the same for a configured set of notes refs only
> * only have "git pull" and "git status" notify about notes refs being
> not uptodate, and add an explicit "git notes pull" command of some
> sort (maybe just "git notes merge" without an argument, which would
> be consistent with latest "git merge") * surely others

I guess there are a lot of different possibilities here, and there will 
probably be disagreement on what's the best default, so I'd suggest the 
following guidelines:

* make it as configurable as possible.

* follow the existing conventions of pull/merge w.r.t. branches, but 
only so far as it makes sense for notes.

* leave the defaults conservative (e.g. don't do any merging by default, 
but make pull/status notify about update-able notes refs).


My idea so far, is to model the notes configuration on the current 
branch configuration, e.g. something like this:

  [remote "origin"]
      ...
      fetch = +refs/notes/*:refs/remotes/origin/notes/*
      ...

  [notes "commits"]
      remote = origin
      merge = refs/notes/commits

  [notes "bugs"]
      remote = origin
      merge = refs/notes/bugs
      mergeoptions = --strategy=cat_sort_uniq
      automerge = true

Except for the "automerge" option, everything is analogous to current 
branch.<name>.* options. The above configuration sets up a default 
tracking ref for "refs/notes/commits", making

  git notes --ref commits merge

equivalent to

  git notes --ref commits merge refs/remotes/origin/notes/commits

This notes merge would not happen automatically.

The last section, however, would presumably trigger an automatic notes 
merge (on fetch? pull?) because of notes.bugs.automerge being enabled. 
In this case, the

  git notes --ref bugs merge

command would be issued, which would be equivalent to

  git notes --ref bugs merge --strategy=cat_sort_uniq \
      refs/remotes/origin/notes/bugs

This is just a suggestion, and we might want to impose additional 
restrictions not mentioned above. For example, enabling "automerge" 
without enabling a non-"manual" notes merge strategy is probably unwise, 
because it can force the user to resolve conflicts from a notes merge 
that the user did not explicitly initiate.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: Commit notes workflow

[ydirson]

> > A (minor) problem with the second possibility is that this naming
> > convention could evolve, eg. if we end up with something like was
> > proposed in [1] for 1.8.0.  Is there any real drawback with the
> first
> > suggestion ?
> > 
> > [1] http://marc.info/?l=git&m=129661334011986&w=4
> 
> My gut feeling is to keep some sort of limit notes refs, and if/when we get 
> around to implementing my proposal in [1] (or some variation thereof), we 
> will of course extend the limit to put "refs/remotes/$remote/notes/*" (or 
> whatever is decided) in the same category as "refs/notes/*".
> 
> In the meantime, I'm unsure if it's a good idea to remove the limitation 
> altogether (allowing notes refs everywhere), since re-introducing a limit at 
> a later point will then be MUCH harder...

I'm still unsure what that limitation brings to us.  OTOH, it has at least one
funny downside: when someone tries to refer to some forbidden ref using --ref, it
gets silently requalified:

$ git notes --ref=refs/remote-notes/foo add
$ find .git/refs/notes/ -type f
.git/refs/notes/refs/remote-notes/foo
$

It just seems so wrong...  Surely we can mitigate it by considering a ref starting
with "refs/" to be absolute, and thus never prepend "refs/notes/" to it, but it rather
sounds to me a symptom that we may not want to filter things anyway.

Re: Commit notes workflow

[Johan Herland]

On Wednesday 15. June 2011, ydirson@free.fr wrote:
> > > A (minor) problem with the second possibility is that this naming
> > > convention could evolve, eg. if we end up with something like was
> > > proposed in [1] for 1.8.0.  Is there any real drawback with the
> > 
> > first
> > 
> > > suggestion ?
> > > 
> > > [1] http://marc.info/?l=git&m=129661334011986&w=4
> > 
> > My gut feeling is to keep some sort of limit notes refs, and
> > if/when we get around to implementing my proposal in [1] (or some
> > variation thereof), we will of course extend the limit to put
> > "refs/remotes/$remote/notes/*" (or whatever is decided) in the
> > same category as "refs/notes/*".
> > 
> > In the meantime, I'm unsure if it's a good idea to remove the
> > limitation altogether (allowing notes refs everywhere), since
> > re-introducing a limit at a later point will then be MUCH
> > harder...
> 
> I'm still unsure what that limitation brings to us.  OTOH, it has at
> least one funny downside: when someone tries to refer to some
> forbidden ref using --ref, it gets silently requalified:
> 
> $ git notes --ref=refs/remote-notes/foo add
> $ find .git/refs/notes/ -type f
> .git/refs/notes/refs/remote-notes/foo
> $
> 
> It just seems so wrong...  Surely we can mitigate it by considering a
> ref starting with "refs/" to be absolute, and thus never prepend
> "refs/notes/" to it, but it rather sounds to me a symptom that we
> may not want to filter things anyway.

The reason we put the limitation there, is to prevent the notes code 
from screwing with non-notes trees. The notes code reorganizes the notes 
tree depending on the number of tree entries, in order to achieve 
acceptable performance for notes trees of all sizes. Therefore, you 
definitely DON'T want the notes code rummaging around in non-notes trees 
(especially if some of your tree entries can be mistaken for strings of 
hex digits).

That said, the example you give above ("refs/remote-notes/foo" -> 
"refs/notes/refs/remote-notes/foo" is obviously a stupid failure, and 
should be fixed. Considering "refs/*" to be absolute seems safe to me. 
(Obviously we loose the "refs/notes/refs/*" namespace, but I can live 
with that.)


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: Commit notes workflow

[ydirson]

> I'm still unsure what that limitation brings to us.  OTOH, it has at least one
> funny downside: when someone tries to refer to some forbidden ref using --ref, it
> gets silently requalified:
> 
> $ git notes --ref=refs/remote-notes/foo add
> $ find .git/refs/notes/ -type f
> .git/refs/notes/refs/remote-notes/foo
> $
> 
> It just seems so wrong...  Surely we can mitigate it by considering a ref starting
> with "refs/" to be absolute, and thus never prepend "refs/notes/" to it, but it rather
> sounds to me a symptom that we may not want to filter things anyway.

While playing with this, I realized that when editing the template
does not name the notes ref being edited.  When looking at the code,
I notice that, contrarily to commit.c which uses stdio, notes.c uses
write_or_die(), which is a bit less flexible for formatting.

I'd think we could me things more consistent - is there any objection
to switch notes.c to using stdio for this ?

Re: Commit notes workflow

[Johan Herland]

On Wednesday 15. June 2011, ydirson@free.fr wrote:
> > I'm still unsure what that limitation brings to us.  OTOH, it has
> > at least one funny downside: when someone tries to refer to some
> > forbidden ref using --ref, it gets silently requalified:
> > 
> > $ git notes --ref=refs/remote-notes/foo add
> > $ find .git/refs/notes/ -type f
> > .git/refs/notes/refs/remote-notes/foo
> > $
> > 
> > It just seems so wrong...  Surely we can mitigate it by considering
> > a ref starting with "refs/" to be absolute, and thus never prepend
> > "refs/notes/" to it, but it rather sounds to me a symptom that we
> > may not want to filter things anyway.
> 
> While playing with this, I realized that when editing the template
> does not name the notes ref being edited.  When looking at the code,
> I notice that, contrarily to commit.c which uses stdio, notes.c uses
> write_or_die(), which is a bit less flexible for formatting.
> 
> I'd think we could me things more consistent - is there any objection
> to switch notes.c to using stdio for this ?

Go ahead, Doing things in line with commit.c seems good to me.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net