git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Tejun Heo <tj@kernel.org>
Cc: git@vger.kernel.org, Junio C Hamano <jch2355@gmail.com>,
	kernel-team@fb.com
Subject: Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
Date: Thu, 15 Nov 2018 09:40:44 -0500	[thread overview]
Message-ID: <20181115144044.GA16450@sigill.intra.peff.net> (raw)
In-Reply-To: <20181017143921.GR270328@devbig004.ftw2.facebook.com>

On Wed, Oct 17, 2018 at 07:39:21AM -0700, Tejun Heo wrote:

> A while ago, I proposed changes to name-rev and describe so that they
> can identify the commits cherry-picked from the one which is being
> shown.
> 
>   https://public-inbox.org/git/20180726153714.GX1934745@devbig577.frc2.facebook.com/T/
> 
> While the use-cases - e.g. tracking down which release / stable
> branches a given commit ended up in - weren't controversial, it was
> suggested that it'd make more sense to use notes to link cherry-picks
> instead of building the feature into name-rev.

Sorry for the slow reply. This was on my to-look-at pile, but for
some reason I accidentally put in my done pile.

> The patch appended to this message implements most of it (sans tests
> and documentation).  It's composed of the following two parts.
> 
> * 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.

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
tools, and fill in their gaps in a way that lets people easily do
similar things.

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.

> 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).

For (2) and (3), you can push/pull a notes tree, but setting up the
refspecs and config is fairly manual. Using the cherry-pick option I
suggested above would most locally-made picks, but you'd probably still
need to backfill sometimes anyway.

I don't think you'd need to be (nor am I sure you _could_ be) as fancy
as fetching notes and their respective commits together. The notes are
bound together in a single tree, so you cannot say "I don't have commit
1234abcd, I don't need the note for it". You get the whole tree. And
that is not such a bad thing. The big reason _not_ to fetch tags that
point to commits we don't have is that the tag fetch implies
reachability, which may mean sucking in a big chunk of history. Whereas
you can happily have a note pointing to a commit you don't have (though
I suppose you should avoid running "git notes prune" in that case).

> ---
>  Makefile                    |    1 
>  builtin.h                   |    1 
>  builtin/note-cherry-picks.c |  197 ++++++++++++++++++++++++++++++++++++++++++++
>  builtin/notes.c             |   17 ++-
>  git.c                       |    1 
>  notes.c                     |   95 +++++++++++++++++++++
>  notes.h                     |    7 +
>  object.c                    |    4 
>  object.h                    |    6 +
>  9 files changed, 320 insertions(+), 9 deletions(-)

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.

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.

-Peff

  parent reply	other threads:[~2018-11-15 14:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-17 14:39 [RFC] cherry-pick notes to find out cherry-picks from the origin Tejun Heo
2018-10-24 12:24 ` Tejun Heo
2018-11-13 18:00   ` Tejun Heo
2018-11-15 14:40 ` Jeff King [this message]
2018-12-05 16:20   ` Tejun Heo
2018-12-06 22:15     ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181115144044.GA16450@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jch2355@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).