From 55c2831bbbfe3fc3e48ae989a836053b4cd4edb3 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 8 Oct 2026 16:13:13 -0700 Subject: [PATCH 1/4] fix(targethasher): only hash local FILE lines in bzlmod repo markers readMarkerHash hashed every non-ENV marker line. Some FILE: lines point at files generated during a repo's fetch (e.g. go_repository_cache's go.env and the binaries built by go_repository_tools) and embed the Bazel output_base path. The output_base is derived from the workspace path, so the collapsed hash of every file in such a repo differed between two computes done in different workspaces, flagging the repo and all of its reverse dependencies as changed even though no dependency changed. Hash only the first line (the repo rule's declarative inputs) and FILE: lines for main-repo files (@@// labels, which cover patches). ENV:, REPO_MAPPING: and FILE: lines for other repos are skipped. Co-Authored-By: Claude Sonnet 5.5 --- core/targethasher/BUILD.bazel | 1 + core/targethasher/graph.go | 8 ++- core/targethasher/markers.go | 27 +++++++-- core/targethasher/markers_test.go | 92 +++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 core/targethasher/markers_test.go diff --git a/core/targethasher/BUILD.bazel b/core/targethasher/BUILD.bazel index 7b304093..590517a1 100644 --- a/core/targethasher/BUILD.bazel +++ b/core/targethasher/BUILD.bazel @@ -23,6 +23,7 @@ go_test( srcs = [ "graph_rapid_test.go", "graph_test.go", + "markers_test.go", "mock_sourcehasher_test.go", "sourcehasher_test.go", ], diff --git a/core/targethasher/graph.go b/core/targethasher/graph.go index f37f4330..51a22211 100644 --- a/core/targethasher/graph.go +++ b/core/targethasher/graph.go @@ -438,9 +438,11 @@ func shouldCollapseToBzlmodRepo(target *Target, repo string, fullHashRepos set.S // // Marker files track both the repo rule's declarative inputs (version, // URL, integrity) and per-file SHA-256 content hashes for local patches -// applied via single_version_override. readMarkerHash hashes all stable -// lines (skipping ENV) so the collapsed hash changes on dependency -// upgrades AND patch content modifications. +// applied via single_version_override. readMarkerHash hashes the first line +// and the main-repo FILE: lines only (skipping ENV, REPO_MAPPING and FILE: +// lines for other repos, which can embed the output_base path) so the +// collapsed hash changes on dependency upgrades AND patch content +// modifications. // // Every external repo referenced in the query result should have a // marker file after bazel query completes. Returns an error if a diff --git a/core/targethasher/markers.go b/core/targethasher/markers.go index 3805fb70..aa76fa49 100644 --- a/core/targethasher/markers.go +++ b/core/targethasher/markers.go @@ -29,7 +29,8 @@ import ( // external repo marker files and returns a map from canonical repo name // to a hash derived from the stable lines of each marker file. // This hash changes whenever the repo's version, URL, or patch file -// contents change. ENV lines are excluded for cross-environment stability. +// contents change. See readMarkerHash for the lines that are excluded for +// cross-environment and cross-workspace stability. func ReadRepoMarkerHashes(ctx context.Context, workspacePath, bazelCommand string) (map[string][]byte, error) { outputBase, err := bazel.OutputBase(ctx, workspacePath, bazelCommand) if err != nil { @@ -66,12 +67,27 @@ func ReadRepoMarkerHashes(ctx context.Context, workspacePath, bazelCommand strin return hashes, nil } +// localFilePrefix marks a FILE: marker line whose label is in the main repo +// ("@@//pkg:file"; the main repo's canonical name is empty). These are +// checked-in files such as patches, so their content hash is the same in any +// checkout. +const localFilePrefix = "FILE:@@//" + // readMarkerHash computes a hash from the stable lines of a marker file. // The first line is a hash of the repo rule's declarative inputs (URL, // version, patch paths) but does NOT include the content hashes of patch // files. Those appear on FILE: lines alongside their SHA-256 content -// hashes. ENV: lines are skipped because environment variables can differ -// between CI environments and would cause unnecessary hash instability. +// hashes. +// +// Only the first line and FILE: lines for main-repo files (@@// labels) are +// hashed. Every other line is skipped: +// - ENV: lines, because environment variables can differ between CI hosts. +// - FILE: lines for files in other repos, because those can be generated +// during the fetch and embed the Bazel output_base path (for example +// go_repository_cache's go.env and the binaries built by +// go_repository_tools), which differs between workspaces even when no +// dependency changed. +// - REPO_MAPPING: lines, which only restate canonical repo names. func readMarkerHash(path string) ([]byte, error) { f, err := os.Open(path) if err != nil { @@ -81,13 +97,16 @@ func readMarkerHash(path string) ([]byte, error) { h := newHash() hasContent := false + seenFirstLine := false scanner := bufio.NewScanner(f) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if line == "" { continue } - if strings.HasPrefix(line, "ENV:") { + isFirstLine := !seenFirstLine + seenFirstLine = true + if !isFirstLine && !strings.HasPrefix(line, localFilePrefix) { continue } h.Write([]byte(line)) diff --git a/core/targethasher/markers_test.go b/core/targethasher/markers_test.go new file mode 100644 index 00000000..a6c874f9 --- /dev/null +++ b/core/targethasher/markers_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package targethasher + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testDigest = "fbbfccf5194415fded38959668a5627efc0e02be1bbaa6420edf3c25c057b11a" + testPatchLine = "FILE:@@//patches/protobuf/use_prebuilt_protoc.patch 0f4b98e755d6ad0cee62a8fe8ce36d02fe129936301a34fa59abe9bbd9945a43" + testGoEnvLine = "FILE:@@gazelle++non_module_deps+bazel_gazelle_go_repository_cache//go.env 860dcdd7df8b260d298aa300fed108cda552d3e5420f2565fb742c3183025dbb" + testToolBinary = "FILE:@@gazelle++non_module_deps+bazel_gazelle_go_repository_tools//bin/gazelle 3c218b37322806fdfa4b7e6a090d7fd5db919b07df3dde930dc029c1854a9989" + testSourceFile = "FILE:@@gazelle+//BUILD.bazel eda52fca39edfb09f3c35adf12ad3bde86f4f92a7586b59eebfd3aea" + testMapping = "REPO_MAPPING:gazelle+,bazel_tools bazel_tools" + testEnv = "ENV:GOPATH /Users/someone/go-code" +) + +func markerHash(t *testing.T, lines ...string) []byte { + t.Helper() + path := filepath.Join(t.TempDir(), "repo.marker") + require.NoError(t, os.WriteFile(path, []byte(strings.Join(lines, "\n")+"\n"), 0o644)) + h, err := readMarkerHash(path) + require.NoError(t, err) + return h +} + +func TestReadMarkerHash_IgnoresNonLocalAndEnvLines(t *testing.T) { + base := markerHash(t, testDigest, testPatchLine) + + // Same digest and patch, but with every kind of line that must not count, + // including generated files whose hashes differ between workspaces. + withNoise := markerHash(t, + testDigest, + testGoEnvLine, + testToolBinary, + testSourceFile, + testPatchLine, + testMapping, + testEnv, + ) + assert.Equal(t, base, withNoise) + + otherWorkspace := markerHash(t, + testDigest, + strings.Replace(testGoEnvLine, "860dcdd7", "a0b4be1a", 1), + strings.Replace(testToolBinary, "3c218b37", "fcfe8347", 1), + testPatchLine, + ) + assert.Equal(t, base, otherWorkspace, "path-dependent FILE: lines from other repos must not affect the hash") +} + +func TestReadMarkerHash_ChangesOnRuleInputs(t *testing.T) { + base := markerHash(t, testDigest, testPatchLine) + bumped := markerHash(t, strings.Replace(testDigest, "fbbfccf5", "00000000", 1), testPatchLine) + assert.NotEqual(t, base, bumped, "first line (repo rule inputs) must affect the hash") +} + +func TestReadMarkerHash_ChangesOnPatchContent(t *testing.T) { + base := markerHash(t, testDigest, testPatchLine) + edited := markerHash(t, testDigest, strings.Replace(testPatchLine, "0f4b98e7", "ffffffff", 1)) + assert.NotEqual(t, base, edited, "main-repo FILE: lines (patches) must affect the hash") + + added := markerHash(t, testDigest, testPatchLine, "FILE:@@//patches/protobuf/extra.patch 1111111111111111111111111111111111111111111111111111111111111111") + assert.NotEqual(t, base, added, "adding a patch must affect the hash") +} + +func TestReadMarkerHash_Empty(t *testing.T) { + path := filepath.Join(t.TempDir(), "empty.marker") + require.NoError(t, os.WriteFile(path, []byte("\n \n"), 0o644)) + h, err := readMarkerHash(path) + require.NoError(t, err) + assert.Nil(t, h) +} From 900740982e332eb7fddfa0247c6b2029b4ad5e83 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 8 Oct 2026 16:26:40 -0700 Subject: [PATCH 2/4] fix(targethasher): also hash local DIRTREE and DIRENTS marker inputs Bazel records a repo rule's file-system inputs as KIND:@@//, with FILE, DIRTREE and DIRENTS kinds. Only FILE:@@// lines were treated as local to the main repo, so a checked-in directory input (e.g. a patches directory) would not have affected the marker hash. Treat all three kinds with an @@// path as local. Inputs in other repos and absolute-path inputs stay excluded. Co-Authored-By: Claude Sonnet 5.5 --- core/targethasher/markers.go | 33 +++++++++++++++++++++++-------- core/targethasher/markers_test.go | 26 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/core/targethasher/markers.go b/core/targethasher/markers.go index aa76fa49..cc4033f0 100644 --- a/core/targethasher/markers.go +++ b/core/targethasher/markers.go @@ -67,11 +67,27 @@ func ReadRepoMarkerHashes(ctx context.Context, workspacePath, bazelCommand strin return hashes, nil } -// localFilePrefix marks a FILE: marker line whose label is in the main repo -// ("@@//pkg:file"; the main repo's canonical name is empty). These are -// checked-in files such as patches, so their content hash is the same in any -// checkout. -const localFilePrefix = "FILE:@@//" +// localInputPrefixes mark the marker lines for inputs that live in the main +// repo. Bazel records a repo rule's file-system inputs as KIND:@@// +// (the main repo's canonical name is empty), where KIND is FILE for a file, +// DIRTREE for a directory tree and DIRENTS for a directory listing. Main-repo +// inputs are checked-in sources such as patches, so their hashes are the same +// in any checkout. Inputs in other repos can be generated during the fetch, +// and inputs outside any repo are recorded as absolute paths. +var localInputPrefixes = []string{ + "FILE:@@//", + "DIRTREE:@@//", + "DIRENTS:@@//", +} + +func isLocalInput(line string) bool { + for _, p := range localInputPrefixes { + if strings.HasPrefix(line, p) { + return true + } + } + return false +} // readMarkerHash computes a hash from the stable lines of a marker file. // The first line is a hash of the repo rule's declarative inputs (URL, @@ -79,8 +95,9 @@ const localFilePrefix = "FILE:@@//" // files. Those appear on FILE: lines alongside their SHA-256 content // hashes. // -// Only the first line and FILE: lines for main-repo files (@@// labels) are -// hashed. Every other line is skipped: +// Only the first line and the inputs local to the main repo (FILE:, DIRTREE: +// and DIRENTS: lines with an @@// path, see localInputPrefixes) are hashed. +// Every other line is skipped: // - ENV: lines, because environment variables can differ between CI hosts. // - FILE: lines for files in other repos, because those can be generated // during the fetch and embed the Bazel output_base path (for example @@ -106,7 +123,7 @@ func readMarkerHash(path string) ([]byte, error) { } isFirstLine := !seenFirstLine seenFirstLine = true - if !isFirstLine && !strings.HasPrefix(line, localFilePrefix) { + if !isFirstLine && !isLocalInput(line) { continue } h.Write([]byte(line)) diff --git a/core/targethasher/markers_test.go b/core/targethasher/markers_test.go index a6c874f9..e7b56cd9 100644 --- a/core/targethasher/markers_test.go +++ b/core/targethasher/markers_test.go @@ -83,6 +83,32 @@ func TestReadMarkerHash_ChangesOnPatchContent(t *testing.T) { assert.NotEqual(t, base, added, "adding a patch must affect the hash") } +func TestReadMarkerHash_LocalDirectoryInputs(t *testing.T) { + base := markerHash(t, testDigest, testPatchLine) + + for _, kind := range []string{"DIRTREE", "DIRENTS"} { + local := kind + ":@@//patches/protobuf 1111111111111111111111111111111111111111111111111111111111111111" + withDir := markerHash(t, testDigest, testPatchLine, local) + assert.NotEqual(t, base, withDir, "main-repo %s: lines must affect the hash", kind) + + changed := markerHash(t, testDigest, testPatchLine, strings.Replace(local, "11111111", "22222222", 1)) + assert.NotEqual(t, withDir, changed, "main-repo %s: content must affect the hash", kind) + + other := kind + ":@@gazelle+//cmd 1111111111111111111111111111111111111111111111111111111111111111" + assert.Equal(t, base, markerHash(t, testDigest, testPatchLine, other), + "%s: lines for other repos must not affect the hash", kind) + } +} + +func TestReadMarkerHash_IgnoresAbsolutePathInputs(t *testing.T) { + base := markerHash(t, testDigest, testPatchLine) + // Inputs outside any repo are recorded as absolute paths, which are tied to + // the machine and not to the workspace's checked-in sources. + withAbsolute := markerHash(t, testDigest, testPatchLine, + "FILE:/usr/bin/env 1111111111111111111111111111111111111111111111111111111111111111") + assert.Equal(t, base, withAbsolute) +} + func TestReadMarkerHash_Empty(t *testing.T) { path := filepath.Join(t.TempDir(), "empty.marker") require.NoError(t, os.WriteFile(path, []byte("\n \n"), 0o644)) From e8cb986fcf69d472c0b1441f1b438abe6f999792 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 8 Oct 2026 17:17:54 -0700 Subject: [PATCH 3/4] docs(targethasher): keep the original readMarkerHash comment Co-Authored-By: Claude Sonnet 5.5 --- core/targethasher/markers.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/core/targethasher/markers.go b/core/targethasher/markers.go index cc4033f0..89abce12 100644 --- a/core/targethasher/markers.go +++ b/core/targethasher/markers.go @@ -93,18 +93,9 @@ func isLocalInput(line string) bool { // The first line is a hash of the repo rule's declarative inputs (URL, // version, patch paths) but does NOT include the content hashes of patch // files. Those appear on FILE: lines alongside their SHA-256 content -// hashes. -// -// Only the first line and the inputs local to the main repo (FILE:, DIRTREE: -// and DIRENTS: lines with an @@// path, see localInputPrefixes) are hashed. -// Every other line is skipped: -// - ENV: lines, because environment variables can differ between CI hosts. -// - FILE: lines for files in other repos, because those can be generated -// during the fetch and embed the Bazel output_base path (for example -// go_repository_cache's go.env and the binaries built by -// go_repository_tools), which differs between workspaces even when no -// dependency changed. -// - REPO_MAPPING: lines, which only restate canonical repo names. +// hashes. ENV: lines are skipped because environment variables can differ +// between CI environments and would cause unnecessary hash instability. +// Only inputs local to the main repo are hashed, see localInputPrefixes. func readMarkerHash(path string) ([]byte, error) { f, err := os.Open(path) if err != nil { From 89f363a1d2629830d98a7c61b4b6490b39fe8f72 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 8 Oct 2026 20:15:09 -0700 Subject: [PATCH 4/4] refactor(targethasher): rename isFirstLine to isHeader in readMarkerHash Co-Authored-By: Claude Sonnet 5.5 --- core/targethasher/markers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/targethasher/markers.go b/core/targethasher/markers.go index 89abce12..dd5cd1df 100644 --- a/core/targethasher/markers.go +++ b/core/targethasher/markers.go @@ -105,16 +105,16 @@ func readMarkerHash(path string) ([]byte, error) { h := newHash() hasContent := false - seenFirstLine := false + seenHeader := false scanner := bufio.NewScanner(f) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if line == "" { continue } - isFirstLine := !seenFirstLine - seenFirstLine = true - if !isFirstLine && !isLocalInput(line) { + isHeader := !seenHeader // first line: hash of the repo rule's declarative inputs + seenHeader = true + if !isHeader && !isLocalInput(line) { continue } h.Write([]byte(line))