diff --git a/rules/builtins.build_defs b/rules/builtins.build_defs index 5edca910d..b8e4ccafb 100644 --- a/rules/builtins.build_defs +++ b/rules/builtins.build_defs @@ -3,9 +3,9 @@ # Do not change the order of arguments to this function without updating the iota in targets.go to match it. def build_rule(name:str, cmd:str|dict='', test_cmd:str|dict='', debug_cmd:str='', srcs:list|dict=None, data:list|dict=None, - debug_data:list|dict=None, outs:list|dict=None, deps:list=None, exported_deps:list=None, secrets:list|dict=None, - tools:str|list|dict=None, test_tools:str|list|dict=None, debug_tools:str|list|dict=None, labels:list=None, - visibility:list=CONFIG.DEFAULT_VISIBILITY, hashes:list=None, binary:bool=False, test:bool=False, + debug_data:list|dict=None, outs:list|dict=None, deps:list=None, exported_deps:list=None, runtime_deps:list=None, + secrets:list|dict=None, tools:str|list|dict=None, test_tools:str|list|dict=None, debug_tools:str|list|dict=None, + labels:list=None, visibility:list=CONFIG.DEFAULT_VISIBILITY, hashes:list=None, binary:bool=False, test:bool=False, test_only:bool=CONFIG.DEFAULT_TESTONLY, building_description:str=None, needs_transitive_deps:bool=False, output_is_complete:bool=False, sandbox:bool=CONFIG.BUILD_SANDBOX, test_sandbox:bool=CONFIG.TEST_SANDBOX, no_test_output:bool=False, flaky:bool|int=0, build_timeout:int|str=0, test_timeout:int|str=0, pre_build:function=None, @@ -249,10 +249,12 @@ def has_label(name:str, prefix:str, all:bool=False) -> bool: return len(get_labels(name, prefix, all)) > 0 def add_label(name:str, label:str): pass -def add_dep(target:str, dep:str, exported:bool=False): +def add_dep(target:str, dep:str, exported:bool=False, runtime:bool=False): pass def add_exported_dep(target:str, dep:str): - add_dep(target, dep, True) + add_dep(target, dep, exported=True) +def add_runtime_dep(target:str, dep:str): + add_dep(target, dep, runtime=True) def add_data(target:str, datum:str|list|dict): pass def add_out(target:str, name:str, out:str=''): diff --git a/rules/misc_rules.build_defs b/rules/misc_rules.build_defs index 3d9a260eb..6992313db 100644 --- a/rules/misc_rules.build_defs +++ b/rules/misc_rules.build_defs @@ -2,7 +2,7 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs:list|dict=None, deps:list=None, - exported_deps:list=None, labels:list&features&tags=None, visibility:list=None, + exported_deps:list=None, runtime_deps:list=None, labels:list&features&tags=None, visibility:list=None, building_description:str='Building...', data:list|dict=None, hashes:list=None, timeout:int=0, binary:bool=False, sandbox:bool=None, needs_transitive_deps:bool=False, output_is_complete:bool=True, test_only:bool&testonly=False, secrets:list|dict=None, requires:list=None, provides:dict=None, @@ -34,6 +34,13 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs names to lists, with similar semantics to those of srcs. deps (list): Dependencies of this rule. exported_deps (list): Dependencies that will become visible to any rules that depend on this rule. + runtime_deps (list): Run-time dependencies of this rule. If this rule is run (i.e. with 'plz run'), + rules in this list, as well as those rules' transitive run-time dependencies, + are guaranteed to be built before this rule runs. If this rule is declared as + a dependency of another rule, the outputs of rules in this list, as well as + the outputs of those rules' transitive run-time dependencies, will exist in + the dependent rule's build environment. Requires the rule to produce a runnable + output (i.e. binary = True). tools (str | list | dict): Tools used to build this rule; similar to srcs but are not copied to the temporary build directory. Should be accessed via $(exe //path/to:tool) or similar. @@ -119,6 +126,7 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs cmd = ' && '.join(cmd) if isinstance(cmd, list) else cmd, deps = deps, exported_deps = exported_deps, + runtime_deps = runtime_deps, data = data, tools = tools, secrets = secrets, @@ -147,11 +155,12 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs def gentest(name:str, test_cmd:str|list|dict, labels:list&features&tags=None, cmd:str|list|dict=None, srcs:list|dict=None, - outs:list=None, deps:list=None, exported_deps:list=None, tools:str|list|dict=None, test_tools:str|list|dict=None, - data:list|dict=None, visibility:list=None, timeout:int=0, needs_transitive_deps:bool=False, - flaky:bool|int=0, secrets:list|dict=None, no_test_output:bool=False, test_outputs:list=None, - output_is_complete:bool=True, requires:list=None, sandbox:bool=None, size:str=None, local:bool=False, - pass_env:list=None, env:dict=None, exit_on_error:bool=CONFIG.EXIT_ON_ERROR, no_test_coverage:bool=False): + outs:list=None, deps:list=None, exported_deps:list=None, runtime_deps:list=None, tools:str|list|dict=None, + test_tools:str|list|dict=None, data:list|dict=None, visibility:list=None, timeout:int=0, + needs_transitive_deps:bool=False, flaky:bool|int=0, secrets:list|dict=None, no_test_output:bool=False, + test_outputs:list=None, output_is_complete:bool=True, requires:list=None, sandbox:bool=None, size:str=None, + local:bool=False, pass_env:list=None, env:dict=None, exit_on_error:bool=CONFIG.EXIT_ON_ERROR, + no_test_coverage:bool=False): """A rule which creates a test with an arbitrary command. The command must return zero on success and nonzero on failure. Test results are written @@ -172,6 +181,10 @@ def gentest(name:str, test_cmd:str|list|dict, labels:list&features&tags=None, cm outs (list): Output files of this rule. deps (list): Dependencies of this rule. exported_deps (list): Dependencies that will become visible to any rules that depend on this rule. + runtime_deps (list): Run-time dependencies of this rule. When the test command runs, rules in this + list, as well as those rules' transitive run-time dependencies, will exist in + the test environment. Requires the rule to produce a runnable output (i.e. + binary = True). tools (str | list | dict): Tools used to build this rule; similar to srcs but are not copied to the temporary build directory. test_tools (str | list | dict): Like tools but available to test_cmd instead. @@ -210,6 +223,7 @@ def gentest(name:str, test_cmd:str|list|dict, labels:list&features&tags=None, cm outs = outs, deps = deps, exported_deps = exported_deps, + runtime_deps = runtime_deps, data = data, tools = tools, test_tools = test_tools, @@ -256,7 +270,7 @@ def export_file(name:str, src:str, visibility:list=None, binary:bool=False, test ) -def filegroup(name:str, tag:str='', srcs:list=None, deps:list=None, exported_deps:list=None, +def filegroup(name:str, tag:str='', srcs:list=None, deps:list=None, exported_deps:list=None, runtime_deps:list=None, visibility:list=None, labels:list&features&tags=None, binary:bool=False, output_is_complete:bool=True, requires:list=None, provides:dict=None, hashes:list=None, test_only:bool&testonly=False): """Defines a collection of files which other rules can depend on. @@ -271,6 +285,8 @@ def filegroup(name:str, tag:str='', srcs:list=None, deps:list=None, exported_dep srcs (list): Source files for the rule. deps (list): Dependencies of the rule. exported_deps (list): Dependencies that will become visible to any rules that depend on this rule. + runtime_deps (list): Run-time dependencies of this rule. Requires the rule to produce a runnable + output (i.e. binary = True). visibility (list): Visibility declaration labels (list): Labels to apply to this rule binary (bool): True to mark the rule outputs as binary @@ -289,6 +305,7 @@ def filegroup(name:str, tag:str='', srcs:list=None, deps:list=None, exported_dep srcs=srcs, deps=deps, exported_deps=exported_deps, + runtime_deps=runtime_deps, visibility=visibility, building_description='Copying...', output_is_complete=output_is_complete, @@ -373,7 +390,7 @@ def system_library(name:str, srcs:list, deps:list=None, hashes:list=None, def remote_file(name:str, url:str|list, hashes:list=None, out:str=None, binary:bool=False, visibility:list=None, licences:list=None, test_only:bool&testonly=False, - labels:list=[], deps:list=None, exported_deps:list=None, + labels:list=[], deps:list=None, exported_deps:list=None, runtime_deps:list=None, extract:bool=False, strip_prefix:str='', _tag:str='',exported_files=[], entry_points:dict={}, username:str=None, password_file:str=None, headers:dict={}, secret_headers:dict={}, pass_env:list=[]): @@ -392,6 +409,8 @@ def remote_file(name:str, url:str|list, hashes:list=None, out:str=None, binary:b labels (list): Labels to apply to this rule. deps (list): List of extra dependencies for this rule. exported_deps (list): Dependencies that will become visible to any rules that depend on this rule. + runtime_deps (list): Run-time dependencies of this rule. Requires the rule to produce a runnable + output (i.e. binary = True). extract (bool): Extracts the contents of the downloaded file. It must be either zip or tar format. strip_prefix (str): When extracting, strip this prefix from the extracted files. @@ -454,6 +473,7 @@ def remote_file(name:str, url:str|list, hashes:list=None, out:str=None, binary:b building_description = 'Extracting...', deps = deps, exported_deps = exported_deps, + runtime_deps = runtime_deps, entry_points = entry_points, ) @@ -484,6 +504,7 @@ def remote_file(name:str, url:str|list, hashes:list=None, out:str=None, binary:b building_description = 'Fetching...', deps = deps, exported_deps = exported_deps, + runtime_deps = runtime_deps, test_only = test_only, labels = labels, sandbox = False, diff --git a/src/build/incrementality_test.go b/src/build/incrementality_test.go index a2ce75cdf..e57cc709a 100644 --- a/src/build/incrementality_test.go +++ b/src/build/incrementality_test.go @@ -81,9 +81,10 @@ var KnownFields = map[string]bool{ "Debug.namedTools": true, // These only contribute to the runtime hash, not at build time. - "Data": true, - "NamedData": true, - "ContainerSettings": true, + "runtimeDependencies": true, + "Data": true, + "NamedData": true, + "ContainerSettings": true, // These would ideally not contribute to the hash, but we need that at present // because we don't have a good way to force a recheck of its reverse dependencies. diff --git a/src/core/build_target.go b/src/core/build_target.go index 18dc8ed2f..3e2b8b32b 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -115,6 +115,8 @@ type BuildTarget struct { // Maps the original declaration to whatever dependencies actually got attached, // which may be more than one in some cases. Also contains info about exporting etc. dependencies []depInfo `name:"deps"` + // The run-time dependencies of this target. + runtimeDependencies []BuildLabel `name:"runtime_deps"` // List of build target patterns that can use this build target. Visibility []BuildLabel // Source files of this rule. Can refer to build rules themselves. @@ -307,6 +309,7 @@ type depInfo struct { resolved bool // has the graph resolved it exported bool // is it an exported dependency internal bool // is it an internal dependency (that is not picked up implicitly by transitive searches) + runtime bool // is it a run-time (and therefore implicitly transitive) dependency source bool // is it implicit because it's a source (not true if it's a dependency too) data bool // is it a data item for a test } @@ -632,7 +635,7 @@ func (target *BuildTarget) DeclaredDependenciesStrict() []BuildLabel { defer target.mutex.RUnlock() ret := make(BuildLabels, 0, len(target.dependencies)) for _, dep := range target.dependencies { - if !dep.exported && !dep.source && !target.IsTool(*dep.declared) { + if !dep.runtime && !dep.exported && !dep.source && !target.IsTool(*dep.declared) { ret = append(ret, *dep.declared) } } @@ -672,13 +675,13 @@ func (target *BuildTarget) ExternalDependencies() []*BuildTarget { return ret } -// BuildDependencies returns the build-time dependencies of this target (i.e. not data, internal nor source). +// BuildDependencies returns the build-time dependencies of this target (i.e. not run-time dependencies, data, internal nor source). func (target *BuildTarget) BuildDependencies() []*BuildTarget { target.mutex.RLock() defer target.mutex.RUnlock() ret := make(BuildTargets, 0, len(target.dependencies)) for _, deps := range target.dependencies { - if !deps.data && !deps.internal && !deps.source { + if !deps.runtime && !deps.data && !deps.internal && !deps.source { for _, dep := range deps.deps { ret = append(ret, dep) } @@ -701,6 +704,52 @@ func (target *BuildTarget) ExportedDependencies() []BuildLabel { return ret } +// RuntimeDependencies returns any run-time dependencies of this target. +// +// Although run-time dependencies are transitive, RuntimeDependencies only returns this target's direct run-time +// dependencies. Use IterAllRuntimeDependencies to iterate over the target's run-time dependencies transitively. +func (target *BuildTarget) RuntimeDependencies() []BuildLabel { + target.mutex.RLock() + defer target.mutex.RUnlock() + ret := make(BuildLabels, 0, len(target.dependencies)) + for _, deps := range target.dependencies { + if deps.runtime { + ret = append(ret, *deps.declared) + } + } + return ret +} + +// IterAllRuntimeDependencies returns an iterator over the transitive run-time dependencies of this target. +// Require/provide relationships between pairs of targets are resolved as they are with build-time dependencies. +func (target *BuildTarget) IterAllRuntimeDependencies(graph *BuildGraph) iter.Seq[BuildLabel] { + var ( + push func(*BuildTarget, func(BuildLabel) bool) bool + done = make(map[string]bool) + ) + push = func(t *BuildTarget, yield func(BuildLabel) bool) bool { + if done[t.String()] { + return true + } + done[t.String()] = true + for _, runDep := range t.runtimeDependencies { + runDepLabel, _ := runDep.Label() + for _, providedDep := range graph.TargetOrDie(runDepLabel).ProvideFor(t) { + if !yield(providedDep) { + return false + } + if !push(graph.TargetOrDie(providedDep), yield) { + return false + } + } + } + return true + } + return func(yield func(BuildLabel) bool) { + push(target, yield) + } +} + // DependenciesFor returns the dependencies that relate to a given label. func (target *BuildTarget) DependenciesFor(label BuildLabel) []*BuildTarget { target.mutex.RLock() @@ -1287,7 +1336,7 @@ func (target *BuildTarget) addSource(sources []BuildInput, source BuildInput) [] } // Add a dependency if this is not just a file. if label, ok := source.Label(); ok { - target.AddMaybeExportedDependency(label, false, true, false) + target.AddMaybeExportedDependency(label, false, true, false, false) } return append(sources, source) } @@ -1653,7 +1702,7 @@ func (target *BuildTarget) AllNamedTools() map[string][]BuildInput { // AddDependency adds a dependency to this target. It deduplicates against any existing deps. func (target *BuildTarget) AddDependency(dep BuildLabel) { - target.AddMaybeExportedDependency(dep, false, false, false) + target.AddMaybeExportedDependency(dep, false, false, false, false) } // HintDependencies allocates space for at least the given number of dependencies without reallocating. @@ -1662,17 +1711,30 @@ func (target *BuildTarget) HintDependencies(n int) { } // AddMaybeExportedDependency adds a dependency to this target which may be exported. It deduplicates against any existing deps. -func (target *BuildTarget) AddMaybeExportedDependency(dep BuildLabel, exported, source, internal bool) { +func (target *BuildTarget) AddMaybeExportedDependency(dep BuildLabel, exported, source, internal, runtime bool) { if dep == target.Label { log.Fatalf("Attempted to add %s as a dependency of itself.\n", dep) } + if runtime { + if !target.IsBinary { + log.Fatalf("%s: output must be marked as binary to have run-time dependencies", target.String()) + } + target.runtimeDependencies = append(target.runtimeDependencies, dep) + } info := target.dependencyInfo(dep) if info == nil { - target.dependencies = append(target.dependencies, depInfo{declared: &dep, exported: exported, source: source, internal: internal}) + target.dependencies = append(target.dependencies, depInfo{ + declared: &dep, + exported: exported, + source: source, + internal: internal, + runtime: runtime, + }) } else { info.exported = info.exported || exported info.source = info.source && source info.internal = info.internal && internal + info.runtime = info.runtime && runtime info.data = false // It's not *only* data any more. } } diff --git a/src/core/build_target_test.go b/src/core/build_target_test.go index d9a6cea38..4360be131 100644 --- a/src/core/build_target_test.go +++ b/src/core/build_target_test.go @@ -4,6 +4,7 @@ package core import ( "fmt" "os" + "slices" "testing" "github.com/stretchr/testify/assert" @@ -263,7 +264,7 @@ func TestAddDatum(t *testing.T) { assert.Equal(t, target1.Data, []BuildInput{target2.Label}) assert.True(t, target1.dependencies[0].data) // Now we add it as a dependency too, which unsets the data label - target1.AddMaybeExportedDependency(target2.Label, false, false, false) + target1.AddMaybeExportedDependency(target2.Label, false, false, false, false) assert.False(t, target1.dependencies[0].data) } @@ -427,20 +428,64 @@ func TestBuildDependencies(t *testing.T) { target1 := makeTarget1("//src/core:target1", "") target2 := makeTarget1("//src/core:target2", "", target1) target3 := makeTarget1("//src/core:target3", "", target2) + target4 := makeTarget1("//src/core:target4", "") + target5 := makeTarget1("//src/core:target5", "") target3.AddDatum(target1.Label) + // BuildDependencies shouldn't return run-time dependencies: + target5.IsBinary = true + target5.AddMaybeExportedDependency(target4.Label, false, false, false, true) // runtime assert.Equal(t, []*BuildTarget{}, target1.BuildDependencies()) assert.Equal(t, []*BuildTarget{target1}, target2.BuildDependencies()) assert.Equal(t, []*BuildTarget{target2}, target3.BuildDependencies()) + assert.Equal(t, []*BuildTarget{}, target5.BuildDependencies()) } func TestDeclaredDependenciesStrict(t *testing.T) { target1 := makeTarget1("//src/core:target1", "") target2 := makeTarget1("//src/core:target2", "", target1) target3 := makeTarget1("//src/core:target3", "", target2) - target3.AddMaybeExportedDependency(target1.Label, true, false, false) + target4 := makeTarget1("//src/core:target4", "") + target5 := makeTarget1("//src/core:target5", "") + target3.AddMaybeExportedDependency(target1.Label, true, false, false, false) + // DeclaredDependenciesStrict shouldn't return run-time dependencies: + target5.IsBinary = true + target5.AddMaybeExportedDependency(target4.Label, false, false, false, true) // runtime assert.Equal(t, []BuildLabel{}, target1.DeclaredDependenciesStrict()) assert.Equal(t, []BuildLabel{target1.Label}, target2.DeclaredDependenciesStrict()) assert.Equal(t, []BuildLabel{target2.Label}, target3.DeclaredDependenciesStrict()) + assert.Equal(t, []*BuildTarget{}, target5.BuildDependencies()) +} + +func TestRuntimeDependencies(t *testing.T) { + target1 := makeTarget1("//src/core:target1", "") + target2 := makeTarget1("//src/core:target2", "") + target3 := makeTarget1("//src/core:target3", "") + target2.IsBinary = true + target2.AddMaybeExportedDependency(target1.Label, false, false, false, true) // runtime + target3.IsBinary = true + target3.AddMaybeExportedDependency(target2.Label, false, false, false, true) // runtime + // RuntimeDependencies shouldn't return transitive run-time dependencies. + assert.Equal(t, []BuildLabel{}, target1.RuntimeDependencies()) + assert.Equal(t, []BuildLabel{target1.Label}, target2.RuntimeDependencies()) + assert.Equal(t, []BuildLabel{target2.Label}, target3.RuntimeDependencies()) +} + +func TestIterAllRuntimeDependencies(t *testing.T) { + target1 := makeTarget1("//src/core:target1", "") + target2 := makeTarget1("//src/core:target2", "") + target3 := makeTarget1("//src/core:target3", "") + target2.IsBinary = true + target2.AddMaybeExportedDependency(target1.Label, false, false, false, true) // runtime + target3.IsBinary = true + target3.AddMaybeExportedDependency(target2.Label, false, false, false, true) // runtime + graph := NewGraph() + graph.AddTarget(target1) + graph.AddTarget(target2) + graph.AddTarget(target3) + // IterAllRuntimeDependencies should yield transitive run-time dependencies. + assert.Nil(t, slices.Collect(target1.IterAllRuntimeDependencies(graph))) + assert.ElementsMatch(t, []BuildLabel{target1.Label}, slices.Collect(target2.IterAllRuntimeDependencies(graph))) + assert.ElementsMatch(t, []BuildLabel{target1.Label, target2.Label}, slices.Collect(target3.IterAllRuntimeDependencies(graph))) } func TestAddDependency(t *testing.T) { @@ -451,7 +496,7 @@ func TestAddDependency(t *testing.T) { target2.AddDependency(target1.Label) assert.Equal(t, []BuildLabel{target1.Label}, target2.DeclaredDependencies()) assert.Equal(t, []BuildLabel{}, target2.ExportedDependencies()) - target2.AddMaybeExportedDependency(target1.Label, true, false, false) + target2.AddMaybeExportedDependency(target1.Label, true, false, false, false) assert.Equal(t, []BuildLabel{target1.Label}, target2.DeclaredDependencies()) assert.Equal(t, []BuildLabel{target1.Label}, target2.ExportedDependencies()) assert.Equal(t, []*BuildTarget{}, target2.Dependencies()) @@ -459,13 +504,25 @@ func TestAddDependency(t *testing.T) { assert.Equal(t, []*BuildTarget{target1}, target2.Dependencies()) } +func TestAddRuntimeDependency(t *testing.T) { + target1 := makeTarget1("//src/core:target1", "PUBLIC") + target2 := makeTarget1("//src/core:target2", "PUBLIC") + target1.IsBinary = true + target1.AddMaybeExportedDependency(target2.Label, false, false, false, true) // runtime + assert.Equal(t, target1.runtimeDependencies, []BuildLabel{target2.Label}) + assert.True(t, target1.dependencies[0].runtime) + // Now we add it as a build-time dependency too, which should unset the runtime flag. + target1.AddMaybeExportedDependency(target2.Label, false, false, false, false) + assert.False(t, target1.dependencies[0].runtime) +} + func TestAddDependencySource(t *testing.T) { target1 := makeTarget1("//src/core:target1", "") target2 := makeTarget1("//src/core:target2", "") - target2.AddMaybeExportedDependency(target1.Label, true, true, false) + target2.AddMaybeExportedDependency(target1.Label, true, true, false, false) assert.True(t, target2.IsSourceOnlyDep(target1.Label)) // N.B. It's important that calling this again cancels the source flag. - target2.AddMaybeExportedDependency(target1.Label, true, false, false) + target2.AddMaybeExportedDependency(target1.Label, true, false, false, false) assert.False(t, target2.IsSourceOnlyDep(target1.Label)) } diff --git a/src/core/state.go b/src/core/state.go index 3596885b5..18a03d70a 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -1353,18 +1353,44 @@ func (state *BuildState) IterInputs(target *BuildTarget, test bool) iter.Seq[Bui return IterInputs(state, state.Graph, target, true, target.IsFilegroup) } return func(yield func(BuildInput) bool) { + // The target itself, plus its transitive run-time dependencies (since we're about to run the target): if !yield(target.Label) { return } + for runDep := range target.IterAllRuntimeDependencies(state.Graph) { + if !yield(runDep) { + return + } + } + // The target's data, plus the transitive run-time dependencies for data that are also targets: for _, datum := range target.AllData() { if !yield(datum) { return } + label, ok := datum.Label() + if !ok { + continue + } + for runDep := range state.Graph.TargetOrDie(label).IterAllRuntimeDependencies(state.Graph) { + if !yield(runDep) { + return + } + } } + // The target's test tools, plus the transitive run-time dependencies for test tools that are also targets: for _, tool := range target.AllTestTools() { if !yield(tool) { return } + label, ok := tool.Label() + if !ok { + continue + } + for runDep := range state.Graph.TargetOrDie(label).IterAllRuntimeDependencies(state.Graph) { + if !yield(runDep) { + return + } + } } } } diff --git a/src/core/utils.go b/src/core/utils.go index abc7998cf..3c63f5351 100644 --- a/src/core/utils.go +++ b/src/core/utils.go @@ -146,6 +146,11 @@ func IterInputs(state *BuildState, graph *BuildGraph, target *BuildTarget, inclu if !yield(p) { return false } + for runDep := range graph.TargetOrDie(p).IterAllRuntimeDependencies(graph) { + if !yield(runDep) { + return false + } + } } return true } @@ -157,6 +162,11 @@ func IterInputs(state *BuildState, graph *BuildGraph, target *BuildTarget, inclu if !yield(dependency.Label) { return false } + for runDep := range graph.TargetOrDie(dependency.Label).IterAllRuntimeDependencies(graph) { + if !yield(runDep) { + return false + } + } } done[dependency.Label] = true @@ -257,6 +267,15 @@ func IterRuntimeFiles(graph *BuildGraph, target *BuildTarget, absoluteOuts bool, } } + for runDep := range target.IterAllRuntimeDependencies(graph) { + fullPaths := runDep.FullPaths(graph) + for i, depPath := range runDep.Paths(graph) { + if !pushOut(fullPaths[i], depPath) { + return + } + } + } + for _, data := range target.AllData() { fullPaths := data.FullPaths(graph) for i, dataPath := range data.Paths(graph) { @@ -264,6 +283,18 @@ func IterRuntimeFiles(graph *BuildGraph, target *BuildTarget, absoluteOuts bool, return } } + label, ok := data.Label() + if !ok { + continue + } + for runDep := range graph.TargetOrDie(label).IterAllRuntimeDependencies(graph) { + fullPaths := runDep.FullPaths(graph) + for i, depPath := range runDep.Paths(graph) { + if !pushOut(fullPaths[i], depPath) { + return + } + } + } } if target.Test != nil { @@ -274,6 +305,18 @@ func IterRuntimeFiles(graph *BuildGraph, target *BuildTarget, absoluteOuts bool, return } } + label, ok := tool.Label() + if !ok { + continue + } + for runDep := range graph.TargetOrDie(label).IterAllRuntimeDependencies(graph) { + fullPaths := runDep.FullPaths(graph) + for i, depPath := range runDep.Paths(graph) { + if !pushOut(fullPaths[i], depPath) { + return + } + } + } } } @@ -285,6 +328,18 @@ func IterRuntimeFiles(graph *BuildGraph, target *BuildTarget, absoluteOuts bool, return } } + label, ok := data.Label() + if !ok { + continue + } + for runDep := range graph.TargetOrDie(label).IterAllRuntimeDependencies(graph) { + fullPaths := runDep.FullPaths(graph) + for i, depPath := range runDep.Paths(graph) { + if !pushOut(fullPaths[i], depPath) { + return + } + } + } } for _, tool := range target.AllDebugTools() { fullPaths := tool.FullPaths(graph) @@ -293,6 +348,18 @@ func IterRuntimeFiles(graph *BuildGraph, target *BuildTarget, absoluteOuts bool, return } } + label, ok := tool.Label() + if !ok { + continue + } + for runDep := range graph.TargetOrDie(label).IterAllRuntimeDependencies(graph) { + fullPaths := runDep.FullPaths(graph) + for i, depPath := range runDep.Paths(graph) { + if !pushOut(fullPaths[i], depPath) { + return + } + } + } } } } diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go index c34479f30..8aede7823 100644 --- a/src/parse/asp/builtins.go +++ b/src/parse/asp/builtins.go @@ -1176,7 +1176,8 @@ func addDep(s *scope, args []pyObject) pyObject { target := getTargetPost(s, string(args[0].(pyString))) dep := s.parseLabelInPackage(string(args[1].(pyString)), s.pkg) exported := args[2].IsTruthy() - target.AddMaybeExportedDependency(dep, exported, false, false) + runtime := args[3].IsTruthy() + target.AddMaybeExportedDependency(dep, exported, false, false, runtime) // Queue this dependency if it'll be needed. if target.State() > core.Inactive { err := s.state.QueueTarget(dep, target.Label, false, core.ParseModeNormal) diff --git a/src/parse/asp/targets.go b/src/parse/asp/targets.go index 94c656830..96baf2483 100644 --- a/src/parse/asp/targets.go +++ b/src/parse/asp/targets.go @@ -30,6 +30,7 @@ const ( outsBuildRuleArgIdx depsBuildRuleArgIdx exportedDepsBuildRuleArgIdx + runtimeDepsBuildRuleArgIdx secretsBuildRuleArgIdx toolsBuildRuleArgIdx testToolsBuildRuleArgIdx @@ -270,9 +271,10 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) { addMaybeNamedOutput(s, "outs", args[outsBuildRuleArgIdx], t.AddOutput, t.AddNamedOutput, t, false) addMaybeNamedOutput(s, "optional_outs", args[optionalOutsBuildRuleArgIdx], t.AddOptionalOutput, nil, t, true) t.HintDependencies(depLen(args[depsBuildRuleArgIdx]) + depLen(args[exportedDepsBuildRuleArgIdx]) + depLen(args[internalDepsBuildRuleArgIdx])) - addDependencies(s, "deps", args[depsBuildRuleArgIdx], t, false, false) - addDependencies(s, "exported_deps", args[exportedDepsBuildRuleArgIdx], t, true, false) - addDependencies(s, "internal_deps", args[internalDepsBuildRuleArgIdx], t, false, true) + addDependencies(s, "deps", args[depsBuildRuleArgIdx], t, false, false, false) + addDependencies(s, "exported_deps", args[exportedDepsBuildRuleArgIdx], t, true, false, false) + addDependencies(s, "internal_deps", args[internalDepsBuildRuleArgIdx], t, false, true, false) + addDependencies(s, "runtime_deps", args[runtimeDepsBuildRuleArgIdx], t, false, false, true) addStrings(s, "labels", args[labelsBuildRuleArgIdx], t.AddLabel) addStrings(s, "hashes", args[hashesBuildRuleArgIdx], t.AddHash) addStrings(s, "licences", args[licencesBuildRuleArgIdx], t.AddLicence) @@ -472,13 +474,13 @@ func addMaybeNamedSecret(s *scope, name string, obj pyObject, anon func(string), } // addDependencies adds dependencies to a target, which may or may not be exported. -func addDependencies(s *scope, name string, obj pyObject, target *core.BuildTarget, exported, internal bool) { +func addDependencies(s *scope, name string, obj pyObject, target *core.BuildTarget, exported, internal, runtime bool) { addStrings(s, name, obj, func(str string) { if s.state.Config.Bazel.Compatibility && !core.LooksLikeABuildLabel(str) && !strings.HasPrefix(str, "@") { // *sigh*... Bazel seems to allow an implicit : on the start of dependencies str = ":" + str } - target.AddMaybeExportedDependency(assertNotPseudoLabel(s, s.parseLabelInPackage(str, s.pkg)), exported, false, internal) + target.AddMaybeExportedDependency(assertNotPseudoLabel(s, s.parseLabelInPackage(str, s.pkg)), exported, false, internal, runtime) }) } diff --git a/src/query/print.go b/src/query/print.go index 9d5524bd3..0c551055d 100644 --- a/src/query/print.go +++ b/src/query/print.go @@ -137,6 +137,9 @@ func specialFields() specialFieldsMap { "exported_deps": func(target *core.BuildTarget) interface{} { return target.ExportedDependencies() }, + "runtime_deps": func(target *core.BuildTarget) interface{} { + return target.RuntimeDependencies() + }, "visibility": func(target *core.BuildTarget) interface{} { if len(target.Visibility) == 1 && target.Visibility[0] == core.WholeGraph[0] { return []string{"PUBLIC"} diff --git a/test/runtime_deps/BUILD b/test/runtime_deps/BUILD new file mode 100644 index 000000000..fb6f9ed9d --- /dev/null +++ b/test/runtime_deps/BUILD @@ -0,0 +1,68 @@ +subinclude("//test/build_defs") + +please_repo_e2e_test( + name = "runtime_deps_test", + plz_command = " && ".join([ + "plz test //test:runtime_deps_test_case", + ]), + repo = "repo", +) + +# Ensure that the runtime_deps field is correctly printed for those that have one (and that it is omitted for those that +# don't). Importantly, ensure that run-time dependencies are not printed transitively - `plz query print` should only +# print the target's direct run-time dependencies. +please_repo_e2e_test( + name = "query_print_test", + plz_command = " && ".join([ + "plz query print -f runtime_deps //test:runtime_deps_test_case > runtime_deps_test_case", + "plz query print -f runtime_deps //test:target_with_no_runtime_deps > target_with_no_runtime_deps", + ]), + expected_output = { + "runtime_deps_test_case": "//test:target_with_runtime_deps", + "target_with_no_runtime_deps": "", + }, + repo = "repo", +) + +# Ensure that run-time dependencies are in fact considered dependencies by `plz query deps`. Run-time dependencies added +# by a call to the add_runtime_dep function in a post-build function should not appear here. +_expected_deps = """\ +//test:dep_with_runtime_dep + //test:dep_runtime_dep +//test:src_with_runtime_dep + //test:src_dep_with_runtime_dep + //test:src_dep_runtime_dep + //test:src_runtime_dep +//test:target_with_runtime_deps + //test:runtime_dep + //test:target_requiring_kittens + //test:target_with_provides_runtime_dep + //test:provides_runtime_dep + //test:target_with_another_runtime_dep + //test:another_runtime_dep + //test:target_with_build_and_runtime_deps + //test:build_and_runtime_dep + //test:target_with_post_build_runtime_dep +//test:test_data_with_runtime_dep + //test:test_data_runtime_dep +//test:test_tool_with_runtime_dep + //test:test_tool_runtime_dep""" + +please_repo_e2e_test( + name = "query_deps_test", + plz_command = "plz query deps //test:runtime_deps_test_case > deps", + expected_output = { + "deps": _expected_deps, + }, + repo = "repo", +) + +# Ensure that run-time dependencies are in fact considered dependencies by `plz query revdeps`. +please_repo_e2e_test( + name = "query_revdeps_test", + plz_command = "plz query revdeps //test:another_runtime_dep > revdeps", + expected_output = { + "revdeps": "//test:target_with_another_runtime_dep", + }, + repo = "repo", +) diff --git a/test/runtime_deps/repo/.plzconfig b/test/runtime_deps/repo/.plzconfig new file mode 100644 index 000000000..e69de29bb diff --git a/test/runtime_deps/repo/test/BUILD_FILE b/test/runtime_deps/repo/test/BUILD_FILE new file mode 100644 index 000000000..de3274738 --- /dev/null +++ b/test/runtime_deps/repo/test/BUILD_FILE @@ -0,0 +1,197 @@ +def exists(file:str): + path = join_path(package_name(), file) + return f"echo -n '{path} exists: '; if [ -f '{path}' ]; then echo OK; else echo FAIL; exit 1; fi" + +def not_exists(file:str): + path = join_path(package_name(), file) + return f"echo -n '{path} does not exist: '; if [ -f '{path}' ]; then echo FAIL; exit 1; else echo OK; fi" + +def runtime_dep(name:str): + return genrule( + name = name, + outs = [name], + cmd = "touch $OUTS", + ) + +def target(name:str, build_tests:list=[], post_build:function=None, requires:list=None, provides:dict=None, + runtime_deps:list=[], deps:list=[]): + # Ensure that run-time dependencies are not present at build time, unless they are also + # explicitly given as build-time dependencies. (The lstrips here turn build target names + # into the names of files they output - they're slightly grungy, but they allow us to + # re-use exists and not_exists outside of this function; they work because targets + # generated by runtime_dep output a single file who name is identical to that of the + # target's.) + cmd = [not_exists(r.lstrip(":")) for r in runtime_deps if r not in deps] + cmd += [exists(d.lstrip(":")) for d in deps] + return genrule( + name = name, + outs = [name], + # Assume that if a post-build function is defined, it dynamically adds a run-time + # dependency, which requires this target to be marked as binary. + binary = len(runtime_deps) > 0 or post_build is not None, + cmd = cmd + build_tests + ["touch $OUTS"], + post_build = post_build, + requires = requires, + provides = provides, + runtime_deps = runtime_deps, + deps = deps, + ) + + +target( + name = "target_with_no_runtime_deps", +) + +target( + name = "target_with_runtime_deps", + build_tests = [ + # Ensure that the run-time dependencies :target_with_post_build_runtime_dep and + # :target_requiring_kittens are not present at build time (these ones are tricky + # to identify statically, hence explicitly listing them here). + not_exists("post_build_runtime_dep"), + not_exists("provides_runtime_dep"), + ], + runtime_deps = [ + ":runtime_dep", + ":target_with_another_runtime_dep", + ":target_with_build_and_runtime_deps", + ":target_with_post_build_runtime_dep", + ":target_requiring_kittens", + ], +) + +target( + name = "target_with_another_runtime_dep", + runtime_deps = [":another_runtime_dep"], +) + +target( + name = "target_with_build_and_runtime_deps", + runtime_deps = [":build_and_runtime_dep"], + deps = [":build_and_runtime_dep"], +) + +target( + name = "target_with_post_build_runtime_dep", + build_tests = [ + # Ensure that the run-time dependency added by the post-build function is not present at + # build time (this one can't be identified statically, hence explicitly listing it here). + not_exists("post_build_runtime_dep"), + ], + post_build = lambda name, _: add_runtime_dep(name, ":post_build_runtime_dep"), +) + +target( + name = "src_with_runtime_dep", + runtime_deps = [":src_runtime_dep"], + deps = [":src_dep_with_runtime_dep"], +) + +target( + name = "src_dep_with_runtime_dep", + runtime_deps = [":src_dep_runtime_dep"], +) + +target( + name = "dep_with_runtime_dep", + runtime_deps = [":dep_runtime_dep"], +) + +target( + name = "target_requiring_kittens", + build_tests = [ + # Ensure that neither of the run-time dependencies that could possibly be provided by + # :runtime_dep_providing_kittens or :target_with_provides_runtime_dep are present at + # build time (only the latter *should* be provided, but it isn't possible to generate + # both of these tests statically, hence explicitly listing them here). + not_exists("runtime_dep_providing_kittens_runtime_dep"), + not_exists("provides_runtime_dep"), + ], + requires = ["kittens"], + runtime_deps = [":runtime_dep_providing_kittens"], +) + +target( + name = "runtime_dep_providing_kittens", + provides = { + "kittens": ":target_with_provides_runtime_dep", + }, + runtime_deps = [":runtime_dep_providing_kittens_runtime_dep"], +) + +target( + name = "target_with_provides_runtime_dep", + runtime_deps = [":provides_runtime_dep"], +) + +target( + name = "test_data_with_runtime_dep", + runtime_deps = [":test_data_runtime_dep"], +) + +target( + name = "test_tool_with_runtime_dep", + runtime_deps = [":test_tool_runtime_dep"], +) + +runtime_dep("runtime_dep") +runtime_dep("another_runtime_dep") +runtime_dep("build_and_runtime_dep") +runtime_dep("post_build_runtime_dep") +runtime_dep("src_runtime_dep") +runtime_dep("src_dep_runtime_dep") +runtime_dep("dep_runtime_dep") +runtime_dep("test_data_runtime_dep") +runtime_dep("test_tool_runtime_dep") +runtime_dep("provides_runtime_dep") +runtime_dep("runtime_dep_providing_kittens_runtime_dep") + +gentest( + name = "runtime_deps_test_case", + srcs = { + "src_with_runtime_dep": [":src_with_runtime_dep"], + }, + outs = ["runtime_deps_test_case"], + cmd = [ + # Ensure this target's sources' and dependencies' run-time dependencies are present at build time... + exists("src_runtime_dep"), + exists("dep_runtime_dep"), + # ...but that those targets' (build-time) dependencies' run-time dependencies are not... + not_exists("src_dep_runtime_dep"), + # ...and that this target's run-time dependencies are not... + not_exists("runtime_dep"), + not_exists("another_runtime_dep"), + not_exists("build_and_runtime_dep"), + not_exists("post_build_runtime_dep"), + not_exists("provides_runtime_dep"), + not_exists("runtime_dep_providing_kittens_runtime_dep"), + # ...and that this target's test-time dependencies are not. + not_exists("test_data_runtime_dep"), + not_exists("test_tool_runtime_dep"), + "touch $OUTS", + ], + data = [":test_data_with_runtime_dep"], + no_test_output = True, + runtime_deps = [":target_with_runtime_deps"], + test_cmd = [ + # Ensure this target's sources' and dependencies' (transitive) run-time dependencies are not present at test time... + not_exists("src_runtime_dep"), + not_exists("dep_runtime_dep"), + not_exists("src_dep_runtime_dep"), + # ...but that this target's run-time dependencies are... + exists("runtime_dep"), + exists("another_runtime_dep"), + exists("build_and_runtime_dep"), + exists("post_build_runtime_dep"), + exists("provides_runtime_dep"), + # ...and that this target's test-time dependencies are... + exists("test_data_runtime_dep"), + exists("test_tool_runtime_dep"), + # ...and that the requires/provides in the run-time dependency tree were correctly resolved. + not_exists("runtime_dep_providing_kittens_runtime_dep"), + ], + test_tools = [":test_tool_with_runtime_dep"], + deps = [ + ":dep_with_runtime_dep", + ], +) diff --git a/tools/build_langserver/lsp/definition_test.go b/tools/build_langserver/lsp/definition_test.go index ff05ee0af..1adb13bab 100644 --- a/tools/build_langserver/lsp/definition_test.go +++ b/tools/build_langserver/lsp/definition_test.go @@ -25,7 +25,7 @@ func TestDefinition(t *testing.T) { assert.Equal(t, []lsp.Location{ { URI: lsp.DocumentURI("file://" + filepath.Join(cacheDir, "please/misc_rules.build_defs")), - Range: xrng(3, 0, 145, 5), + Range: xrng(3, 0, 153, 5), }, }, locs) } @@ -45,7 +45,7 @@ func TestDefinitionStatement(t *testing.T) { assert.Equal(t, []lsp.Location{ { URI: lsp.DocumentURI("file://" + filepath.Join(cacheDir, "please/misc_rules.build_defs")), - Range: xrng(3, 0, 145, 5), + Range: xrng(3, 0, 153, 5), }, }, locs) } @@ -65,7 +65,7 @@ func TestDefinitionBuiltin(t *testing.T) { assert.Equal(t, []lsp.Location{ { URI: lsp.DocumentURI("file://" + filepath.Join(cacheDir, "please/misc_rules.build_defs")), - Range: xrng(3, 0, 145, 5), + Range: xrng(3, 0, 153, 5), }, }, locs) }