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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: "got patch" format?
To:
Omar Polo <op@omarpolo.com>
Cc:
Christian Weisgerber <naddy@mips.inka.de>, gameoftrees@openbsd.org
Date:
Sat, 19 Mar 2022 18:31:21 +0100

Download raw body.

Thread
On Sat, Mar 19, 2022 at 06:15:23PM +0100, Omar Polo wrote:
> Christian Weisgerber <naddy@mips.inka.de> wrote:
> > I just tried to use "got patch" and promptly failed because I had
> > generated the patch file with "diff -u foo.orig foo", and "got patch"
> > choked because foo.orig didn't exist in the worktree.  Is this a bug,
> > work in progress, ...?
> 
> patch(1) (and in most cases 'git apply' too) prefer the new path over
> the old path, so in your case both pick up 'foo' as path and it works.
> 
> in 'got patch' i tried to handle the rename case too, and in fact at the
> moment it handles "git-style rename diffs" well.  This means however
> that both the "old" path and the "new" path must exists in the worktree.
> (see test_patch_rename)
> 
> (The only case where 'git diff' produces a diff for a rename that's not
> currently handled by 'got patch' is when a file is renamed without any
> change.  in that case the diff has only the header and no hunks in it.)
> 
> 'git apply' solves this by requiring some git-specific "annotations" to
> the diff for it to interpret the paths as rename.  ("headers" like
> similarity index, rename from/to and the leading "diff --git".)  In
> practice thought seems that "diff --git" and paths wit a/ and b/ are
> enough for it to do the rename.  I haven't checked the sources and the
> git-apply manpage doesn't mention anything (judging from a quick grep.)
> 
> We have multiple way to fix this:
> 
>  - don't support patches that rename files (can always be achieved by a
>    remove folled by the addition of the file like 'got diff' already
>    does.)

Given these three options, I would prefer the above, for now.

>  - add some heuristics in got_worktree_patch_check_path.  We could, for
>    instance, ignore the old path and just use the new when the old file
>    is nonexistent or unversioned.  Diff below is an example of this.
> 
>  - invent our own annotations?  "diff --got" anyone? :D

That would be bad for interop. Everyone else would have to parse our
custom annotation format.

So I think 'got patch' should expect git-compatible annotations,
and only support renames if they are present. We can leave this
for later, it doesn't have to happen right now.

'got diff' might eventually produce git-compatible annotations
instead of full delete + full add.

SVN did this, too. There is 'svn diff --git', and 'svn patch' can
parse Git's annotations. Everyone wins.