diff --git a/ChangeLog b/ChangeLog index b99449cc6e..eb5044aa8f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +Version 17.21.0 +--------------- + * Expose build target source list via the filesystem instead of the environment with build_rule's src_list_files parameter (#3393) + Version 17.20.0 --------------- * Bump arcat to v1.2.1, fixing a number of bugs relating to handling of ar archives (#3420) diff --git a/VERSION b/VERSION index 1f8b337687..d0abdafb91 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -17.20.0 +17.21.0 diff --git a/rules/builtins.build_defs b/rules/builtins.build_defs index a8c9fcfd2e..5edca910d3 100644 --- a/rules/builtins.build_defs +++ b/rules/builtins.build_defs @@ -13,7 +13,7 @@ def build_rule(name:str, cmd:str|dict='', test_cmd:str|dict='', debug_cmd:str='' test_outputs:list=None, system_srcs:list=None, stamp:bool=False, tag:str='', optional_outs:list=None, progress:bool=False, size:str=None, _urls:list=None, internal_deps:list=None, pass_env:list=None, local:bool=False, output_dirs:list=[], exit_on_error:bool=CONFIG.EXIT_ON_ERROR, entry_points:dict={}, env:dict={}, _file_content:str=None, - _subrepo:bool=False, no_test_coverage:bool=False): + _subrepo:bool=False, no_test_coverage:bool=False, src_list_files:bool=False): pass def chr(i:int) -> str: @@ -285,7 +285,7 @@ def licenses(licences): def json(value, pretty=False) -> str: """Returns a JSON-formatted representation of a plz value. - + If pretty is true, the JSON will be pretty-printed with 2 spaces of indentation per level. """ pass diff --git a/src/build/build_step.go b/src/build/build_step.go index b2c8e4c050..088654e856 100644 --- a/src/build/build_step.go +++ b/src/build/build_step.go @@ -606,6 +606,13 @@ func prepareSources(state *core.BuildState, graph *core.BuildGraph, target *core return err } } + if target.SrcListFiles { + for slf := range target.SourceListFiles(graph) { + if err := fs.WriteFile(bytes.NewReader(slf.Content), filepath.Join(target.TmpDir(), slf.Dirname, slf.Filename), 0644); err != nil { + return err + } + } + } return nil } diff --git a/src/build/incrementality.go b/src/build/incrementality.go index 92307cef7f..f233a23a82 100644 --- a/src/build/incrementality.go +++ b/src/build/incrementality.go @@ -200,6 +200,7 @@ func ruleHash(state *core.BuildState, target *core.BuildTarget, runtime bool) [] hashBool(h, target.IsTextFile) hashBool(h, target.IsRemoteFile) hashBool(h, target.Local) + hashBool(h, target.SrcListFiles) hashOptionalBool(h, target.ExitOnError) for _, require := range target.Requires { h.Write([]byte(require)) diff --git a/src/build/incrementality_test.go b/src/build/incrementality_test.go index dfb35d8ac8..a2ce75cdfb 100644 --- a/src/build/incrementality_test.go +++ b/src/build/incrementality_test.go @@ -30,6 +30,7 @@ var KnownFields = map[string]bool{ "Commands": true, "NeedsTransitiveDependencies": true, "Local": true, + "SrcListFiles": true, "OptionalOutputs": true, "OutputIsComplete": true, "Requires": true, diff --git a/src/core/build_env.go b/src/core/build_env.go index 912c9bf6fd..732b71b9fa 100644 --- a/src/core/build_env.go +++ b/src/core/build_env.go @@ -77,7 +77,6 @@ func BuildEnvironment(state *BuildState, target *BuildTarget, tmpDir string) Bui env["TMP_DIR"] = tmpDir env["TMPDIR"] = tmpDir - env["SRCS"] = strings.Join(sources, " ") env["OUTS"] = strings.Join(outEnv, " ") env["HOME"] = tmpDir // Set a consistent hash seed for Python. Important for build determinism. @@ -87,16 +86,21 @@ func BuildEnvironment(state *BuildState, target *BuildTarget, tmpDir string) Bui if len(outEnv) == 1 { env["OUT"] = resolveOut(outEnv[0], tmpDir, target.Sandbox) } - // The SRC variable is only available on rules that have a single source file. - if len(sources) == 1 { - env["SRC"] = sources[0] - } - // Named source groups if the target declared any. - for name, srcs := range target.NamedSources { - paths := target.SourcePaths(state.Graph, srcs) - // TODO(macripps): Quote these to prevent spaces from breaking everything (consider joining with NUL or sth?) - env["SRCS_"+strings.ToUpper(name)] = strings.Join(paths, " ") + + if !target.SrcListFiles { + env["SRCS"] = strings.Join(sources, " ") + // The SRC variable is only available on rules that have a single source file. + if len(sources) == 1 { + env["SRC"] = sources[0] + } + // Named source groups if the target declared any. + for name, srcs := range target.NamedSources { + paths := target.SourcePaths(state.Graph, srcs) + // TODO(macripps): Quote these to prevent spaces from breaking everything (consider joining with NUL or sth?) + env["SRCS_"+strings.ToUpper(name)] = strings.Join(paths, " ") + } } + // Named output groups similarly. for name, outs := range target.DeclaredNamedOutputs() { outs = target.GetTmpOutputAll(outs) diff --git a/src/core/build_target.go b/src/core/build_target.go index c8f83d78c3..18dc8ed2f8 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -2,7 +2,9 @@ package core import ( "fmt" + "iter" "os" + "path" "path/filepath" "slices" "sort" @@ -241,6 +243,15 @@ type BuildTarget struct { IsTextFile bool `print:"false"` // Marks that the target was added in a post-build function. AddedPostBuild bool `print:"false"` + // If true, skips generating environment variables for sources; instead files will be generated in + // the build environment containing the lists of sources as follows: + // - _plz/srcs (equivalent to $SRCS) always + // - _plz/src (equivalent to $SRC) if the target has exactly one src + // - _plz/named_srcs/foo (equivalent to $SRCS_FOO) if the target has a named src called foo + // Note that in contrast to the environment variables which are space separated, these files are + // newline separated. This can be used for targets which have too many sources to fit in an + // environment variable. + SrcListFiles bool `name:"src_list_files"` // If true, the interactive progress display will try to infer the target's progress // via some heuristics on its output. showProgress atomic.Bool `name:"progress"` @@ -1952,3 +1963,47 @@ func (slice BuildTargets) Less(i, j int) bool { func (slice BuildTargets) Swap(i, j int) { slice[i], slice[j] = slice[j], slice[i] } + +const sourceListFileDir = "_plz" +const sourceListFileNamedDir = "named_srcs" + +type SourceListFile struct { + Dirname string + Filename string + Content []byte +} + +func (target *BuildTarget) SourceListFiles(graph *BuildGraph) iter.Seq[*SourceListFile] { + return func(yield func(*SourceListFile) bool) { + sources := target.AllSourcePaths(graph) + + if !yield(&SourceListFile{ + Dirname: sourceListFileDir, + Filename: "srcs", + Content: []byte(strings.Join(sources, "\n")), + }) { + return + } + + if len(sources) == 1 { + if !yield(&SourceListFile{ + Dirname: sourceListFileDir, + Filename: "src", + Content: []byte(sources[0]), + }) { + return + } + } + + for name, srcs := range target.NamedSources { + paths := target.SourcePaths(graph, srcs) + if !yield(&SourceListFile{ + Dirname: path.Join(sourceListFileDir, sourceListFileNamedDir), + Filename: name, + Content: []byte(strings.Join(paths, "\n")), + }) { + return + } + } + } +} diff --git a/src/parse/asp/targets.go b/src/parse/asp/targets.go index cd496a742a..94c656830d 100644 --- a/src/parse/asp/targets.go +++ b/src/parse/asp/targets.go @@ -72,6 +72,7 @@ const ( fileContentArgIdx subrepoArgIdx noTestCoverageArgIdx + srcListFilesIdx ) // createTarget creates a new build target as part of build_rule(). @@ -112,6 +113,7 @@ func createTarget(s *scope, args []pyObject) *core.BuildTarget { target.IsTextFile = args[cmdBuildRuleArgIdx] == textFileCommand target.Local = isTruthy(localBuildRuleArgIdx) target.ExitOnError = isTruthy(exitOnErrorArgIdx) + target.SrcListFiles = isTruthy(srcListFilesIdx) for _, o := range asStringList(s, args[outDirsBuildRuleArgIdx], "output_dirs") { target.AddOutputDirectory(o) } diff --git a/src/remote/action.go b/src/remote/action.go index 0b0daa59d5..69f6eaa6eb 100644 --- a/src/remote/action.go +++ b/src/remote/action.go @@ -296,6 +296,19 @@ func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildT Digest: entry.Digest.ToProto(), }) } + if target.SrcListFiles { + for slf := range target.SourceListFiles(c.state.Graph) { + entry := uploadinfo.EntryFromBlob(slf.Content) + if ch != nil { + ch <- entry + } + d := b.Dir(slf.Dirname) + d.Files = append(d.Files, &pb.FileNode{ + Name: slf.Filename, + Digest: entry.Digest.ToProto(), + }) + } + } return b, nil } diff --git a/test/source_list_files/BUILD b/test/source_list_files/BUILD new file mode 100644 index 0000000000..b6d95ab7e9 --- /dev/null +++ b/test/source_list_files/BUILD @@ -0,0 +1,35 @@ +subinclude("//test/build_defs") + +please_repo_e2e_test( + name = "srcs_test", + plz_command = "plz build //:with_srcs", + expected_output = { + "plz-out/gen/with_srcs.txt": "a.txt\nb.txt", + }, + repo = "test_repo", +) + +please_repo_e2e_test( + name = "src_test", + plz_command = "plz build //:with_one_src", + expected_output = { + "plz-out/gen/with_one_src.txt": "a.txt", + }, + repo = "test_repo", +) + +please_repo_e2e_test( + name = "named_srcs_test", + plz_command = "plz build //:with_named_srcs", + expected_output = { + "plz-out/gen/with_named_srcs.txt": "a.txt\nb.txt", + }, + repo = "test_repo", +) + +please_repo_e2e_test( + name = "flag_not_set_test", + plz_command = "plz build //:with_flag_not_set", + repo = "test_repo", + expected_failure = True, +) diff --git a/test/source_list_files/test_repo/.plzconfig b/test/source_list_files/test_repo/.plzconfig new file mode 100644 index 0000000000..a5a48e3384 --- /dev/null +++ b/test/source_list_files/test_repo/.plzconfig @@ -0,0 +1,2 @@ +[Parse] +BuildFileName = BUILD.test diff --git a/test/source_list_files/test_repo/BUILD.test b/test/source_list_files/test_repo/BUILD.test new file mode 100644 index 0000000000..1caa4855c9 --- /dev/null +++ b/test/source_list_files/test_repo/BUILD.test @@ -0,0 +1,43 @@ +build_rule( + name = "with_srcs", + srcs = [ + "a.txt", + "b.txt", + ], + outs = ["with_srcs.txt"], + cmd = "cat _plz/srcs > with_srcs.txt", + src_list_files = True, +) + +build_rule( + name = "with_one_src", + srcs = [ + "a.txt", + ], + outs = ["with_one_src.txt"], + cmd = "cat _plz/src > with_one_src.txt", + src_list_files = True, +) + +build_rule( + name = "with_named_srcs", + srcs = { + "foo": [ + "a.txt", + "b.txt", + ], + }, + outs = ["with_named_srcs.txt"], + cmd = "cat _plz/named_srcs/foo > with_named_srcs.txt", + src_list_files = True, +) + +build_rule( + name = "with_flag_not_set", + srcs = [ + "a.txt", + "b.txt", + ], + outs = ["with_flag_not_set.txt"], + cmd = "cat _plz/srcs > with_flag_not_set.txt", +) diff --git a/test/source_list_files/test_repo/a.txt b/test/source_list_files/test_repo/a.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/source_list_files/test_repo/b.txt b/test/source_list_files/test_repo/b.txt new file mode 100644 index 0000000000..e69de29bb2