Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern

From: Jeff King <peff@peff.net>
Date: Oct 9, 2020, 11:53 UTC
Message-ID: <20201009115335.GA2855114@coredump.intra.peff.net>
In-Reply-To: <874kn3rfxy.fsf@igel.home>

On Fri, Oct 09, 2020 at 09:55:53AM +0200, Andreas Schwab wrote:

Show 8 quoted lines

> >> >  search_merges () {
> >> >  	git rev-list --all --grep="Merge branch '$1'" \
> >> >  		--pretty=tformat:"%P %s" |
> >> > -	sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> >> > +	sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
> [...]
> I wonder why --pretty uses %s when it is filtered out again anyway?
> (There is also a duplicate --all.)

It does confirm that the commit in question is a (likely) merge commit, and not one that happens to have "Merge branch 'foo'" in the body, which the earlier --grep would have hit. But it doesn't actually check for 'foo' in the sed match, so it would be fooled by a commit message like:

  Merge branch 'bar'

    * bar:
      Merge branch 'foo'

If we wanted to tighten that up, then sed should match the branch name. If we're willing to accept the looseness, the whole thing could probably be:

  git rev-list --all --grep="Merge branch '$1'" \
    --min-parents 2 --parents ${early_exit:-"-1"} |
  awk '{print $3}'

I'm happy either way, but I'm not sure anybody overly cares. Let's not derail Denton's actual fix to make the script work in a sha256 world.

-Peff