Skip to content

Commit 4dd57fe

Browse files
committed
fix(integration): repair make test-integration after config field removal
Removing RepositoryConfig.ExcludeExternalTargets left the Bazel query always including //external:all-targets, which errors under Bzlmod ("no such package 'external'") since this repo's own config sets bzlmod_enabled: true. Gate the //external:all-targets clause on BzlmodEnabled instead, matching the behavior documented in config/README.md. Removing ExcludedFiles also dropped the "^@@?bazel_tools/" exclusion the integration test relied on to skip hashing @bazel_tools' files, some of which (e.g. tools/test/tw.exe, Windows-only) don't exist on every OS. Restore the equivalent exclusion via the per-request RequestOptions.ExtraExcludeFilesRegex, which still exists precisely for this kind of ad hoc exclusion.
1 parent cfceae5 commit 4dd57fe

2 files changed

Lines changed: 19 additions & 4 deletions

File tree

graphrunner/native.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner {
5858
}
5959

6060
func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) (targethasher.Result, error) {
61-
query := "//external:all-targets + deps(//...:all-targets)"
61+
bzlmodEnabled := g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled
62+
query := "deps(//...:all-targets)"
63+
if !bzlmodEnabled {
64+
// //external is only queryable under legacy WORKSPACE resolution;
65+
// Bzlmod repos resolve external deps under @@<module> instead.
66+
query = "//external:all-targets + " + query
67+
}
6268
additionalArgs := append(
6369
[]string{"--order_output=no", "--proto:locations", "--noproto:default_values"},
6470
g.config.BazelExtraArgs...,
@@ -71,8 +77,6 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
7177
// --proto:locations: we need to get external file location to make CTC more accurate
7278
// --noproto: parameters exclude fields from the output that are not used for hashing anyways, making
7379
// proto blob smaller and serialization/deserialization faster
74-
// TODO: pass in --enable_workspace or --enable_bzlmod based on the config
75-
7680
AdditionalArgs: additionalArgs,
7781
})
7882
g.scope.Timer("bazel_query_duration").Record(time.Since(bazelStart))
@@ -90,7 +94,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
9094
hashConfig := targethasher.HashConfig{
9195
KnownSourceHashes: knownSourceHashes,
9296
ExcludedRegex: g.extraExcludedFiles,
93-
UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled,
97+
UseBzlmod: bzlmodEnabled,
9498
}
9599

96100
hashStart := time.Now()

integration/integration_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ const (
4848
configTemplateFile = "testdata/tango-config.yaml.tmpl"
4949
)
5050

51+
// bazelToolsExcludeRegex excludes @bazel_tools's source files from disk-based hashing.
52+
// It ships platform-specific files (e.g. tools/test/tw.exe, a Windows-only test wrapper)
53+
// that don't exist on every OS, so stat-ing them to hash fails on machines missing them.
54+
var bazelToolsExcludeRegex = []string{"^@@?bazel_tools/"}
55+
5156
func repoRemote(t *testing.T) string {
5257
t.Helper()
5358
remote := os.Getenv("TANGO_REPO_REMOTE")
@@ -266,6 +271,9 @@ func getChangedTargets(t *testing.T, client pb.TangoYARPCClient, remote, firstSH
266271
Remote: remote,
267272
BaseSha: secondSHA,
268273
},
274+
RequestOptions: &pb.RequestOptions{
275+
ExtraExcludeFilesRegex: bazelToolsExcludeRegex,
276+
},
269277
})
270278
require.NoError(t, err, "failed to initiate GetChangedTargets stream")
271279

@@ -361,6 +369,9 @@ func TestIntegration_GetTargetGraph(t *testing.T) {
361369
Remote: remote,
362370
BaseSha: pinnedSHA,
363371
},
372+
RequestOptions: &pb.RequestOptions{
373+
ExtraExcludeFilesRegex: bazelToolsExcludeRegex,
374+
},
364375
})
365376
require.NoError(t, err, "failed to initiate GetTargetGraph stream")
366377

0 commit comments

Comments
 (0)