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..dd5cd1df 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,35 @@ func ReadRepoMarkerHashes(ctx context.Context, workspacePath, bazelCommand strin return hashes, nil } +// 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, // 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. +// 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 { @@ -81,13 +105,16 @@ func readMarkerHash(path string) ([]byte, error) { h := newHash() hasContent := false + seenHeader := false scanner := bufio.NewScanner(f) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if line == "" { continue } - if strings.HasPrefix(line, "ENV:") { + isHeader := !seenHeader // first line: hash of the repo rule's declarative inputs + seenHeader = true + if !isHeader && !isLocalInput(line) { 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..e7b56cd9 --- /dev/null +++ b/core/targethasher/markers_test.go @@ -0,0 +1,118 @@ +// 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_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)) + h, err := readMarkerHash(path) + require.NoError(t, err) + assert.Nil(t, h) +}