Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package config
import (
"fmt"
"os"
"path/filepath"

yaml "github.com/goccy/go-yaml"
)
Expand Down Expand Up @@ -65,9 +64,6 @@ func Parse(configFilePath string) (*Config, error) {
if config.Service.WorkspacesRootPath == "" {
return nil, fmt.Errorf("service.workspaces_root_path must be set")
}
if config.Service.WorkerRootPath == "" {
config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRootPath, ".workers")
}
if config.Service.MaxWorkerPoolSize <= 0 {
return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize)
}
Expand Down
37 changes: 0 additions & 37 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,43 +70,6 @@ repository:
}
}

func TestParse_ServiceDefaults(t *testing.T) {
tests := []struct {
name string
give string
wantWorkerRootPath string
}{
{
name: "worker_root_path defaults when unset",
give: _baseServiceYAML + `
repository:
- remote: "r1"
`,
wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"),
},
{
name: "worker_root_path explicit value preserved",
give: `
service:
workspaces_root_path: "/tmp/x"
worker_root_path: "/tmp/custom-workers"
max_worker_pool_size: 1
repository:
- remote: "r1"
`,
wantWorkerRootPath: "/tmp/custom-workers",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg, err := Parse(writeConfig(t, tt.give))
require.NoError(t, err)
assert.Equal(t, tt.wantWorkerRootPath, cfg.Service.WorkerRootPath)
})
}
}

