Skip to content
Closed
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
23 changes: 23 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"bytes"
"fmt"
"os"
"strings"

yaml "github.com/goccy/go-yaml"
)
Expand Down Expand Up @@ -105,6 +106,12 @@ func ParseBytes(yamlBytes []byte) (*Config, error) {
if config.Repository[i].BzlmodEnabled == nil {
config.Repository[i].BzlmodEnabled = &_bzlmodEnabledDefault
}
if err := validateBazelDependencyModeArgs(i, "bazel_extra_args", config.Repository[i].BazelExtraArgs); err != nil {
return nil, err
}
if err := validateBazelDependencyModeArgs(i, "bazel_startup_options", config.Repository[i].BazelStartupOptions); err != nil {
return nil, err
}
// Default query_timeout_seconds: 600 seconds (10 minutes).
if config.Repository[i].QueryTimeoutSeconds <= 0 {
config.Repository[i].QueryTimeoutSeconds = DefaultQueryTimeoutSeconds
Expand All @@ -113,3 +120,19 @@ func ParseBytes(yamlBytes []byte) (*Config, error) {
}
return &config, nil
}

func validateBazelDependencyModeArgs(repositoryIndex int, field string, args []string) error {
for _, arg := range args {
flag, _, _ := strings.Cut(arg, "=")
switch flag {
case "--enable_bzlmod", "--noenable_bzlmod", "--enable_workspace", "--noenable_workspace":
return fmt.Errorf(
"repository[%d].%s must not set Bazel dependency mode flag %q; use bzlmod_enabled",
repositoryIndex,
field,
arg,
)
}
}
return nil
}
27 changes: 27 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,33 @@ service:
assert.Equal(t, 1000000, cfg.Service.MaxMessageBytes)
}

func TestParseBytes_RejectsBazelDependencyModeArgs(t *testing.T) {
tests := []struct {
name string
field string
flag string
}{
{name: "Bzlmod enabled in query args", field: "bazel_extra_args", flag: "--enable_bzlmod=true"},
{name: "Bzlmod disabled in query args", field: "bazel_extra_args", flag: "--noenable_bzlmod"},
{name: "WORKSPACE enabled in query args", field: "bazel_extra_args", flag: "--enable_workspace"},
{name: "WORKSPACE disabled in startup options", field: "bazel_startup_options", flag: "--noenable_workspace"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
yamlStr := `
repository:
- remote: "https://example.com/repo.git"
` + tt.field + `: ["` + tt.flag + `"]
service:
max_worker_pool_size: 1
workspaces_root_path: "/tmp/tango-repo-manager"
`
_, err := ParseBytes([]byte(yamlStr))
require.Error(t, err)
})
}
}

