`git apply -p` doesn't remove source prefix in 'rename' diff entries.

Commit Message

Iago-lito Nov. 1, 2024, 12:17 a.m. UTC

Hello,

I have troubles understanding the behaviour of `git apply` with respect to renames in a `--no-index` context.

Let us craft a toy folder:
```sh
$ mkdir x
$ echo a > x/a # To be modified.
$ echo b > x/b # To be renamed.
```

Duplicate it twice to get three identical folders `x = y = z`.

```sh
$ cp -r x y
$ cp -r x z
```

Modify `y`:
```sh
$ echo newline >> y/a # Edit.
$ mv y/b y/c # Rename.
```

Now I would like to use git as a "better GNU patch".
Calculate the diff from `x` to `y`:
```sh
$ git diff --no-prefix x y | tee patch
```

Then apply this patch to `z` so as to end up with `x != y = z`:
```sh
$ git apply -p1 --directory z patch
error: z/x/b: No such file or directory
```

I would expect `z/b` to be renamed into `z/c`, but I get the above error instead.

Although understand that `z/x/b` is not a valid path, I don't understand that git produces such a path, because I'm assuming that `-p1` is supposed to remove the `x/` part.

Interestingly, the procedure works fine without the rename. The rename also worth fine if I manually remove the `x/` and `y/` prefixes from the rename lines in the patch.

This makes the behaviour of `git apply` appear inconsistent. It is nonetheless expected ? If so, why ? If not, then is it a bug ?

Thank you for support,

[System Info]
git version:
git version 2.46.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
libcurl: 8.10.1
OpenSSL: OpenSSL 3.3.2 3 Sep 2024
zlib: 1.3.1
uname: Linux 6.10.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 12 Sep 2024
17:21:02 +0000 x86_64
compiler info: gnuc: 14.2
libc info: glibc: 2.40
$SHELL (typically, interactive shell): /usr/bin/zsh


[Enabled Hooks]
not run from a git repository - no hooks to show

--
Iago-lito

Comments

Junio C Hamano Nov. 1, 2024, 7:05 a.m. UTC | #1

Iago-lito <[e-mail]> writes:

> Hello,
>
> I have troubles understanding the behaviour of `git apply` with respect to renames in a `--no-index` context.
>
> Let us craft a toy folder:
> ```sh
> $ mkdir x
> $ echo a > x/a # To be modified.
> $ echo b > x/b # To be renamed.
> ```
>
> Duplicate it twice to get three identical folders `x = y = z`.
>
> ```sh
> $ cp -r x y
> $ cp -r x z
> ```
>
> Modify `y`:
> ```sh
> $ echo newline >> y/a # Edit.
> $ mv y/b y/c # Rename.
> ```
>
> Now I would like to use git as a "better GNU patch".
> Calculate the diff from `x` to `y`:
> ```sh
> $ git diff --no-prefix x y | tee patch
> diff --git x/a y/a
> index 7898192..4030aa5 100644
> --- x/a
> +++ y/a
> @@ -1 +1,2 @@
> a
> +newline
> diff --git x/b y/c
> similarity index 100%
> rename from x/b
> rename to y/c
> ```

Interesting.  I think "git diff --no-index" is "wrong" here, but
"wrong" is not in the sense that it is _incorrect_, but in the sense
that it merely is incompatible with what you are trying to achieve.

If you had files a and b, modified a in place and renamed b to c,
and if you did this in a repository with tracked files, you would
have seen

   $ git diff --src-prefix=x --dst-prefix=y
   diff --git x/a y/a
   index ...
   --- x/a
   +++ y/a
   ...
   diff --git x/b y/c
   similarity index 100%
   rename from b
   rename to c

Notice how "rename from" and "rename to" information is different in
the "real" git-diff output, as opposed to "git diff --no-index"
output you have?

At least when comparing two directories, "git diff --no-index X Y"
could be interpreted as a request to treat as if X and Y were
checkouts from two Git commits X and Y, and by comparing them the
output should be identical to the result of comparing the contents
of commits X and Y.

But feeding "git apply" however may not be the only reason why
people are using "diff --no-index X Y", and there may be a
legitimate reason why peole want to see "from x/b to y/c" in the
output, so unconditionally changing the output may very likely break
workflows of other people.

Iago-lito Nov. 4, 2024, 8:35 p.m. UTC | #2

Hello @Junio, and thank you for feedback :)

If I understand correctly, you are suggesting that `git diff`/`git 
apply` are behaving correctly here, but they might not be the right tool 
for the job? I sort of sense that `git` could achieve this "z = x + 

Junio C Hamano Nov. 5, 2024, 12:36 a.m. UTC | #3

Iago-lito <[e-mail]> writes:

> Hello @Junio, and thank you for feedback :)
>
> If I understand correctly, you are suggesting that `git diff`/`git
> apply` are behaving correctly here, but they might not be the right
> tool for the job?

No.  

"git diff --no-index" is giving "wrong" information in the
rename/copy extended diff headers, but regular "git diff" is
perfectly fine.  Also "git apply" is perfectly fine.

And the "wrong"-ness above is in the eyes of beholder, in the sense
that the original folks who coaxed "these two directories are not
controlled by Git, but unlike GNU diff, "git diff" has a few nice
bells and whistles, so instead of adding these properly to GNU diff,
abuse "git diff" to add a mode where two directories are compared"
probably did not view that "git diff --no-index X Y" must be
equivalent to committing the contents of X first and then Y next,
and then doing "git show" on the resulting history.

I do not know why being different and showing X and Y as part of the
paths reported on the "rename from" etc. extended diff headers could
sometimes be useful, but they must have had some use cases I am not
seeing offhand.

So in order to lift this "wrong"-ness, you'd probably want an option
to the "no-index" mode of the "git diff" (in "diff-no-index.c") to
omit the leading directory name (i.e. X/ and Y/) from these
rename/copy extended diff headers.  Then your toy example would work
when the "git diff --no-index --fixed-rename-copy-path-info" is used.

Patch

diff --git x/a y/a
index 7898192..4030aa5 100644
--- x/a
+++ y/a
@@ -1 +1,2 @@ 
a
+newline
diff --git x/b y/c
similarity index 100%
rename from x/b
rename to y/c