func TestParse_RepositoryDefaults(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 2 additions & 11 deletions config/repository_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,18 @@ type RepositoryConfig struct {
// unique across all entries and match exactly what clients send in
// BuildDescription.remote.
Remote string `yaml:"remote"`
// TODO: FullHashRepos, ExcludedFiles, and ExcludeExternalTargets are not
// documented in config/README.md. Delete them if they turn out to be unneeded,
// otherwise document them there.
FullHashRepos []string `yaml:"full_hash_repos"`
ExcludedFiles []string `yaml:"excluded_files"`
ExcludeExternalTargets bool `yaml:"exclude_external_targets"`
// BzlmodEnabled indicates whether this repository uses Bzlmod for external
// dependency management. Defaults to true if unset. Set to false only for
// repositories still using WORKSPACE.
BzlmodEnabled *bool `yaml:"bzlmod_enabled"`
// BazelCommandPath overrides the Bazel binary path. When empty, Tango
// automatically downloads and caches Bazelisk from GitHub.
BazelCommandPath string `yaml:"bazel_command_path"`
// QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600.
QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"`
// BazelExtraArgs are extra arguments passed to `bazel query` invocations,
// inserted between the `query` subcommand and the query expression.
BazelExtraArgs []string `yaml:"bazel_extra_args"`
// TODO: StreamBazelLogs is not documented in config/README.md. Delete it if
// it turns out to be unneeded, otherwise document it there.
StreamBazelLogs bool `yaml:"stream_bazel_logs"`
// QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600.
QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"`
}

// RepositoryConfigProvider looks up per-repository configuration by remote.
Expand Down
7 changes: 2 additions & 5 deletions config/service_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@ type ServiceConfig struct {
// and worker checkouts. Required. Layout: <workspaces_root_path>/<repo>/ for
// origin clones and <workspaces_root_path>/.workers/<repo>/worker-{1..N}/ for
// worker checkouts.
WorkspacesRootPath string `yaml:"workspaces_root_path"`
// TODO: WorkerRootPath is not documented in config/README.md. Delete it if
// it turns out to be unneeded, otherwise document it there.
WorkerRootPath string `yaml:"worker_root_path"` // root directory for worker workspace checkouts; defaults to workspaces_root_path/.workers
Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults
WorkspacesRootPath string `yaml:"workspaces_root_path"`
Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults
}

// ChunkConfig controls the number of entries per gRPC stream message.
Expand Down
5 changes: 1 addition & 4 deletions core/repomanager/repo_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type RepoManager interface {
type repoManager struct {
git git.Interface
repoManagerClonePath string
workerRootPath string
logger *zap.SugaredLogger
poolSize int

Expand Down Expand Up @@ -77,7 +76,6 @@ type Params struct {
Git git.Interface
Logger *zap.SugaredLogger
RepoManagerClonePath string
WorkerRootPath string
PoolSize int
}

Expand All @@ -93,7 +91,6 @@ func NewRepoManager(appCtx context.Context, p Params) (RepoManager, error) {
return &repoManager{
git: p.Git,
repoManagerClonePath: p.RepoManagerClonePath,
workerRootPath: p.WorkerRootPath,
logger: p.Logger,
poolSize: p.PoolSize,
pools: make(map[string]*workerPool),
Expand All @@ -115,7 +112,7 @@ func (r *repoManager) poolFor(repo string) *workerPool {

// Pre-allocate fixed worker slots. Existing directories from a previous
// run are detected and reused without re-cloning.
workersDir := filepath.Join(r.workerRootPath, repo)
workersDir := filepath.Join(r.repoManagerClonePath, ".workers", repo)
for i := 1; i <= r.poolSize; i++ {
dir := filepath.Join(workersDir, fmt.Sprintf("worker-%d", i))
slot := &workerSlot{dir: dir}
Expand Down
22 changes: 11 additions & 11 deletions core/repomanager/repo_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestLease_ClonesOriginAndCreatesWorker(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote, originDir, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ws, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.NoError(t, err)
assert.Equal(t, workerDir, ws.Path())
Expand All @@ -81,7 +81,7 @@ func TestLease_SkipsOriginClone_WhenExists(t *testing.T) {
// Only worker clone expected
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ws, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.NoError(t, err)
assert.Equal(t, workerDir, ws.Path())
Expand All @@ -102,7 +102,7 @@ func TestLease_ReusesWorker_AfterRelease(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote, originDir, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ctx := context.Background()

ws1, err := rm.Lease(ctx, entity.BuildDescription{Remote: remote})
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestLease_CreatesMultipleWorkers(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), originDir, dir, "--local", "-c", "gc.auto=0").Return(nil)
}

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 2})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 2})
ctx := context.Background()

ws1, err := rm.Lease(ctx, entity.BuildDescription{Remote: remote})
Expand All @@ -157,7 +157,7 @@ func TestLease_BlocksUntilReturn(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote, originDir, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ctx := context.Background()

ws1, err := rm.Lease(ctx, entity.BuildDescription{Remote: remote})
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestLease_CtxCanceled(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote, originDir, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})

ws1, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.NoError(t, err)
Expand All @@ -225,7 +225,7 @@ func TestLease_OriginCloneFails(t *testing.T) {
remote := "git@github.com:org/repo"
g.EXPECT().Clone(gomock.Any(), remote, filepath.Join(root, "org/repo"), "-c", "gc.auto=0").Return(assert.AnError)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
_, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.Error(t, err)
assert.Contains(t, err.Error(), "clone origin")
Expand All @@ -244,7 +244,7 @@ func TestLease_WorkerCloneFails(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote, originDir, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(assert.AnError)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
_, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.Error(t, err)
assert.Contains(t, err.Error(), "create worker")
Expand All @@ -263,7 +263,7 @@ func TestLease_DiscoversExistingWorker(t *testing.T) {
require.NoError(t, os.MkdirAll(filepath.Join(root, ".workers", "org/repo", "worker-1", ".git"), 0o755))

// No Clone calls — everything already exists
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ws, err := rm.Lease(context.Background(), entity.BuildDescription{Remote: remote})
require.NoError(t, err)
assert.Contains(t, ws.Path(), "worker-1")
Expand All @@ -289,7 +289,7 @@ func TestLease_DifferentRepos_IndependentPools(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), remote2, origin2, "-c", "gc.auto=0").Return(nil)
g.EXPECT().Clone(gomock.Any(), origin2, worker2, "--local", "-c", "gc.auto=0").Return(nil)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ctx := context.Background()

// Both repos can be leased concurrently even with pool size 1
Expand Down Expand Up @@ -322,7 +322,7 @@ func TestLease_WorkerCloneFails_SlotReturnedToPool(t *testing.T) {
g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local", "-c", "gc.auto=0").Return(nil),
)

rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, WorkerRootPath: filepath.Join(root, ".workers"), PoolSize: 1})
rm := newTestRepoManager(t, context.Background(), Params{Git: g, Logger: zap.NewNop().Sugar(), RepoManagerClonePath: root, PoolSize: 1})
ctx := context.Background()

// First attempt fails
Expand Down
6 changes: 0 additions & 6 deletions example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,15 @@ func run() error {

// Repo manager and orchestrator
repoManagerClonePath := cfg.Service.WorkspacesRootPath
workerRootPath := cfg.Service.WorkerRootPath
if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil {
return fmt.Errorf("failed to create repo manager clone path: %w", err)
}
defer os.RemoveAll(repoManagerClonePath)
if err := os.MkdirAll(workerRootPath, 0o755); err != nil {
return fmt.Errorf("failed to create worker root path: %w", err)
}
defer os.RemoveAll(workerRootPath)

rm, err := repomanager.NewRepoManager(appCtx, repomanager.Params{
Git: git.New(repoManagerClonePath, logger),
Logger: logger,
RepoManagerClonePath: repoManagerClonePath,
WorkerRootPath: workerRootPath,
PoolSize: cfg.Service.MaxWorkerPoolSize,
})
if err != nil {
Expand Down
7 changes: 0 additions & 7 deletions example/tango-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ storage:
# Repository configuration
repository:
- remote: "https://github.com/uber/tango.git"
full_hash_repos:
- ""
excluded_files:
- "^@@?bazel_tools/"
exclude_external_targets: true
bzlmod_enabled: true
query_timeout_seconds: 300

Expand All @@ -22,5 +17,3 @@ service:
max_worker_pool_size: 5
# root for origin clones; required
workspaces_root_path: "/tmp/tango-repo-manager"
# root for worker checkouts; defaults to workspaces_root_path/.workers
worker_root_path: "/tmp/tango/workers"
16 changes: 8 additions & 8 deletions graphrunner/native.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner {
}

func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) (targethasher.Result, error) {
query := "//external:all-targets + deps(//...:all-targets)"
if g.config.ExcludeExternalTargets {
query = "deps(//...:all-targets)"
bzlmodEnabled := g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled
query := "deps(//...:all-targets)"
if !bzlmodEnabled {
// //external is only queryable under legacy WORKSPACE resolution;
// Bzlmod repos resolve external deps under @@<module> instead.
query = "//external:all-targets + " + query
}
additionalArgs := append(
[]string{"--order_output=no", "--proto:locations", "--noproto:default_values"},
Expand All @@ -74,8 +77,6 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
// --proto:locations: we need to get external file location to make CTC more accurate
// --noproto: parameters exclude fields from the output that are not used for hashing anyways, making
// proto blob smaller and serialization/deserialization faster
// TODO: pass in --enable_workspace or --enable_bzlmod based on the config

AdditionalArgs: additionalArgs,
})
g.scope.Timer("bazel_query_duration").Record(time.Since(bazelStart))
Expand All @@ -92,9 +93,8 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)

hashConfig := targethasher.HashConfig{
KnownSourceHashes: knownSourceHashes,
FullHashRepos: g.config.FullHashRepos,
ExcludedRegex: append(g.config.ExcludedFiles, g.extraExcludedFiles...),
UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled,
ExcludedRegex: g.extraExcludedFiles,
UseBzlmod: bzlmodEnabled,
}

hashStart := time.Now()
Expand Down
19 changes: 13 additions & 6 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,19 @@ const (
configTemplateFile = "testdata/tango-config.yaml.tmpl"
)

// bazelToolsExcludeRegex excludes @bazel_tools's source files from disk-based hashing.
// It ships platform-specific files (e.g. tools/test/tw.exe, a Windows-only test wrapper)
// that don't exist on every OS, so stat-ing them to hash fails on machines missing them.
var bazelToolsExcludeRegex = []string{"^@@?bazel_tools/"}

func repoRemote(t *testing.T) string {
t.Helper()
remote := os.Getenv("TANGO_REPO_REMOTE")
require.NotEmpty(t, remote, "TANGO_REPO_REMOTE must be set (pass --test_env=TANGO_REPO_REMOTE=... to bazel test)")
return remote
}

func writeConfig(t *testing.T, dir, remote, clonePath, workerPath string) string {
func writeConfig(t *testing.T, dir, remote, clonePath string) string {
t.Helper()

tmpl, err := template.ParseFiles(configTemplateFile)
Expand All @@ -69,12 +74,10 @@ func writeConfig(t *testing.T, dir, remote, clonePath, workerPath string) string
err = tmpl.Execute(f, struct {
Remote string
ClonePath string
WorkerPath string
BazelCommand string
}{
Remote: remote,
ClonePath: clonePath,
WorkerPath: workerPath,
BazelCommand: filepath.Join(remote, "tools", "bazel"),
})
require.NoError(t, err, "failed to render config template")
Expand All @@ -87,9 +90,8 @@ func startServer(t *testing.T, remote string) string {

configDir := t.TempDir()
clonePath := t.TempDir()
workerPath := t.TempDir()

configPath := writeConfig(t, configDir, remote, clonePath, workerPath)
configPath := writeConfig(t, configDir, remote, clonePath)

zl := zaptest.NewLogger(t)
logger := zl.Sugar()
Expand All @@ -103,7 +105,6 @@ func startServer(t *testing.T, remote string) string {
Git: git.New(clonePath, logger),
Logger: logger,
RepoManagerClonePath: clonePath,
WorkerRootPath: workerPath,
PoolSize: 2,
})
require.NoError(t, err, "failed to create repo manager")
Expand Down Expand Up @@ -266,6 +267,9 @@ func getChangedTargets(t *testing.T, client pb.TangoYARPCClient, remote, firstSH
Remote: remote,
BaseSha: secondSHA,
},
RequestOptions: &pb.RequestOptions{
ExtraExcludeFilesRegex: bazelToolsExcludeRegex,
},
})
require.NoError(t, err, "failed to initiate GetChangedTargets stream")

Expand Down Expand Up @@ -361,6 +365,9 @@ func TestIntegration_GetTargetGraph(t *testing.T) {
Remote: remote,
BaseSha: pinnedSHA,
},
RequestOptions: &pb.RequestOptions{
ExtraExcludeFilesRegex: bazelToolsExcludeRegex,
},
})
require.NoError(t, err, "failed to initiate GetTargetGraph stream")

Expand Down
Loading
Loading