Skip to content

Commit ef60d42

Browse files
committed
Allow 'default' entry in provide map to decide what happens when a require doesn't match
1 parent 0a9031b commit ef60d42

16 files changed

+117
-80
lines changed

src/build/filegroup.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ func buildFilegroup(state *core.BuildState, target *core.BuildTarget) (bool, err
108108
}
109109
changed := false
110110
outDir := target.OutDir()
111-
localSources := target.AllSourceLocalPaths(state.Graph)
112-
for i, source := range target.AllSourceFullPaths(state.Graph) {
111+
localSources := target.AllSourceLocalPaths(state.Graph, state.Config.FeatureFlags.FFDefaultProvides)
112+
for i, source := range target.AllSourceFullPaths(state.Graph, state.Config.FeatureFlags.FFDefaultProvides) {
113113
out := filepath.Join(outDir, localSources[i])
114114
fileChanged, err := theFilegroupBuilder.Build(state, target, source, out)
115115
if err != nil {
@@ -154,8 +154,8 @@ func buildFilegroup(state *core.BuildState, target *core.BuildTarget) (bool, err
154154
// This is a small optimisation to ensure we don't need to recalculate them unnecessarily.
155155
func copyFilegroupHashes(state *core.BuildState, target *core.BuildTarget) {
156156
outDir := target.OutDir()
157-
localSources := target.AllSourceLocalPaths(state.Graph)
158-
for i, source := range target.AllSourceFullPaths(state.Graph) {
157+
localSources := target.AllSourceLocalPaths(state.Graph, state.Config.FeatureFlags.FFDefaultProvides)
158+
for i, source := range target.AllSourceFullPaths(state.Graph, state.Config.FeatureFlags.FFDefaultProvides) {
159159
if out := filepath.Join(outDir, localSources[i]); out != source {
160160
state.PathHasher.CopyHash(source, out)
161161
}

src/core/build_env.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func TargetEnvironment(state *BuildState, target *BuildTarget) BuildEnv {
6969
// We read this as being slightly more POSIX-compliant than not having it set at all...
7070
func BuildEnvironment(state *BuildState, target *BuildTarget, tmpDir string) BuildEnv {
7171
env := TargetEnvironment(state, target)
72-
sources := target.AllSourcePaths(state.Graph)
72+
sources := target.AllSourcePaths(state.Graph, state.Config.FeatureFlags.FFDefaultProvides)
7373
outEnv := target.GetTmpOutputAll(target.Outputs())
7474
abs := filepath.IsAbs(tmpDir)
7575

@@ -92,7 +92,7 @@ func BuildEnvironment(state *BuildState, target *BuildTarget, tmpDir string) Bui
9292
}
9393
// Named source groups if the target declared any.
9494
for name, srcs := range target.NamedSources {
95-
paths := target.SourcePaths(state.Graph, srcs)
95+
paths := target.SourcePaths(state.Graph, srcs, state.Config.FeatureFlags.FFDefaultProvides)
9696
// TODO(macripps): Quote these to prevent spaces from breaking everything (consider joining with NUL or sth?)
9797
env = append(env, "SRCS_"+strings.ToUpper(name)+"="+strings.Join(paths, " "))
9898
}

src/core/build_target.go

+31-21
Original file line numberDiff line numberDiff line change
@@ -491,28 +491,28 @@ func (target *BuildTarget) StartTestSuite() {
491491
}
492492

493493
// AllSourcePaths returns all the source paths for this target
494-
func (target *BuildTarget) AllSourcePaths(graph *BuildGraph) []string {
495-
return target.allSourcePaths(graph, BuildInput.Paths)
494+
func (target *BuildTarget) AllSourcePaths(graph *BuildGraph, ffDefaultProvide bool) []string {
495+
return target.allSourcePaths(graph, BuildInput.Paths, ffDefaultProvide)
496496
}
497497

498498
// AllSourceFullPaths returns all the source paths for this target, with a leading
499499
// plz-out/gen etc if appropriate.
500-
func (target *BuildTarget) AllSourceFullPaths(graph *BuildGraph) []string {
501-
return target.allSourcePaths(graph, BuildInput.FullPaths)
500+
func (target *BuildTarget) AllSourceFullPaths(graph *BuildGraph, ffDefaultProvide bool) []string {
501+
return target.allSourcePaths(graph, BuildInput.FullPaths, ffDefaultProvide)
502502
}
503503

504504
// AllSourceLocalPaths returns the local part of all the source paths for this target,
505505
// i.e. without this target's package in it.
506-
func (target *BuildTarget) AllSourceLocalPaths(graph *BuildGraph) []string {
507-
return target.allSourcePaths(graph, BuildInput.LocalPaths)
506+
func (target *BuildTarget) AllSourceLocalPaths(graph *BuildGraph, ffDefaultProvide bool) []string {
507+
return target.allSourcePaths(graph, BuildInput.LocalPaths, ffDefaultProvide)
508508
}
509509

510510
type buildPathsFunc func(BuildInput, *BuildGraph) []string
511511

512-
func (target *BuildTarget) allSourcePaths(graph *BuildGraph, full buildPathsFunc) []string {
512+
func (target *BuildTarget) allSourcePaths(graph *BuildGraph, full buildPathsFunc, ffDefaultProvide bool) []string {
513513
ret := make([]string, 0, len(target.Sources))
514514
for _, source := range target.AllSources() {
515-
ret = append(ret, target.sourcePaths(graph, source, full)...)
515+
ret = append(ret, target.sourcePaths(graph, source, full, ffDefaultProvide)...)
516516
}
517517
return ret
518518
}
@@ -533,7 +533,7 @@ func (target *BuildTarget) AllURLs(state *BuildState) []string {
533533
// TODO(peterebden,tatskaari): Work out if we really want to have this and how the suite of *Dependencies functions
534534
//
535535
// below should behave (preferably nicely).
536-
func (target *BuildTarget) resolveDependencies(graph *BuildGraph, callback func(*BuildTarget) error) error {
536+
func (target *BuildTarget) resolveDependencies(graph *BuildGraph, callback func(*BuildTarget) error, ffDefaultProvide bool) error {
537537
var g errgroup.Group
538538
target.mutex.RLock()
539539
for i := range target.dependencies {
@@ -542,7 +542,7 @@ func (target *BuildTarget) resolveDependencies(graph *BuildGraph, callback func(
542542
continue // already done
543543
}
544544
g.Go(func() error {
545-
if err := target.resolveOneDependency(graph, dep); err != nil {
545+
if err := target.resolveOneDependency(graph, dep, ffDefaultProvide); err != nil {
546546
return err
547547
}
548548
for _, d := range dep.deps {
@@ -557,14 +557,15 @@ func (target *BuildTarget) resolveDependencies(graph *BuildGraph, callback func(
557557
return g.Wait()
558558
}
559559

560-
func (target *BuildTarget) resolveOneDependency(graph *BuildGraph, dep *depInfo) error {
560+
func (target *BuildTarget) resolveOneDependency(graph *BuildGraph, dep *depInfo, ffDefaultProvide bool) error {
561561
t := graph.WaitForTarget(*dep.declared)
562562
if t == nil {
563563
return fmt.Errorf("Couldn't find dependency %s", dep.declared)
564564
}
565565
dep.declared = &t.Label // saves memory by not storing the label twice once resolved
566566

567-
labels := t.provideFor(target)
567+
// TODO(jpoole): shouldn't this use the ProvideFor wrapper that handles resolving to self if there's no match?
568+
labels := t.provideFor(target, ffDefaultProvide)
568569

569570
if len(labels) == 0 {
570571
target.mutex.Lock()
@@ -595,7 +596,7 @@ func (target *BuildTarget) resolveOneDependency(graph *BuildGraph, dep *depInfo)
595596
// MustResolveDependencies is exposed only for testing purposes.
596597
// TODO(peterebden, tatskaari): See if we can get rid of this.
597598
func (target *BuildTarget) ResolveDependencies(graph *BuildGraph) error {
598-
return target.resolveDependencies(graph, func(*BuildTarget) error { return nil })
599+
return target.resolveDependencies(graph, func(*BuildTarget) error { return nil }, false)
599600
}
600601

601602
// DeclaredDependencies returns all the targets this target declared any kind of dependency on (including sources and tools).
@@ -880,19 +881,19 @@ func (target *BuildTarget) GetRealOutput(output string) string {
880881
}
881882

882883
// SourcePaths returns the source paths for a given set of sources.
883-
func (target *BuildTarget) SourcePaths(graph *BuildGraph, sources []BuildInput) []string {
884+
func (target *BuildTarget) SourcePaths(graph *BuildGraph, sources []BuildInput, ffDefaultProvide bool) []string {
884885
ret := make([]string, 0, len(sources))
885886
for _, source := range sources {
886-
ret = append(ret, target.sourcePaths(graph, source, BuildInput.Paths)...)
887+
ret = append(ret, target.sourcePaths(graph, source, BuildInput.Paths, ffDefaultProvide)...)
887888
}
888889
return ret
889890
}
890891

891892
// sourcePaths returns the source paths for a single source.
892-
func (target *BuildTarget) sourcePaths(graph *BuildGraph, source BuildInput, f buildPathsFunc) []string {
893+
func (target *BuildTarget) sourcePaths(graph *BuildGraph, source BuildInput, f buildPathsFunc, ffDefaultProvide bool) []string {
893894
if label, ok := source.nonOutputLabel(); ok {
894895
ret := []string{}
895-
for _, providedLabel := range graph.TargetOrDie(label).ProvideFor(target) {
896+
for _, providedLabel := range graph.TargetOrDie(label).ProvideFor(target, ffDefaultProvide) {
896897
ret = append(ret, f(providedLabel, graph)...)
897898
}
898899
return ret
@@ -1195,20 +1196,25 @@ func (target *BuildTarget) AddProvide(language string, label BuildLabel) {
11951196
}
11961197

11971198
// ProvideFor returns the build label that we'd provide for the given target.
1198-
func (target *BuildTarget) ProvideFor(other *BuildTarget) []BuildLabel {
1199-
if p := target.provideFor(other); len(p) > 0 {
1199+
func (target *BuildTarget) ProvideFor(other *BuildTarget, ffDefaultProvide bool) []BuildLabel {
1200+
if p := target.provideFor(other, ffDefaultProvide); len(p) > 0 {
12001201
return p
12011202
}
12021203
return []BuildLabel{target.Label}
12031204
}
12041205

12051206
// provideFor is like ProvideFor but returns an empty slice if there is a direct dependency.
12061207
// It's a small optimisation to save allocating extra slices.
1207-
func (target *BuildTarget) provideFor(other *BuildTarget) []BuildLabel {
1208+
func (target *BuildTarget) provideFor(other *BuildTarget, ffDefaultProvide bool) []BuildLabel {
12081209
target.mutex.RLock()
12091210
defer target.mutex.RUnlock()
1211+
var defaultValue []BuildLabel
1212+
if def, ok := target.Provides["default"]; ok && ffDefaultProvide {
1213+
defaultValue = []BuildLabel{def}
1214+
}
1215+
12101216
if target.Provides == nil || len(other.Requires) == 0 {
1211-
return nil
1217+
return defaultValue
12121218
}
12131219
// Never do this if the other target has a data or tool dependency on us.
12141220
for _, data := range other.Data {
@@ -1228,6 +1234,10 @@ func (target *BuildTarget) provideFor(other *BuildTarget) []BuildLabel {
12281234
ret = append(ret, label)
12291235
}
12301236
}
1237+
1238+
if len(ret) == 0 {
1239+
return defaultValue
1240+
}
12311241
return ret
12321242
}
12331243

src/core/build_target_benchmark_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func BenchmarkProvideFor(b *testing.B) {
1717
b.Run("Simple", func(b *testing.B) {
1818
b.ReportAllocs()
1919
for i := 0; i < b.N; i++ {
20-
result = target1.provideFor(target2)
20+
result = target1.provideFor(target2, true)
2121
}
2222
})
2323

@@ -26,7 +26,7 @@ func BenchmarkProvideFor(b *testing.B) {
2626
b.Run("NoMatch", func(b *testing.B) {
2727
b.ReportAllocs()
2828
for i := 0; i < b.N; i++ {
29-
result = target2.provideFor(target1)
29+
result = target2.provideFor(target1, true)
3030
}
3131
})
3232

@@ -35,15 +35,15 @@ func BenchmarkProvideFor(b *testing.B) {
3535
b.Run("OneMatch", func(b *testing.B) {
3636
b.ReportAllocs()
3737
for i := 0; i < b.N; i++ {
38-
result = target2.provideFor(target1)
38+
result = target2.provideFor(target1, true)
3939
}
4040
})
4141

4242
target1.Requires = []string{"go", "go_src"}
4343
b.Run("TwoMatches", func(b *testing.B) {
4444
b.ReportAllocs()
4545
for i := 0; i < b.N; i++ {
46-
result = target2.provideFor(target1)
46+
result = target2.provideFor(target1, true)
4747
}
4848
})
4949

@@ -52,15 +52,15 @@ func BenchmarkProvideFor(b *testing.B) {
5252
b.Run("FourMatches", func(b *testing.B) {
5353
b.ReportAllocs()
5454
for i := 0; i < b.N; i++ {
55-
result = target2.provideFor(target1)
55+
result = target2.provideFor(target1, true)
5656
}
5757
})
5858

5959
target1.AddDatum(target2.Label)
6060
b.Run("IsData", func(b *testing.B) {
6161
b.ReportAllocs()
6262
for i := 0; i < b.N; i++ {
63-
result = target2.provideFor(target1)
63+
result = target2.provideFor(target1, true)
6464
}
6565
})
6666
}

src/core/build_target_test.go

+28-4
Original file line numberDiff line numberDiff line change
@@ -219,18 +219,42 @@ func TestProvideFor(t *testing.T) {
219219
// target1 is provided directly since they have a simple dependency
220220
target1 := makeTarget1("//src/core:target1", "PUBLIC")
221221
target2 := makeTarget1("//src/core:target2", "PUBLIC", target1)
222-
assert.Equal(t, []BuildLabel{target1.Label}, target1.ProvideFor(target2))
222+
assert.Equal(t, []BuildLabel{target1.Label}, target1.ProvideFor(target2, true))
223223
// Now have target2 provide target1. target3 will get target1 instead.
224224
target2.Provides = map[string]BuildLabel{"whatevs": target1.Label}
225225
target3 := makeTarget1("//src/core:target3", "PUBLIC", target2)
226226
target3.Requires = append(target3.Requires, "whatevs")
227-
assert.Equal(t, []BuildLabel{target1.Label}, target2.ProvideFor(target3))
227+
assert.Equal(t, []BuildLabel{target1.Label}, target2.ProvideFor(target3, true))
228228
// Now target4 has a data dependency on target2. It has the same requirement as target3 but
229229
// it gets target2 instead of target1, because that's just how data deps work.
230230
target4 := makeTarget1("//src/core:target4", "PUBLIC", target2)
231231
target4.Data = append(target4.Data, target2.Label)
232232
target4.Requires = append(target4.Requires, "whatevs")
233-
assert.Equal(t, []BuildLabel{target2.Label}, target2.ProvideFor(target4))
233+
assert.Equal(t, []BuildLabel{target2.Label}, target2.ProvideFor(target4, true))
234+
}
235+
236+
func TestDefaultProvide(t *testing.T) {
237+
// target1 is provided directly since they have a simple dependency
238+
goTarget := makeTarget1("//src/core:go", "PUBLIC")
239+
defaultTarget := makeTarget1("//src/core:default", "PUBLIC")
240+
providesGo := makeTarget1("//src/core:provider", "PUBLIC", goTarget, defaultTarget)
241+
242+
requireGo := makeTarget1("//src/core:req_go", "PUBLIC")
243+
requirePython := makeTarget1("//src/core:req_python", "PUBLIC")
244+
requireNothing := makeTarget1("//src/core:req_python", "PUBLIC")
245+
246+
requireGo.Requires = []string{"go"}
247+
requirePython.Requires = []string{"python"}
248+
249+
providesGo.Provides = map[string]BuildLabel{
250+
"go": goTarget.Label,
251+
"default": defaultTarget.Label,
252+
}
253+
254+
assert.Equal(t, []BuildLabel{defaultTarget.Label}, providesGo.ProvideFor(requirePython, true))
255+
assert.Equal(t, []BuildLabel{defaultTarget.Label}, providesGo.ProvideFor(requireNothing, true))
256+
assert.Equal(t, []BuildLabel{providesGo.Label}, providesGo.ProvideFor(requirePython, false))
257+
assert.Equal(t, []BuildLabel{goTarget.Label}, providesGo.ProvideFor(requireGo, true))
234258
}
235259

236260
func TestAddProvide(t *testing.T) {
@@ -240,7 +264,7 @@ func TestAddProvide(t *testing.T) {
240264
target2.AddDependency(target1.Label)
241265
target2.AddProvide("go", ParseBuildLabel(":target1", "src/core"))
242266
target3.Requires = append(target3.Requires, "go")
243-
assert.Equal(t, []BuildLabel{target1.Label}, target2.ProvideFor(target3))
267+
assert.Equal(t, []BuildLabel{target1.Label}, target2.ProvideFor(target3, true))
244268
}
245269

246270
func TestAddDatum(t *testing.T) {

src/core/config.go

+1
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ type Configuration struct {
688688
buildEnvStored *storedBuildEnv
689689

690690
FeatureFlags struct {
691+
FFDefaultProvides bool `help:"Allows setting 'default' in the provides map to control what happens when none of the provided targets match"`
691692
} `help:"Flags controlling preview features for the next release. Typically these config options gate breaking changes and only have a lifetime of one major release."`
692693
Metrics struct {
693694
PrometheusGatewayURL string `help:"The gateway URL to push prometheus updates to."`

src/core/graph.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,10 @@ func NewGraph() *BuildGraph {
157157

158158
// DependentTargets returns the labels that 'from' should actually depend on when it declared a dependency on 'to'.
159159
// This is normally just 'to' but could be otherwise given require/provide shenanigans.
160-
func (graph *BuildGraph) DependentTargets(from, to BuildLabel) []BuildLabel {
160+
func (graph *BuildGraph) DependentTargets(from, to BuildLabel, ffDefaultProvide bool) []BuildLabel {
161161
fromTarget := graph.Target(from)
162162
if toTarget := graph.Target(to); fromTarget != nil && toTarget != nil {
163-
return toTarget.ProvideFor(fromTarget)
163+
return toTarget.ProvideFor(fromTarget, ffDefaultProvide)
164164
}
165165
return []BuildLabel{to}
166166
}

src/core/graph_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestDependentTargets(t *testing.T) {
3939
graph.AddTarget(target1)
4040
graph.AddTarget(target2)
4141
graph.AddTarget(target3)
42-
assert.Equal(t, []BuildLabel{target3.Label}, graph.DependentTargets(target2.Label, target1.Label))
42+
assert.Equal(t, []BuildLabel{target3.Label}, graph.DependentTargets(target2.Label, target1.Label, true))
4343
}
4444

4545
func TestSubrepo(t *testing.T) {

src/core/state.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,7 @@ func (state *BuildState) ActivateTarget(pkg *Package, label, dependent BuildLabe
10341034
}
10351035
}
10361036
} else {
1037-
for _, l := range state.Graph.DependentTargets(dependent, label) {
1037+
for _, l := range state.Graph.DependentTargets(dependent, label, state.Config.FeatureFlags.FFDefaultProvides) {
10381038
// We use :all to indicate a dependency needed for parse.
10391039
if err := state.QueueTarget(l, dependent, dependent.IsAllTargets(), mode); err != nil {
10401040
return err
@@ -1088,7 +1088,7 @@ func (state *BuildState) queueTarget(label, dependent BuildLabel, forceBuild boo
10881088
if dependent.IsAllTargets() || dependent == OriginalTarget {
10891089
return state.queueResolvedTarget(target, forceBuild, mode)
10901090
}
1091-
for _, l := range target.ProvideFor(state.Graph.TargetOrDie(dependent)) {
1091+
for _, l := range target.ProvideFor(state.Graph.TargetOrDie(dependent), state.Config.FeatureFlags.FFDefaultProvides) {
10921092
if l == label {
10931093
if err := state.queueResolvedTarget(target, forceBuild, mode); err != nil {
10941094
return err
@@ -1167,7 +1167,7 @@ func (state *BuildState) queueTargetAsync(target *BuildTarget, forceBuild, build
11671167
if err := target.resolveDependencies(state.Graph, func(t *BuildTarget) error {
11681168
called.SetTrue()
11691169
return state.queueResolvedTarget(t, forceBuild, ParseModeNormal)
1170-
}); err != nil {
1170+
}, state.Config.FeatureFlags.FFDefaultProvides); err != nil {
11711171
state.asyncError(target.Label, err)
11721172
return
11731173
}

0 commit comments

Comments
 (0)