Gazelle diff mode patch generation creates invalid patches #1916
Comments
|
It appears that the diff library used does not correctly ignore trailing new lines, thus resulting in invalid patches. The diff library used is no longer being maintained, so long term this should likely migrate to a different diff library that is actively maintained, however in the short term I think we could look at simply trimming the trailing new lines after the diff object is created. The PR I have linked contains the required changes for the new line trimming. I wanted to gather feedback from the maintainers on the desired approach and possible paths forward. |
alandonham
changed the title
fmeum
pushed a commit
that referenced
this issue
Sep 14, 2024
<!-- Thanks for sending a PR! Before submitting: 1. If this is your first PR, please read CONTRIBUTING.md and sign the CLA first. We cannot review code without a signed CLA. 2. Please file an issue *first*. All features and most bug fixes should have an associated issue with a design discussed and decided upon. Small bug fixes and documentation improvements don't need issues. 3. New features and bug fixes must have tests. Documentation may need to be updated. If you're unsure what to update, send the PR, and we'll discuss in review. --> **What type of PR is this?** Bug fix **What package or component does this PR mostly affect?** cmd/gazelle **What does this PR do? Why is it needed?** This PR adds a workaround to strip trailing newlines from the resulting diff when running Gazelle in `mode=diff`. This aligns with the expected behavior of modern diff tools, which will ignore the last newline in a file. This is implemented to resolve an issue where modifying the end of a file caused Gazelle to output a diff that was invalid, due to the newlines. Applying patches generated in this case could not be applied when using `git apply` and would apply, but with a warning identifying this issues when using the system `patch` utility. **Which issues(s) does this PR fix?** Fixes #1916 **Other notes for review** How to Reproduce: Edit a `BUILD.bazel` file in this repository, and then run `bazel run //:gazelle -- -mode=diff -patch=temp.patch`. Next, run `git apply -p0 temp.patch`. Without these changes, patch application will fail, due to the issue described above.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What version of gazelle are you using?
1.22.5What version of rules_go are you using?
0.46.0What version of Bazel are you using?
7.2.1Does this issue reproduce with the latest releases of all the above?
Yes.
What operating system and processor architecture are you using?
MAC OS ARM64
What did you do?
How to Reproduce:
Edit a BUILD.bazel file that has already had Gazelle run on it (I edited one in this repository to reproduce) and add new lines to the end of the file. Next, run the command
bazel run //:gazelle -- -mode=diff -patch=temp.patchand generate a patch file. Next, run the commandgit apply -p0 temp.patch.What did you expect to see?
Patch is successfully applied.
File modifications that added the newlines are reverted.
What did you see instead?
It's also worth noting that the system
patchutility does apply the patches successfully, however it prints a warning regarding the validity of the patch.No such line 134 in input file, ignoring*The lines numbers in the error messages will be subject to change based on the file you are editing.
The text was updated successfully, but these errors were encountered: