Re: [RFC] cherry-pick notes to find out cherry-picks from the origin

[Tejun Heo <tj@kernel.org>]

Hello, Jeff.

On Thu, Nov 15, 2018 at 09:40:44AM -0500, Jeff King wrote:
> Sorry for the slow reply. This was on my to-look-at pile, but for
> some reason I accidentally put in my done pile.

No worries and sorry about my late reply too.  Things were a bit
hectic.

> > * A new built-in command note-cherry-picks, which walks the specified
> >   commits and if they're marked with the cherry-pick trailer, adds the
> >   backlink to the origin commit using Cherry-picked-to tag in a
> >   cherry-picks note.
> 
> That makes sense. I think this could also be an option to cherry-pick,
> to instruct it to create the note when the cherry-pick is made.
> 
> But you may still want a command to backfill older cherry-picks, or
> those done by other people who do not care themselves about maintaining
> the notes tree.

So, I wanted to do both with the same command.  git-cherry-pick knows
which commits are new, so it can just pass those commits to
note-cherry-picks.  When backfilling the whole tree or newly pulled
commits, the appropriate command can invoke note-cherry-picks with the
new commits which should be super cheap.

> It _feels_ like this is something that should be do-able by plugging a
> few commands together, rather than writing a new C program. I.e.,
> something like:
> 
>   git rev-list --format='%(trailers)' HEAD |
>   perl -lne '
> 	/^commit ([0-9]+)/ and $commit = $1;
> 	/^\(cherry picked from commit ([0-9]+)/
> 		and print "$commit $1";
>   ' |
>   while read from to; do
> 	# One process per note isn't very efficient. Ideally there would
> 	# be an "append --stdin" mode. Double points if it understands
> 	# how to avoid adding existing lines.
> 	git notes append -m "Cherry-picked-to: $to" $from
>   done
> 
> which is roughly what your program is doing.  Not that I'm entirely
> opposed to doing something in C (we've been moving away from shell
> scripts anyway). But mostly I am wondering if we can leverage existing

But the above wouldn't clean up stale commits, which could happen with
e.g. abandoned releases, and would be prone to creating duplicates.
We sure can add all those to shell / perl scripts but it's difficult
for me to see the upsides of doing it that way.

> tools, and fill in their gaps in a way that lets people easily do
> similar things.

I'll respond to this together below.

> And on that note...
> 
> > * When formatting a cherry-picks note for display, nested cherry-picks
> >   are followed from each Cherry-picked-to tag and printed out with
> >   matching indentations.
> 
> That makes sense to me, but does this have to be strictly related to
> cherry-picks? I.e., in the more generic form, could we have a way of
> marking a note as "transitive" for display, and the notes-display code
> would automatically recognize and walk hashes?
> 
> That would serve your purpose, but would also allow similar things to
> easily be done in the future.

Below.

> > Combined with name-rev --stdin, it can produce outputs like the following.
> > [...]
> 
> Yeah, that looks pretty good.
> 
> > Locally, the notes can be kept up-to-date with a trivial post-commit
> > hook which invokes note-cherry-picks on the new commit; however, I'm
> > having a bit of trouble figuring out a way to keep it up-to-date when
> > multiple trees are involved.  AFAICS, there are two options.
> > 
> > 1. Ensuring that the notes are always generated on local commits and
> >    whenever new commits are received through fetch/pulls.
> > 
> > 2. Ensuring that the notes are always generated on local commits and
> >    transported with push/pulls.
> > 
> > 3. A hybrid approach - also generate notes on the receiving end and
> >    ensure that fetch/pulls receives the notes together (ie. similar to
> >    --tags option to git-fetch).
> > 
> > #1 seems simpler and more robust to me.  Unfortunately, I can't see a
> > way to implement any of the three options with the existing hooks.
> > For #1, there's no post-fetch hook.  For #2 and #3, there doesn't seem
> > to be a fool-proof way to make sure that the notes are transported
> > together.  Any suggestions would be greatly appreciated.
> 
> Yeah, I think (1) is the simplest: it becomes a purely local thing that
> you've generated these annotations. Unfortunately, no, I don't think
> there's anything like a post-fetch hook. This might be a good reason to
> have one. One can always do "git fetch && update-notes" of course, but
> having fetch feed the script the set of updated ref tips would be very
> helpful (so you know you can traverse from $old..$new looking for
> cherry-picks).

This sounds great to me.

> I only looked briefly over your implementation, but didn't see anything
> obviously wrong. I do think it would be nice to make it more generic, as
> much as possible. I think the most generic form is really:
> 
>   traverse-and-show-trailers | invert-trailers | add-notes
> 
> In theory I should be able to do the same inversion step on any trailer
> which mentions another commit.

Hmm... yeah, I can definitely separate out the first part into a
separate function, move dedup of notes to add-notes, and then glue
them together.

I'm a bit hesitant to expose the first part as a separate command
without knowing what other use cases would actually look like.  I'll
make it so that it's easy to do so when a new use case arises.

> If it is going to stay in C and be cherry-pick-specific, one obvious
> improvement would be to use the notes API directly, rather than spawning
> subprocesses. That should be much more efficient if you have a lot of
> notes to write.

I'll try to make the components more generic and then glue them
together in C and make it use the APIs directly.

Thanks.

-- 
tejun