Off-by-one error in http archive patch parsing logic with patch created by `git diff -U0`
- Dominant language
- Java
- Stars
- 25.8k
- Forks
- 4.6k
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 72
Description
### Description of the bug:
Bazel's patch logic is not able to handle patches created with `git diff -U0`. It appears new content are inserted one line before it's intended location.
### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
Consider the following workspace setup ([patch-bug.zip](https://github.com/bazelbuild/bazel/files/11680720/patch-bug.zip)).
```
> ls
BUILD WORKSPACE bad.patch good.patch
> cat BUILD
> cat WORKSPACE
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = "good_bazel_skylib",
patch_args = ["-p1"],
patches = ["//:good.patch"],
sha256 = "66ffd9315665bfaafc96b52278f57c7e2dd09f5ede279ea6d39b2be471e7e3aa",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.4.2/bazel-skylib-1.4.2.tar.gz",
"https://github.com/bazelbuild/bazel-skylib/releases/download/1.4.2/bazel-skylib-1.4.2.tar.gz",
],
)
http_archive(
name = "bad_bazel_skylib",
patch_args = ["-p1"],
patches = ["//:bad.patch"],
sha256 = "66ffd9315665bfaafc96b52278f57c7e2dd09f5ede279ea6d39b2be471e7e3aa",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.4.2/bazel-skylib-1.4.2.tar.gz",
"https://github.com/bazelbuild/bazel-skylib/releases/download/1.4.2/bazel-skylib-1.4.2.tar.gz",
],
)
> cat bad.patch
diff --git a/BUILD b/BUILD
index 3a0651a..4d2b717 100644
--- a/BUILD
+++ b/BUILD
@@ -16,0 +17 @@ filegroup(
+ tags = ["foo"],
> cat good.patch
diff --git a/BUILD b/BUILD
index 3a0651a..4d2b717 100644
--- a/BUILD
+++ b/BUILD
@@ -16,2 +16,3 @@ exports_files([
filegroup(
+ tags = ["foo"],
name = "test_deps",
```
Good patch is created with `git diff -U1` and bad patch is created with `git diff -U0`. (Note, it does appear to me that line number `16` in bad patch is weird. I am not entirely sure what the semantic should be for this case. Could this be a bug with git instead?)
Then we have
```
> bazel query @bad_bazel_skylib//:all
ERROR: /private/var/tmp/_bazel_tgeng/58da6ac068090c85809c6d5045e5abcc/external/bad_bazel_skylib/BUILD:16:4: indentation error
ERROR: /private/var/tmp/_bazel_tgeng/58da6ac068090c85809c6d5045e5abcc/external/bad_bazel_skylib/BUILD:16:20: syntax error at 'outdent': expected expression
ERROR: package contains errors: : indentation error
ERROR: Evaluation of query "@bad_bazel_skylib//:all" failed
Loading: 0 packages loaded
> bazel query @good_bazel_skylib//:all
@good_bazel_skylib//:bzl_library
@good_bazel_skylib//:distribution
@good_bazel_skylib//:lib
@good_bazel_skylib//:test_deps
@good_bazel_skylib//:version
@good_bazel_skylib//:workspace
Loading: 0 packages loaded
```
Specifically, with the bad patch, the content of BUILD file becomes the following, where the inserted line is at a wrong location.
```
exports_files([
"LICENSE",
"MODULE.bazel",
"WORKSPACE",
])
tags = ["foo"],
filegroup(
name = "test_deps",
testonly = True,
srcs = [
"BUILD",
"//lib:test_deps",
"//rules:test_deps",
"//toolchains/unittest:test_deps",
] + glob(["*.bzl"]),
)
```
### Which operating system are you running Bazel on?
macOS
### What is the output of `bazel info release`?
release 6.2.1
### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.
_No response_
### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?
_No response_
### Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.
_No response_
### Have you found anything relevant by searching the web?
_No response_
### Any other information, logs, or outputs that you want to share?
_No response_
Contributor guide
Research direction
Start with the attached patch-bug.zip workspace and compare BUILD, WORKSPACE, bad.patch, and good.patch while running the two bazel query commands. Trace Bazel's HTTP archive patch application for the -U0 hunk and verify that the inserted line lands at the intended location without errors.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100