Stefan Sperling <stsp@stsp.name> wrote:
> Questions for potential follow-up improvements:
> 
> When patch -pN is used in combination with patch -c then the
> stripped version of paths will be looked up in the repository,
> Should we mention this behaviour in the documentation? It could
> lead to surprising results in case some unrelated file exists in
> the specified commit at this stripped path.

oh, that's right!  the paths are stripped early in recv_patch, so once
we try to look up paths we already have the stripped ones.  I admit I
haven't thought about this case.

in one hand i think it's working as intended, but on the other i can see
how someone could be confused.  (tbf i was confused for a moment about
what would happen with both -pN and -c abc specified.)

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.

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

By the way, to expand the narrow edge case with a narrower one, when
exporting diffs from diff.git we should be able to apply them with merge
in got.git!  The commit ids are of course different, but the blob ids
are the same :)


Thanks,

Omar Polo