Skip to content

Commit

Permalink
Fix patch file creation using Gazelle in diff mode (#1915)
Browse files Browse the repository at this point in the history
<!-- 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.
  • Loading branch information
alandonham authored Sep 14, 2024
1 parent ba1cae3 commit 8c64e02
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 13 deletions.
9 changes: 9 additions & 0 deletions cmd/gazelle/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ func diffFile(c *config.Config, f *rule.File) error {
} else {
diff.ToFile = outPath
}
// Trim off trailing newline from the end of the file. Modern diff tools typically
// handle this, however difflib does not and is no longer maintained, so we need to
// handle it here.
if len(diff.A) > 0 {
if diff.A[len(diff.A)-1] == "\n" {diff.A = diff.A[:len(diff.A)-1]}
}
if len(diff.B) > 0 {
if diff.B[len(diff.B)-1] == "\n" {diff.B = diff.B[:len(diff.B)-1]}
}

uc := getUpdateConfig(c)
var out io.Writer = os.Stdout
Expand Down
11 changes: 4 additions & 7 deletions cmd/gazelle/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,17 @@ func TestDiffExisting(t *testing.T) {
Content: `
--- BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
+++ BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
@@ -1,3 +1,11 @@
@@ -1,2 +1,10 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
# gazelle:prefix example.com/hello
+
+go_library(
+ name = "hello",
+ srcs = ["hello.go"],
+ importpath = "example.com/hello",
+ visibility = ["//visibility:public"],
+)
+
`,
})
testtools.CheckFiles(t, dir, want)
Expand All @@ -88,7 +87,7 @@ func TestDiffNew(t *testing.T) {
Content: `
--- /dev/null 1970-01-01 00:00:00.000000001 +0000
+++ BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
@@ -0,0 +1,9 @@
@@ -0,0 +1,8 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
Expand All @@ -97,7 +96,6 @@ func TestDiffNew(t *testing.T) {
+ importpath = "example.com/hello",
+ visibility = ["//visibility:public"],
+)
+
`,
})
testtools.CheckFiles(t, dir, want)
Expand Down Expand Up @@ -160,7 +158,7 @@ func TestDiffReadWriteDir(t *testing.T) {
wantPatch := fmt.Sprintf(`
--- %s 1970-01-01 00:00:00.000000001 +0000
+++ %s 1970-01-01 00:00:00.000000001 +0000
@@ -1 +1,11 @@
@@ -1 +1,10 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
# gazelle:prefix example.com/hello
Expand All @@ -171,7 +169,6 @@ func TestDiffReadWriteDir(t *testing.T) {
+ importpath = "example.com/hello",
+ visibility = ["//visibility:public"],
+)
+
`,
filepath.Join(dir, "read", "BUILD.bazel"),
filepath.Join(dir, "write", "BUILD.bazel"))
Expand Down
5 changes: 2 additions & 3 deletions tests/diff_mode/expectedStdout.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- /dev/null 1970-01-01 00:00:00.000000001 +0000
+++ BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
@@ -0,0 +1,10 @@
@@ -0,0 +1,9 @@
+filegroup(
+ name = "all_files",
+ testonly = True,
Expand All @@ -9,5 +9,4 @@
+ "WORKSPACE",
+ ],
+ visibility = ["//visibility:public"],
+)
+
+)
5 changes: 2 additions & 3 deletions tests/loads_from_flag/expectedStdout.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
--- /dev/null 1970-01-01 00:00:00.000000001 +0000
+++ BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
@@ -0,0 +1,4 @@
@@ -0,0 +1,3 @@
+load("@some//repo:rules.bzl", "custom_rule")
+
+custom_rule(name = "gen")
+
+custom_rule(name = "gen")

0 comments on commit 8c64e02

Please sign in to comment.