bazelbuild / bazelbuild/bazel

Off-by-one error in http archive patch parsing logic with patch created by `git diff -U0`

Open
#18,610 1 comment 0 reactions 0 assignees View on GitHub
help wanted P3 team-ExternalDeps type: bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.