Skip to content

Commit 67934f9

Browse files
committed
config: remove fields not documented in config/README.md
Drops RepositoryConfig.FullHashRepos/ExcludedFiles/ExcludeExternalTargets/ StreamBazelLogs and ServiceConfig's settable WorkerRootPath override — none are documented in config/README.md, and WorkerRootPath is now always computed by Parse as workspaces_root_path/.workers rather than optionally overridden. graphrunner/native.go's Bzlmod query construction is adjusted so external-target querying no longer depends on the removed ExcludeExternalTargets field. Also fixes make test-integration on macOS: excludes @bazel_tools's platform-specific files (e.g. a Windows-only test wrapper) from disk-based hashing via RequestOptions.ExtraExcludeFilesRegex, since a prior config-side exclusion (excluded_files) was just removed as undocumented.
1 parent 5a43167 commit 67934f9

10 files changed

Lines changed: 29 additions & 61 deletions

File tree

config/config.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,10 @@ func Parse(configFilePath string) (*Config, error) {
7272
if config.Service.WorkspacesRootPath == "" {
7373
return nil, fmt.Errorf("service.workspaces_root_path must be set")
7474
}
75-
if config.Service.WorkerRootPath == "" {
76-
config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRootPath, ".workers")
77-
}
7875
if config.Service.MaxWorkerPoolSize <= 0 {
7976
return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize)
8077
}
78+
config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRootPath, ".workers")
8179
if config.Service.Streaming.MaxNumTargets <= 0 {
8280
config.Service.Streaming.MaxNumTargets = _defaultMaxNumTargets
8381
}

config/config_test.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func TestParse_ServiceDefaults(t *testing.T) {
8080
wantMaxNumMetadataEntries int
8181
}{
8282
{
83-
name: "worker_root_path and streaming default when unset",
83+
name: "streaming defaults when unset",
8484
give: _baseServiceYAML + `
8585
repository:
8686
- remote: "r1"
@@ -90,21 +90,6 @@ repository:
9090
wantMaxNumChangedTargets: _defaultMaxNumChangedTargets,
9191
wantMaxNumMetadataEntries: _defaultMaxNumMetadataEntries,
9292
},
93-
{
94-
name: "worker_root_path explicit value preserved",
95-
give: `
96-
service:
97-
workspaces_root_path: "/tmp/x"
98-
worker_root_path: "/tmp/custom-workers"
99-
max_worker_pool_size: 1
100-
repository:
101-
- remote: "r1"
102-
`,
103-
wantWorkerRootPath: "/tmp/custom-workers",
104-
wantMaxNumTargets: _defaultMaxNumTargets,
105-
wantMaxNumChangedTargets: _defaultMaxNumChangedTargets,
106-
wantMaxNumMetadataEntries: _defaultMaxNumMetadataEntries,
107-
},
10893
{
10994
name: "streaming explicit values preserved",
11095
give: _baseServiceYAML + `

config/repository_config.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,18 @@ type RepositoryConfig struct {
2121
// unique across all entries and match exactly what clients send in
2222
// BuildDescription.remote.
2323
Remote string `yaml:"remote"`
24-
// TODO: FullHashRepos, ExcludedFiles, and ExcludeExternalTargets are not
25-
// documented in config/README.md. Delete them if they turn out to be unneeded,
26-
// otherwise document them there.
27-
FullHashRepos []string `yaml:"full_hash_repos"`
28-
ExcludedFiles []string `yaml:"excluded_files"`
29-
ExcludeExternalTargets bool `yaml:"exclude_external_targets"`
3024
// BzlmodEnabled indicates whether this repository uses Bzlmod for external
3125
// dependency management. Defaults to true if unset. Set to false only for
3226
// repositories still using WORKSPACE.
3327
BzlmodEnabled *bool `yaml:"bzlmod_enabled"`
3428
// BazelCommandPath overrides the Bazel binary path. When empty, Tango
3529
// automatically downloads and caches Bazelisk from GitHub.
3630
BazelCommandPath string `yaml:"bazel_command_path"`
37-
// QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600.
38-
QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"`
3931
// BazelExtraArgs are extra arguments passed to `bazel query` invocations,
4032
// inserted between the `query` subcommand and the query expression.
4133
BazelExtraArgs []string `yaml:"bazel_extra_args"`
42-
// TODO: StreamBazelLogs is not documented in config/README.md. Delete it if
43-
// it turns out to be unneeded, otherwise document it there.
44-
StreamBazelLogs bool `yaml:"stream_bazel_logs"`
34+
// QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600.
35+
QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"`
4536
}
4637

4738
// RepositoryConfigProvider looks up per-repository configuration by remote.

config/service_config.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@ type ServiceConfig struct {
2424
// and worker checkouts. Required. Layout: <workspaces_root_path>/<repo>/ for
2525
// origin clones and <workspaces_root_path>/.workers/<repo>/worker-{1..N}/ for
2626
// worker checkouts.
27-
WorkspacesRootPath string `yaml:"workspaces_root_path"`
28-
// TODO: WorkerRootPath is not documented in config/README.md. Delete it if
29-
// it turns out to be unneeded, otherwise document it there.
30-
WorkerRootPath string `yaml:"worker_root_path"` // root directory for worker workspace checkouts; defaults to workspaces_root_path/.workers
31-
Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults
27+
WorkspacesRootPath string `yaml:"workspaces_root_path"`
28+
Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults
29+
30+
// WorkerRootPath is the root directory for worker workspace checkouts,
31+
// set by Parse to <workspaces_root_path>/.workers. Not settable via YAML.
32+
WorkerRootPath string `yaml:"-"`
3233
}
3334

3435
// ChunkConfig controls the number of entries per gRPC stream message.

example/tango-config.yaml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,6 @@ storage:
99
# Repository configuration
1010
repository:
1111
- remote: "https://github.com/uber/tango.git"
12-
full_hash_repos:
13-
- ""
14-
excluded_files:
15-
- "^@@?bazel_tools/"
16-
exclude_external_targets: true
1712
bzlmod_enabled: true
1813
query_timeout_seconds: 300
1914

@@ -22,5 +17,3 @@ service:
2217
max_worker_pool_size: 5
2318
# root for origin clones; required
2419
workspaces_root_path: "/tmp/tango-repo-manager"
25-
# root for worker checkouts; defaults to workspaces_root_path/.workers
26-
worker_root_path: "/tmp/tango/workers"

graphrunner/native.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@ 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)"
62-
if g.config.ExcludeExternalTargets {
63-
query = "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
6467
}
6568
additionalArgs := append(
6669
[]string{"--order_output=no", "--proto:locations", "--noproto:default_values"},
@@ -74,8 +77,6 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
7477
// --proto:locations: we need to get external file location to make CTC more accurate
7578
// --noproto: parameters exclude fields from the output that are not used for hashing anyways, making
7679
// proto blob smaller and serialization/deserialization faster
77-
// TODO: pass in --enable_workspace or --enable_bzlmod based on the config
78-
7980
AdditionalArgs: additionalArgs,
8081
})
8182
g.scope.Timer("bazel_query_duration").Record(time.Since(bazelStart))
@@ -92,9 +93,8 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
9293

