Re: [PATCH] patch-ids.c: cache patch IDs in a notes tree

[John Keeping <john@keeping.me.uk>]

On Sat, May 11, 2013 at 06:57:02PM -0500, Johannes Schindelin wrote:
> On Sat, 11 May 2013, Linus Torvalds wrote:
> 
> > [...] I really think caching patch ID's should be something people
> > should be aware of is fundamentally wrong, even if it might work.
> 
> Given the incredible performance win, I would say it is still worth
> looking into.
> 
> If you store also a hash of Git version and diff options (may even be the
> hash of the raw bytes of the diff options if you do not plan to share the
> ref between machines) with the patch ID, you can make it safe.
> 
> That hash would be generated at patch_id init time and
> load_cached_patch_id() would check this hash in addition to the return
> value of get_sha1() (and ignore the note if the version/diff options
> differ).

I was thinking about this overnight, glad to see someone else had the
same idea :-)

It's slightly annoying because the diff options can be customized after
we return from init_patch_ids() so we either need a new
setup_patch_ids() function to be run after init once diff options have
been set or to set it lazily.  I'll try introducing a setup function.

> If you are following git.git slavishly, maybe hashing just the major/minor
> Git version would be in order to avoid frequent regeneration of identical
> patch IDs.

I think just storing the version is quite good here, and it avoids pain
when a topic that affects patch IDs is working its way through pu and
next.

> > And quite frankly, if you do rebases etc so much that you think patch
> > ID's are so important that they need to be cached, you may be doing
> > odd/wrong things.
> 
> AFAICT John actually gave a very valid scenario that validates his use
> case: git-gui patches are best tested in the git.git scenario but have to
> be contributed via git-gui.git. It's not John's fault that this typically
> requires a lot of rebasing between vastly divergent histories.

Actually, I don't think that use case is valid.  Because it's a subtree
merge I can be absolutely certain that nothing on the LHS of
master...git-gui/master is patch identical to anything on the RHS since
all the paths are different.  So doing "git log --cherry-mark" in that
case is completely useless.  I think my script should be able to learn
that, which gets rid of the really horrible case I was seeing, but it
would be nice to improve the "fast enough" cases as well if it can be
done without too much effort.