diff --git a/VERSION b/VERSION index ab2eec2b6..4c35645a8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -17.34.0-beta.5 +17.34.0-beta.6 diff --git a/src/build/build_step.go b/src/build/build_step.go index bfb43778e..8cf817fda 100644 --- a/src/build/build_step.go +++ b/src/build/build_step.go @@ -3,6 +3,7 @@ package build import ( "bytes" + "context" "encoding/hex" "errors" "fmt" @@ -59,11 +60,11 @@ var successfulLocalTargetBuildDuration = metrics.NewHistogramVec( ) // Build implements the core logic for building a single target. -func Build(state *core.BuildState, target *core.BuildTarget, remote bool) error { +func Build(ctx context.Context, state *core.BuildState, target *core.BuildTarget, remote bool) error { state = state.ForTarget(target) target.SetState(core.Building) start := time.Now() - if err := buildTarget(state, target, remote); err != nil { + if err := buildTarget(ctx, state, target, remote); err != nil { if errors.Is(err, errStop) { target.SetState(core.Stopped) state.LogBuildResult(target, core.TargetBuildStopped, "Build stopped") @@ -159,7 +160,7 @@ func prepareOnly(state *core.BuildState, target *core.BuildTarget) error { // b) attempt to fetch the outputs from the cache based on the output hash // 3. Actually build the rule // 4. Store result in the cache -func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely bool) (err error) { +func buildTarget(ctx context.Context, state *core.BuildState, target *core.BuildTarget, runRemotely bool) (err error) { defer func() { if r := recover(); r != nil { if e, ok := r.(error); ok { @@ -180,7 +181,7 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b // This must run before we can leave this function successfully by any path. if target.PreBuildFunction != nil { log.Debug("Running pre-build function for %s", target.Label) - if err := state.Parser.RunPreBuildFunction(state, target); err != nil { + if err := state.Parser.RunPreBuildFunction(ctx, state, target); err != nil { return err } log.Debug("Finished pre-build function for %s", target.Label) @@ -242,7 +243,7 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b addOutDirOutsFromMetadata(target, metadata) if target.PostBuildFunction != nil { - if err := runPostBuildFunction(state, target, string(metadata.Stdout), ""); err != nil { + if err := runPostBuildFunction(ctx, state, target, string(metadata.Stdout), ""); err != nil { log.Warning("Error from post-build function for %s: %s; will rebuild", target.Label, err) } } @@ -302,7 +303,7 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b addOutDirOutsFromMetadata(target, metadata) if target.PostBuildFunction != nil && !haveRunPostBuildFunction { postBuildOutput = string(metadata.Stdout) - if err := runPostBuildFunction(state, target, postBuildOutput, ""); err != nil { + if err := runPostBuildFunction(ctx, state, target, postBuildOutput, ""); err != nil { return err } } @@ -344,7 +345,7 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b if target.PostBuildFunction != nil { outs := target.Outputs(state.Graph) - if err := runPostBuildFunction(state, target, string(metadata.Stdout), postBuildOutput); err != nil { + if err := runPostBuildFunction(ctx, state, target, string(metadata.Stdout), postBuildOutput); err != nil { return err } @@ -996,7 +997,7 @@ func checkRuleHashesOfType(target *core.BuildTarget, hashes, outputs []string, h // In some cases it may have already run; if so we compare the previous output and warn // if the two differ (they must be deterministic to ensure it's a pure function, since there // are a few different paths through here and we guarantee to only run them once). -func runPostBuildFunction(state *core.BuildState, target *core.BuildTarget, output, prevOutput string) error { +func runPostBuildFunction(ctx context.Context, state *core.BuildState, target *core.BuildTarget, output, prevOutput string) error { if prevOutput != "" { if output != prevOutput { log.Warning("The build output for %s differs from what we got back from the cache earlier.\n"+ @@ -1007,7 +1008,7 @@ func runPostBuildFunction(state *core.BuildState, target *core.BuildTarget, outp } return nil } - return state.Parser.RunPostBuildFunction(state, target, output) + return state.Parser.RunPostBuildFunction(ctx, state, target, output) } // checkLicences checks the licences for the target match what we've accepted / rejected in the config diff --git a/src/build/build_step_stress_test.go b/src/build/build_step_stress_test.go index 67744e055..79cb37da8 100644 --- a/src/build/build_step_stress_test.go +++ b/src/build/build_step_stress_test.go @@ -123,12 +123,12 @@ func (fake *fakeParser) ParseReader(_ context.Context, pkg *core.Package, r io.R } // RunPreBuildFunction stub -func (fake *fakeParser) RunPreBuildFunction(state *core.BuildState, target *core.BuildTarget) error { +func (fake *fakeParser) RunPreBuildFunction(_ context.Context, state *core.BuildState, target *core.BuildTarget) error { return nil } // RunPostBuildFunction stub -func (fake *fakeParser) RunPostBuildFunction(state *core.BuildState, target *core.BuildTarget, output string) error { +func (fake *fakeParser) RunPostBuildFunction(_ context.Context, state *core.BuildState, target *core.BuildTarget, output string) error { if f, present := fake.PostBuildFunctions[target]; present { return f(target, output) } diff --git a/src/build/build_step_test.go b/src/build/build_step_test.go index c4266596c..9aacd5602 100644 --- a/src/build/build_step_test.go +++ b/src/build/build_step_test.go @@ -32,7 +32,7 @@ var cache core.Cache func TestBuildTargetWithNoDeps(t *testing.T) { state, target := newState("//package1:target1") target.AddOutput("file1") - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) } @@ -40,7 +40,7 @@ func TestBuildTargetWithNoDeps(t *testing.T) { func TestFailedBuildTarget(t *testing.T) { state, target := newState("//package1:target1a") target.Command = "false" - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.Error(t, err) } @@ -49,7 +49,7 @@ func TestBuildTargetWhichNeedsRebuilding(t *testing.T) { // because there's no rule hash file. state, target := newState("//package1:target2") target.AddOutput("file2") - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) } @@ -60,7 +60,7 @@ func TestBuildTargetWhichDoesntNeedRebuilding(t *testing.T) { target.AddOutput("file3") StoreTargetMetadata(target, new(core.BuildMetadata)) assert.NoError(t, writeRuleHash(state, target)) - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Reused, target.State()) } @@ -73,7 +73,7 @@ func TestModifiedBuildTargetStillNeedsRebuilding(t *testing.T) { assert.NoError(t, writeRuleHash(state, target)) target.Command = "echo -n 'wibble wibble wibble' > $OUT" target.RuleHash = nil // Have to force a reset of this - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) } @@ -84,7 +84,7 @@ func TestSymlinkedOutputs(t *testing.T) { target.AddOutput("file5") target.AddSource(core.FileLabel{File: "src5", Package: "package1"}) target.Command = "ln -s $SRC $OUT" - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) } @@ -98,7 +98,7 @@ func TestPreBuildFunction(t *testing.T) { target.Command = "echo 'wibble wibble wibble' > $OUT" return nil }) - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) } @@ -112,7 +112,7 @@ func TestPostBuildFunction(t *testing.T) { assert.Equal(t, "wibble wibble wibble", output) return nil }) - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) assert.Equal(t, []string{"file7"}, target.Outputs(state.Graph)) @@ -130,7 +130,7 @@ func TestOutputDir(t *testing.T) { state, target := newTarget() - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.Equal(t, []string{"file7"}, target.Outputs(state.Graph)) @@ -142,7 +142,7 @@ func TestOutputDir(t *testing.T) { // Run again to load the outputs from the metadata state, target = newTarget() - err = buildTarget(state, target, false) + err = buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.Equal(t, []string{"file7"}, target.Outputs(state.Graph)) assert.Equal(t, core.Reused, target.State()) @@ -165,7 +165,7 @@ func TestOutputDirDoubleStar(t *testing.T) { state, target := newTarget(false) - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.Equal(t, []string{"foo"}, target.Outputs(state.Graph)) @@ -181,7 +181,7 @@ func TestOutputDirDoubleStar(t *testing.T) { state, target = newTarget(true) - err = buildTarget(state, target, false) + err = buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.Equal(t, []string{"foo/file7"}, target.Outputs(state.Graph)) @@ -196,7 +196,7 @@ func TestCacheRetrieval(t *testing.T) { target.AddOutput("file8") target.Command = "false" // Will fail if we try to build it. state.Cache = cache - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Cached, target.State()) } @@ -214,7 +214,7 @@ func TestPostBuildFunctionAndCache(t *testing.T) { return nil }) state.Cache = cache - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Built, target.State()) assert.True(t, called) @@ -234,7 +234,7 @@ func TestPostBuildFunctionAndCache2(t *testing.T) { return nil }) state.Cache = cache - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) assert.NoError(t, err) assert.Equal(t, core.Cached, target.State()) assert.True(t, called) @@ -277,7 +277,7 @@ func TestCreatePlzOutGo(t *testing.T) { target.AddLabel("link:plz-out/go/${PKG}/src") target.AddOutput("file1.go") assert.False(t, fs.PathExists("plz-out/go")) - assert.NoError(t, buildTarget(state, target, false)) + assert.NoError(t, buildTarget(t.Context(), state, target, false)) assert.True(t, fs.PathExists("plz-out/go/package1/src/file1.go")) } @@ -436,7 +436,7 @@ func TestBuildMetadatafileIsCreated(t *testing.T) { state, target := newState("//package1:mdtest") target.AddOutput("file1") - err := buildTarget(state, target, false) + err := buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.False(t, target.BuildCouldModifyTarget()) assert.True(t, fs.FileExists(filepath.Join(target.OutDir(), target.TargetBuildMetadataFileName()))) @@ -448,7 +448,7 @@ func TestBuildMetadatafileIsCreated(t *testing.T) { assert.Equal(t, stdOut, output) return nil }) - err = buildTarget(state, target, false) + err = buildTarget(t.Context(), state, target, false) require.NoError(t, err) assert.True(t, target.BuildCouldModifyTarget()) assert.True(t, fs.FileExists(filepath.Join(target.OutDir(), target.TargetBuildMetadataFileName()))) @@ -634,12 +634,12 @@ func (fake *fakeParser) ParseReader(_ context.Context, pkg *core.Package, r io.R } // RunPreBuildFunction stub -func (fake *fakeParser) RunPreBuildFunction(state *core.BuildState, target *core.BuildTarget) error { +func (fake *fakeParser) RunPreBuildFunction(_ context.Context, state *core.BuildState, target *core.BuildTarget) error { return target.PreBuildFunction.Call(target) } // RunPostBuildFunction stub -func (fake *fakeParser) RunPostBuildFunction(state *core.BuildState, target *core.BuildTarget, output string) error { +func (fake *fakeParser) RunPostBuildFunction(_ context.Context, state *core.BuildState, target *core.BuildTarget, output string) error { return target.PostBuildFunction.Call(target, output) } diff --git a/src/core/state.go b/src/core/state.go index e0d75b5ef..e62834857 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -72,9 +72,9 @@ type Parser interface { // ParseReader parses a single BUILD file into the given package. ParseReader(ctx context.Context, pkg *Package, reader io.ReadSeeker, forLabel, dependent *BuildLabel) error // RunPreBuildFunction runs a pre-build function for a target. - RunPreBuildFunction(state *BuildState, target *BuildTarget) error + RunPreBuildFunction(ctx context.Context, state *BuildState, target *BuildTarget) error // RunPostBuildFunction runs a post-build function for a target. - RunPostBuildFunction(state *BuildState, target *BuildTarget, output string) error + RunPostBuildFunction(ctx context.Context, state *BuildState, target *BuildTarget, output string) error } // A RemoteClient is the interface to a remote execution service. diff --git a/src/exec/exec_test.go b/src/exec/exec_test.go index 07c371290..00309200d 100644 --- a/src/exec/exec_test.go +++ b/src/exec/exec_test.go @@ -34,7 +34,7 @@ func TestPrepareRuntimeDir(t *testing.T) { if err := build.StoreTargetMetadata(target, &core.BuildMetadata{}); err != nil { panic(err) } - build.Build(state, target, false) + build.Build(t.Context(), state, target, false) err := core.PrepareRuntimeDir(state, target, "plz-out/exec/pkg") assert.Nil(t, err) diff --git a/src/parse/init.go b/src/parse/init.go index c228e4c36..c5d621aae 100644 --- a/src/parse/init.go +++ b/src/parse/init.go @@ -77,28 +77,27 @@ func (p *aspParser) ParseReader(ctx context.Context, pkg *core.Package, reader i return err } -func (p *aspParser) RunPreBuildFunction(state *core.BuildState, target *core.BuildTarget) error { - return p.runBuildFunction(state, target, "pre", func() error { +func (p *aspParser) RunPreBuildFunction(ctx context.Context, state *core.BuildState, target *core.BuildTarget) error { + return p.runBuildFunction(ctx, state, target, "pre", func() error { return target.PreBuildFunction.Call(target) }) } -func (p *aspParser) RunPostBuildFunction(state *core.BuildState, target *core.BuildTarget, output string) error { - return p.runBuildFunction(state, target, "post", func() error { +func (p *aspParser) RunPostBuildFunction(ctx context.Context, state *core.BuildState, target *core.BuildTarget, output string) error { + return p.runBuildFunction(ctx, state, target, "post", func() error { log.Debug("Running post-build function for %s. Build output:\n%s", target.Label, output) return target.PostBuildFunction.Call(target, output) }) } // runBuildFunction runs either the pre- or post-build function. -func (p *aspParser) runBuildFunction(state *core.BuildState, target *core.BuildTarget, callbackType string, f func() error) error { +func (p *aspParser) runBuildFunction(ctx context.Context, state *core.BuildState, target *core.BuildTarget, callbackType string, f func() error) error { state.LogBuildResult(target, core.PackageParsing, fmt.Sprintf("Running %s-build function for %s", callbackType, target.Label)) // This doesn't re-parse anything; it waits for the parse of this target's package to complete if // one is still in flight. Targets are added to the graph as each rule is created, so a target can // be picked up and built before the file that defines it has been fully interpreted - but the // callback both reads the package out of the graph and mutates it, so it can't run until then. - // There's no parse in flight for us to inherit a context from, hence Background. - if _, err := p.callbacks.Parse(context.Background(), target.Label, target.Label); err != nil { + if _, err := p.callbacks.Parse(ctx, target.Label, target.Label); err != nil { return err } if err := f(); err != nil { diff --git a/src/plz/plz.go b/src/plz/plz.go index 4571e805d..53b5ad053 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -460,6 +460,9 @@ func (r *runner) recursiveParsePackage(ctx context.Context, label, dependent cor func (r *runner) parseTarget(ctx context.Context, label, dependent core.BuildLabel) (*core.BuildTarget, error) { target, wait := r.state.Graph.TargetOrWait(label) if target != nil { + if err := r.ensurePreloads(ctx, r.state.ForTarget(target)); err != nil { + return nil, err + } return target, nil } pkg, err := r.parse(ctx, label, dependent, false, wait) @@ -612,7 +615,7 @@ func (r *runner) runBuildAction(ctx context.Context, target *core.BuildTarget) e if err := ctx.Err(); err != nil { return err } - return build.Build(r.state, target, remote) + return build.Build(ctx, r.state, target, remote) } // buildAll builds all the targets specified by the given label (which can be :all, but can't be ...). diff --git a/test/preload_pre_build/BUILD b/test/preload_pre_build/BUILD new file mode 100644 index 000000000..cd3e44188 --- /dev/null +++ b/test/preload_pre_build/BUILD @@ -0,0 +1,13 @@ +subinclude("//test/build_defs") + +# A preloaded subinclude that needs a target with a pre-build function. Running that function parses +# a package in the same repo, which must not wait on the preloads it's part of resolving. +please_repo_e2e_test( + name = "preload_pre_build_test", + expected_output = { + "out.txt": "//src:src", + }, + plz_command = "plz build //src:src && plz query alltargets //src:all > out.txt", + repo = "test_repo", + timeout = 120, +) diff --git a/test/preload_pre_build/test_repo/.plzconfig b/test/preload_pre_build/test_repo/.plzconfig new file mode 100644 index 000000000..5bde3ad03 --- /dev/null +++ b/test/preload_pre_build/test_repo/.plzconfig @@ -0,0 +1,3 @@ +[parse] +BuildFileName = BUILD +PreloadSubincludes = //build_defs:defs diff --git a/test/preload_pre_build/test_repo/build_defs/BUILD_FILE b/test/preload_pre_build/test_repo/build_defs/BUILD_FILE new file mode 100644 index 000000000..675b5806c --- /dev/null +++ b/test/preload_pre_build/test_repo/build_defs/BUILD_FILE @@ -0,0 +1,16 @@ +def _pre_build(name): + pass + +genrule( + name = "gen", + outs = ["gen.build_defs"], + cmd = "echo 'def gen():\n pass' > $OUT", + pre_build = _pre_build, + visibility = ["PUBLIC"], +) + +filegroup( + name = "defs", + srcs = ["defs.build_defs"], + visibility = ["PUBLIC"], +) diff --git a/test/preload_pre_build/test_repo/build_defs/defs.build_defs b/test/preload_pre_build/test_repo/build_defs/defs.build_defs new file mode 100644 index 000000000..78016c9c0 --- /dev/null +++ b/test/preload_pre_build/test_repo/build_defs/defs.build_defs @@ -0,0 +1,8 @@ +subinclude("//build_defs:gen") + +def defs(name): + gen() + return filegroup( + name = name, + srcs = [name + ".txt"], + ) diff --git a/test/preload_pre_build/test_repo/src/BUILD_FILE b/test/preload_pre_build/test_repo/src/BUILD_FILE new file mode 100644 index 000000000..8f21d4db5 --- /dev/null +++ b/test/preload_pre_build/test_repo/src/BUILD_FILE @@ -0,0 +1 @@ +defs(name = "src") diff --git a/test/preload_pre_build/test_repo/src/src.txt b/test/preload_pre_build/test_repo/src/src.txt new file mode 100644 index 000000000..85de9cf93 --- /dev/null +++ b/test/preload_pre_build/test_repo/src/src.txt @@ -0,0 +1 @@ +src