9394
hashConfig := targethasher.HashConfig{
9495
KnownSourceHashes: knownSourceHashes,
95-
FullHashRepos: g.config.FullHashRepos,
96-
ExcludedRegex: append(g.config.ExcludedFiles, g.extraExcludedFiles...),
97-
UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled,
96+
ExcludedRegex: g.extraExcludedFiles,
97+
UseBzlmod: bzlmodEnabled,
9898
}
9999

100100
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

integration/testdata/tango-config.yaml.tmpl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,9 @@ repository:
77
{{- if .BazelCommand}}
88
bazel_command_path: {{.BazelCommand}}
99
{{- end}}
10-
full_hash_repos: [""]
11-
excluded_files: ["^@@?bazel_tools/"]
12-
exclude_external_targets: true
1310
bzlmod_enabled: true
1411
query_timeout_seconds: 600
1512

1613
service:
1714
max_worker_pool_size: 2
1815
workspaces_root_path: {{.ClonePath}}
19-
worker_root_path: {{.WorkerPath}}

orchestrator/native_orchestrator.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT
191191
Logger: b.logger,
192192
BazelCommand: repoCfg.BazelCommandPath,
193193
QueryTimeout: time.Duration(repoCfg.QueryTimeoutSeconds) * time.Second,
194-
StreamLogs: repoCfg.StreamBazelLogs,
195194
})
196195
if err != nil {
197196
return nil, fmt.Errorf("create bazel client: %w", err)

orchestrator/testdata/config.yaml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
repository:
22
- remote: "git@github:uber/tango"
3-
full_hash_repos:
4-
- "//"
5-
- "//external"
6-
excluded_files:
7-
- "*.gen.go"
8-
exclude_external_targets: true
93
bzlmod_enabled: true
104
query_timeout_seconds: 15
115

0 commit comments

Comments
 (0)