func TestParseBytes_StorageValidation(t *testing.T) {
tests := []struct {
name string
Expand Down
6 changes: 4 additions & 2 deletions config/repository_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ type RepositoryConfig struct {
FullHashRepos []string `yaml:"full_hash_repos"`
ExcludedFiles []string `yaml:"excluded_files"`
// 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.
// dependency management. Tango explicitly disables WORKSPACE when true and
// enables WORKSPACE while disabling Bzlmod when false. Defaults to true if
// unset.
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"`
// BazelExtraArgs are extra arguments passed to `bazel query` invocations,
// inserted between the `query` subcommand and the query expression.
// Dependency-mode flags are reserved; use BzlmodEnabled instead.
BazelExtraArgs []string `yaml:"bazel_extra_args"`
// BazelStartupOptions are Bazel startup flags placed before the `query` subcommand (e.g. "--batch"); empty by default.
BazelStartupOptions []string `yaml:"bazel_startup_options"`
Expand Down
39 changes: 39 additions & 0 deletions core/bazel/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,45 @@ func TestExecuteQuery_WithStartupOptions(t *testing.T) {
}, capturedArgs)
}

func TestSetupCommand_ConstructsBazelDependencyModeArguments(t *testing.T) {
tests := []struct {
name string
modeArgs []string
}{
{
name: "Bzlmod",
modeArgs: []string{"--enable_bzlmod=true", "--enable_workspace=false"},
},
{
name: "WORKSPACE",
modeArgs: []string{"--enable_bzlmod=false", "--enable_workspace=true"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var capturedArgs []string
client, err := NewBazelClient(context.Background(), Params{
BazelCommand: "bazel",
WorkspacePath: "/tmp/test",
EnvVarsMap: map[string]string{},
Logger: zap.NewNop(),
ExecCommandContext: func(_ context.Context, _ string, arg ...string) commander {
capturedArgs = arg
return nil
},
})
require.NoError(t, err)

client.setupCommand(context.Background(), "//...", []string{"--batch"}, tt.modeArgs...)

expected := []string{"--batch", "query"}
expected = append(expected, tt.modeArgs...)
expected = append(expected, "--output=streamed_proto", "//...")
assert.Equal(t, expected, capturedArgs)
})
}
}

func TestExecuteQueryInternal_DrainsStreamsBeforeWait(t *testing.T) {
defer goleak.VerifyNone(t)
ctrl := gomock.NewController(t)
Expand Down
1 change: 1 addition & 0 deletions graphrunner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_test(
srcs = ["native_test.go"],
embed = [":graphrunner"],
deps = [
"//config",
"//core/bazel",
"//core/bazel/bazelmock",
"//core/git/gitmock",
Expand Down
42 changes: 29 additions & 13 deletions graphrunner/native.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ type NativeGraphRunnerParams struct {
Scope tally.Scope
}

type bazelDependencyMode struct {
useBzlmod bool
query string
queryArgs [2]string
}

func resolveBazelDependencyMode(enabled *bool) bazelDependencyMode {
if enabled == nil || *enabled {
return bazelDependencyMode{
useBzlmod: true,
query: "deps(//...:all-targets)",
queryArgs: [2]string{"--enable_bzlmod=true", "--enable_workspace=false"},
}
}
return bazelDependencyMode{
query: "//external:all-targets + deps(//...:all-targets)",
queryArgs: [2]string{"--enable_bzlmod=false", "--enable_workspace=true"},
}
}

// graph runner takes in a bazel query request and computes the graph
func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner {
return &nativeGraphRunner{
Expand All @@ -58,21 +78,17 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
op := metrics.Begin(g.emitter, _opCompute, metrics.SlowDurationBuckets)
defer func() { op.Complete(retErr) }()

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"},
g.config.BazelExtraArgs...,
)
mode := resolveBazelDependencyMode(g.config.BzlmodEnabled)
additionalArgs := make([]string, 0, 3+len(g.config.BazelExtraArgs)+len(mode.queryArgs))
additionalArgs = append(additionalArgs, "--order_output=no", "--proto:locations", "--noproto:default_values")
additionalArgs = append(additionalArgs, g.config.BazelExtraArgs...)
// Keep Tango's selected dependency mode last so programmatically-created
// configs cannot override it with BazelExtraArgs.
additionalArgs = append(additionalArgs, mode.queryArgs[:]...)

bazelStart := time.Now()
queryResult, err := g.bazel.ExecuteQuery(ctx, &bazel.QueryRequest{
Query: query,
Query: mode.query,
StartupOptions: g.config.BazelStartupOptions,
// --order_output=no will make Bazel execute query faster
// --proto:locations: we need to get external file location to make CTC more accurate
Expand Down Expand Up @@ -100,7 +116,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
KnownSourceHashes: knownSourceHashes,
FullHashRepos: g.config.FullHashRepos,
ExcludedRegex: excludedRegex,
UseBzlmod: bzlmodEnabled,
UseBzlmod: mode.useBzlmod,
AllTargetsFiles: g.config.AllTargetsFiles,
}

Expand Down
100 changes: 100 additions & 0 deletions graphrunner/native_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
buildpb "github.com/bazelbuild/buildtools/build_proto"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber/tango/config"
"github.com/uber/tango/core/bazel"
"github.com/uber/tango/core/bazel/bazelmock"
gitmock "github.com/uber/tango/core/git/gitmock"
Expand Down Expand Up @@ -62,6 +63,105 @@ func TestCompute_CallsBazelAndReturnsResult(t *testing.T) {
assert.Equal(t, ruleClass, res.Targets[ruleName].Rule.GetRuleClass())
}

func TestCompute_ConfiguresBazelDependencyMode(t *testing.T) {
enabled := true
disabled := false
tests := []struct {
name string
bzlmodEnabled *bool
expectedMode bazelDependencyMode
expectedQuery string
expectedArgs []string
}{
{
name: "defaults to Bzlmod",
expectedMode: bazelDependencyMode{
useBzlmod: true,
query: "deps(//...:all-targets)",
queryArgs: [2]string{"--enable_bzlmod=true", "--enable_workspace=false"},
},
expectedQuery: "deps(//...:all-targets)",
expectedArgs: []string{
"--order_output=no",
"--proto:locations",
"--noproto:default_values",
"--keep_going",
"--enable_bzlmod=true",
"--enable_workspace=false",
},
},
{
name: "explicitly selects Bzlmod",
bzlmodEnabled: &enabled,
expectedMode: bazelDependencyMode{
useBzlmod: true,
query: "deps(//...:all-targets)",
queryArgs: [2]string{"--enable_bzlmod=true", "--enable_workspace=false"},
},
expectedQuery: "deps(//...:all-targets)",
expectedArgs: []string{
"--order_output=no",
"--proto:locations",
"--noproto:default_values",
"--keep_going",
"--enable_bzlmod=true",
"--enable_workspace=false",
},
},
{
name: "explicitly selects WORKSPACE",
bzlmodEnabled: &disabled,
expectedMode: bazelDependencyMode{
query: "//external:all-targets + deps(//...:all-targets)",
queryArgs: [2]string{"--enable_bzlmod=false", "--enable_workspace=true"},
},
expectedQuery: "//external:all-targets + deps(//...:all-targets)",
expectedArgs: []string{
"--order_output=no",
"--proto:locations",
"--noproto:default_values",
"--keep_going",
"--enable_bzlmod=false",
"--enable_workspace=true",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expectedMode, resolveBazelDependencyMode(tt.bzlmodEnabled))

ctrl := gomock.NewController(t)
bazelMock := bazelmock.NewMockBazel(ctrl)
gitMock := gitmock.NewMockInterface(ctrl)
gitMock.EXPECT().FileHashes(gomock.Any(), "HEAD").Return(map[string][]byte{}, nil)
bazelMock.EXPECT().ExecuteQuery(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, req *bazel.QueryRequest) (*bazel.QueryResponse, error) {
assert.Equal(t, tt.expectedQuery, req.Query)
assert.Equal(t, []string{"--batch"}, req.StartupOptions)
assert.Equal(t, tt.expectedArgs, req.AdditionalArgs)
return &bazel.QueryResponse{Result: &buildpb.QueryResult{}}, nil
},
)

gr := NewNativeGraphRunner(NativeGraphRunnerParams{
BazelClient: bazelMock,
GitClient: gitMock,
Config: config.RepositoryConfig{
BzlmodEnabled: tt.bzlmodEnabled,
BazelExtraArgs: []string{"--keep_going"},
BazelStartupOptions: []string{"--batch"},
},
})
ws := workspace.NewWorkspace(workspace.WorkspaceParams{Path: t.TempDir()})

res, err := gr.Compute(context.Background(), ws)
require.NoError(t, err)
assert.Empty(t, res.Targets)
})
}
}

func TestCompute_PropagatesError(t *testing.T) {
ctrl := gomock.NewController(t)
bazelMock := bazelmock.NewMockBazel(ctrl)
Expand Down