"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got patch: add -c to apply at specified commit
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 28 Jul 2022 13:14:02 +0200

Download raw body.

Thread
On Thu, Jul 28, 2022 at 12:46:50PM +0200, Omar Polo wrote:
> I'm not sure what's the best way.  Should the wording in -p be more
> clear and say that
> 
>     -p strip-count
> 	Specify the number of leading path component to strip
> 	from paths parsed from patchfile before interpeting them.
> 
> or just a generic warning in -c?
> 
>     -c commit
> 	...
> 	If -p is used, the paths are stripped before being looked up.

A warning like this under -c is good enough.

> > If the path cannot be found in the commit, got patch will not do a
> > 3-way merge. Should it try to look up a blob ID directly in that case,
> > if a blob ID was available in diff headers? This might be useful in case
> > a diff was generated against a commit which does not (yet) exist in the
> > local repository, and where the referenced base blob does already exist
> > somewhere in history. Again a narrow edge case, but I don't see why we
> > should not use the base blob if we have it available.
> 
> this should already be the case.  if -c is not used, prepare_merge will
> try to open the blob id parsed from the patch (if any).  The commit id
> extracted is only used to provide a better merge conflict header,
> otherwise the (not so useful) blob id will be used.  (see the `if
> (do_merge) { ... }' part in apply_patch.)

Ah, right. But why not offer the convenience of a blob-id-lookup for
users of the -c option as well? Currently, as I understand, using -c
has the effect that blobs are only looked up by path, and the lookup
by object ID is entirely skipped even when a path-based lookup fails:

	if (commit_id) {
		/* lookup by path */
	} else if (*p->blob != '\0') {
		/* lookup by ID */
	}

So -c helps when there is a blob at a matching path which is suitable as a
merge base, even if it has different blob object ID. But -c doesn't help if
there is a suitable blob with the same ID present at a different path, or
present even in an entirely different commit.
Given that there can be multiple files in a patch, is it not possible that
both of these cases could apply within the context of a patch operation?
Depending on the patch, the source/target repositories, the commit selected
by -c, and use of the -pN option... Maybe we can merge one file using a
blob found at its path, and merge a different file only by blob-ID?