Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Paths not updated for a diff of modified binary file #1621

Closed
imbrish opened this issue Feb 10, 2024 · 2 comments 路 Fixed by #1629
Closed

馃悰 Paths not updated for a diff of modified binary file #1621

imbrish opened this issue Feb 10, 2024 · 2 comments 路 Fixed by #1629

Comments

@imbrish
Copy link
Contributor

imbrish commented Feb 10, 2024

I have a commit in which a binary file is modified, and a new text file is added.

If I diff only the binary file, everything is good:

diff --git a/meta/archive.zip b/meta/archive.zip
index 00de669..d47cd84 100644
Binary files a/meta/archive.zip and b/meta/archive.zip differ

image

But if I diff it together with the text file, the label for the modified binary file becomes incorrect:

diff --git a/meta/added.txt b/meta/added.txt
new file mode 100644
index 0000000..b5eab54
--- /dev/null
+++ b/meta/added.txt
@@ -0,0 +1 @@
+This file was not here before.
diff --git a/meta/archive.zip b/meta/archive.zip
index 00de669..d47cd84 100644
Binary files a/meta/archive.zip and b/meta/archive.zip differ

image

I have built delta from the source, commit e208f4e, but tested also for tag 0.16.5 (03f1569) with the same outcome.

May be related to #96 and #1502.

@imbrish
Copy link
Contributor Author

imbrish commented Feb 10, 2024

Apparently, I have used the wrong executable to test 0.16.5, here is the actual result for b.diff (still not quite right):

image

See also a combined example c.diff (the foo.dll deleted, bar.dll added and abc.dll modified):

diff --git a/exts/foo.dll b/exts/foo.dll
deleted file mode 100644
index 4aa3244..0000000
Binary files a/exts/foo.dll and /dev/null differ
diff --git a/exts/bar.dll b/exts/bar.dll
new file mode 100644
index 0000000..cc5b40c
Binary files /dev/null and b/exts/bar.dll differ
diff --git a/exts/abc.dll b/exts/abc.dll
index 00de669..d47cd84 100644
Binary files a/exts/abc.dll and b/exts/abc.dll differ

0.16.5 03f1569:

image

e208f4e:

image

@imbrish
Copy link
Contributor Author

imbrish commented Feb 18, 2024

This was my first time looking at rust code, but it seems that the minus_file and plus_file are updated from diff --git line at the deleted file mode or new file mode lines, neither of which appear for the modified library diff. Thus, the previous path values remain, and are re-used for the modified diff.

For the current HEAD this means that (binary file) is appended to the filename twice. I think this information should better be stored at something like self.minus_file_binary and used when writing a new header. All of such diff data should probably be reset at each new diff header.

Anyway, why not update the file names already at diff --git line? Could be later overwritten at ---, +++ etc.

@imbrish imbrish changed the title 馃悰 Incorrect label for modified binary file, when a text file is added in the same commit 馃悰 Paths not updated for a diff of modified binary file Feb 18, 2024
imbrish added a commit to imbrish/delta that referenced this issue Feb 24, 2024
imbrish added a commit to imbrish/delta that referenced this issue Feb 24, 2024
imbrish added a commit to imbrish/delta that referenced this issue Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant