From 3ea200549c6d87cf218c4108dae06894403e857c Mon Sep 17 00:00:00 2001 From: rhysm Date: Tue, 21 Nov 2023 00:01:20 +1300 Subject: [PATCH 1/8] Redact secret env vars from test step logging (#2970) Co-authored-by: Rhys M --- src/test/test_step.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_step.go b/src/test/test_step.go index 8b6da2c2d2..6be50f9e28 100644 --- a/src/test/test_step.go +++ b/src/test/test_step.go @@ -336,7 +336,7 @@ func pluralise(word string, quantity int) string { } // testCommandAndEnv returns the test command & environment for a target. -func testCommandAndEnv(state *core.BuildState, target *core.BuildTarget, run int) (string, []string, error) { +func testCommandAndEnv(state *core.BuildState, target *core.BuildTarget, run int) (string, core.BuildEnv, error) { replacedCmd, err := core.ReplaceTestSequences(state, target, target.GetTestCommand(state)) env := core.TestEnvironment(state, target, filepath.Join(core.RepoRoot, target.TestDir(run))) if len(state.TestArgs) > 0 { @@ -350,7 +350,7 @@ func runTest(state *core.BuildState, target *core.BuildTarget, run int) ([]byte, if err != nil { return nil, err } - log.Debugf("Running test %s#%d\nENVIRONMENT:\n%s\n%s", target.Label, run, strings.Join(env, "\n"), replacedCmd) + log.Debugf("Running test %s#%d\nENVIRONMENT:\n%s\n%s", target.Label, run, env, replacedCmd) _, stderr, err := state.ProcessExecutor.ExecWithTimeoutShellStdStreams(target, target.TestDir(run), env, target.Test.Timeout, state.ShowAllOutput, false, process.NewSandboxConfig(target.Test.Sandbox, target.Test.Sandbox), replacedCmd, state.DebugFailingTests) return stderr, err } From 106432eacbf6792bb076cd7ccbeddf2ef098ee9f Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Mon, 20 Nov 2023 06:07:37 -0500 Subject: [PATCH 2/8] Fix plz exec/run sequential/parallel to treat the first arg as a target always (#2967) --- src/please.go | 17 +++++++++++------ test/plz_run/BUILD | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/please.go b/src/please.go index 3d6cf2b7f6..8cacb2ef10 100644 --- a/src/please.go +++ b/src/please.go @@ -554,7 +554,7 @@ var buildFunctions = map[string]func() int{ return 0 }, "exec.sequential": func() int { - annotated, unannotated, args := opts.Exec.Sequential.Args.Targets.Separate() + annotated, unannotated, args := opts.Exec.Sequential.Args.Targets.Separate(true) if len(unannotated) == 0 { return 0 } @@ -568,7 +568,7 @@ var buildFunctions = map[string]func() int{ return 0 }, "exec.parallel": func() int { - annotated, unannotated, args := opts.Exec.Parallel.Args.Targets.Separate() + annotated, unannotated, args := opts.Exec.Parallel.Args.Targets.Separate(true) if len(unannotated) == 0 { return 0 } @@ -602,7 +602,7 @@ var buildFunctions = map[string]func() int{ return 1 // We should never return from run.Run so if we make it here something's wrong. }, "run.parallel": func() int { - annotated, unannotated, args := opts.Run.Parallel.PositionalArgs.Targets.Separate() + annotated, unannotated, args := opts.Run.Parallel.PositionalArgs.Targets.Separate(true) if len(unannotated) == 0 { return 0 } @@ -619,7 +619,7 @@ var buildFunctions = map[string]func() int{ return 1 }, "run.sequential": func() int { - annotated, unannotated, args := opts.Run.Sequential.PositionalArgs.Targets.Separate() + annotated, unannotated, args := opts.Run.Sequential.PositionalArgs.Targets.Separate(true) if len(unannotated) == 0 { return 0 } @@ -1224,7 +1224,12 @@ func (arg *TargetOrArg) UnmarshalFlag(value string) error { type TargetsOrArgs []TargetOrArg // Separate splits the targets & arguments into the labels (in both annotated & unannotated forms) and the arguments. -func (l TargetsOrArgs) Separate() (annotated []core.AnnotatedOutputLabel, unannotated []core.BuildLabel, args []string) { +func (l TargetsOrArgs) Separate(requireOneLabel bool) (annotated []core.AnnotatedOutputLabel, unannotated []core.BuildLabel, args []string) { + if requireOneLabel && l[0].arg != "" && l[0].arg != "-" { + if err := l[0].label.UnmarshalFlag(l[0].arg); err != nil { + log.Fatalf("First argument must be a build label: %s", l[0].arg) + } + } for _, arg := range l { if l, _ := arg.label.Label(); l.IsEmpty() { if arg.arg == "-" { @@ -1246,7 +1251,7 @@ func (l TargetsOrArgs) Separate() (annotated []core.AnnotatedOutputLabel, unanno // SeparateUnannotated splits the targets & arguments into two slices. Annotations aren't permitted. func (l TargetsOrArgs) SeparateUnannotated() ([]core.BuildLabel, []string) { - annotated, unannotated, args := l.Separate() + annotated, unannotated, args := l.Separate(false) for _, a := range annotated { if a.Annotation != "" { log.Fatalf("Invalid build label; annotations are not permitted here: %s", a) diff --git a/test/plz_run/BUILD b/test/plz_run/BUILD index 0aeeae4454..d6e27c7b06 100644 --- a/test/plz_run/BUILD +++ b/test/plz_run/BUILD @@ -129,3 +129,21 @@ please_repo_e2e_test( plz_command = "plz run parallel //src:arg_printer wibble wobble > args.txt", repo = "test_repo", ) + +please_repo_e2e_test( + name = "test_args_sequential_no_slashes", + expected_output = { + "args.txt": "wibble wobble", + }, + plz_command = "plz run sequential src:arg_printer wibble wobble > args.txt", + repo = "test_repo", +) + +please_repo_e2e_test( + name = "test_args_parallel_no_slashes", + expected_output = { + "args.txt": "wibble wobble", + }, + plz_command = "plz run parallel src:arg_printer wibble wobble > args.txt", + repo = "test_repo", +) From 581787ccd46cb7d705b917a3c261ed9488632d04 Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Mon, 20 Nov 2023 17:50:35 +0000 Subject: [PATCH 3/8] Document & meaning in asp (#2971) --- docs/language.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/language.html b/docs/language.html index fb6999c222..748eaa78f7 100644 --- a/docs/language.html +++ b/docs/language.html @@ -271,6 +271,12 @@

if they make mistakes in their BUILD files (e.g. passing a string where a list is required).

+

+ Additionally, arguments can be aliased using the + def func(arg_name: str&arg_alias):... syntax, such that + func(arg_name="example") and + func(arg_alias="example") are equivalent. +

User-defined varargs and kwargs functions are not supported.

From c9553c338e2b2a3b0956a1c27e31f2fc5eb606a3 Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Mon, 20 Nov 2023 17:51:48 +0000 Subject: [PATCH 4/8] Use the remote FS to handle subrepos (#2965) * Use the remote FS to handle subrepos * Fix subrepos which just have a path passed in * Lint * Add missing lock for subrepo tree --- src/core/state.go | 16 +++++----- src/core/subrepo.go | 21 +++++++------ src/help/help.go | 5 ++-- src/parse/asp/builtins.go | 11 +++---- src/parse/parse_step.go | 5 ++-- src/remote/action.go | 25 ++++++++++++++++ src/remote/fs/fs.go | 8 +++++ src/remote/remote.go | 14 +++++++-- src/remote/utils.go | 62 +++++++++++++++++++++------------------ 9 files changed, 109 insertions(+), 58 deletions(-) diff --git a/src/core/state.go b/src/core/state.go index 68220c2bee..933e75e5ad 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -8,6 +8,7 @@ import ( "hash/crc32" "hash/crc64" "io" + iofs "io/fs" "path/filepath" "sort" "sync" @@ -111,6 +112,8 @@ type RemoteClient interface { DataRate() (int, int, int, int) // Disconnect disconnects from the remote execution server. Disconnect() error + // SubrepoFS returns a virtual filesystem for the subrepo target + SubrepoFS(target *BuildTarget, root string) iofs.FS } // A TargetHasher is a thing that knows how to create hashes for targets. @@ -840,12 +843,7 @@ func (state *BuildState) WaitForPackage(l, dependent BuildLabel, mode ParseMode) return state.WaitForPackage(l, dependent, mode) } -// WaitForBuiltTarget blocks until the given label is available as a build target and has been successfully built. -func (state *BuildState) WaitForBuiltTarget(l, dependent BuildLabel) *BuildTarget { - return state.waitForBuiltTarget(l, dependent, ParseModeForSubinclude) -} - -func (state *BuildState) waitForBuiltTarget(l, dependent BuildLabel, mode ParseMode) *BuildTarget { +func (state *BuildState) WaitForBuiltTarget(l, dependent BuildLabel, mode ParseMode) *BuildTarget { if t := state.Graph.Target(l); t != nil { if t.State().IsBuilt() { return t @@ -865,7 +863,7 @@ func (state *BuildState) waitForBuiltTarget(l, dependent BuildLabel, mode ParseM // Do this all over; the re-checking that happens here is actually fairly important to resolve // a potential race condition if the target was built between us checking earlier and registering // the channel just now. - return state.waitForBuiltTarget(l, dependent, mode) + return state.WaitForBuiltTarget(l, dependent, mode) } // AddTarget adds a new target to the build graph. @@ -941,7 +939,7 @@ func (state *BuildState) WaitForInitialTargetAndEnsureDownload(l, dependent Buil } func (state *BuildState) waitForTargetAndEnsureDownload(l, dependent BuildLabel, mode ParseMode) *BuildTarget { - target := state.waitForBuiltTarget(l, dependent, mode) + target := state.WaitForBuiltTarget(l, dependent, mode) if !target.State().IsBuilt() { return nil } @@ -1068,7 +1066,7 @@ func (state *BuildState) QueueTestTarget(target *BuildTarget) { func (state *BuildState) queueTargetData(target *BuildTarget) { for _, data := range target.AllData() { if l, ok := data.Label(); ok { - state.WaitForBuiltTarget(l, target.Label) + state.WaitForBuiltTarget(l, target.Label, ParseModeForSubinclude) } } } diff --git a/src/core/subrepo.go b/src/core/subrepo.go index 3bb4445a97..a4a16a264b 100644 --- a/src/core/subrepo.go +++ b/src/core/subrepo.go @@ -18,8 +18,6 @@ type Subrepo struct { Name string // The root directory to load it from. Root string - // A file system rooted at the subrepo's root directory. - FS iofs.FS // A root directory for outputs of this subrepo's targets PackageRoot string // If this repo is output by a target, this is the target that creates it. Can be nil. @@ -36,15 +34,9 @@ type Subrepo struct { } func NewSubrepo(state *BuildState, name, root string, target *BuildTarget, arch cli.Arch, isCrosscompile bool) *Subrepo { - subrepoFS := os.DirFS(root) - if root == "" { - // This happens for architecture subrepos, which should use the same FS as the host repo - subrepoFS = fs.HostFS - } return &Subrepo{ Name: name, Root: root, - FS: subrepoFS, State: state, Target: target, Arch: arch, @@ -52,6 +44,17 @@ func NewSubrepo(state *BuildState, name, root string, target *BuildTarget, arch } } +func (s *Subrepo) FS() iofs.FS { + if s == nil || s.Root == "" { + // Must be an architecture subrepo + return fs.HostFS + } + if s.Target == nil || s.Target.Local || s.State.RemoteClient == nil { + return os.DirFS(s.Root) + } + return s.State.RemoteClient.SubrepoFS(s.Target, s.Root) +} + // SubrepoForArch creates a new subrepo for the given architecture. func SubrepoForArch(state *BuildState, arch cli.Arch) *Subrepo { s := NewSubrepo(state.ForArch(arch), arch.String(), "", nil, arch, true) @@ -93,7 +96,7 @@ func readSubrepoConfig(repoConfig *Configuration, subrepo *Subrepo) error { return err } } - return readConfigFile(subrepo.FS, repoConfig, ".plzconfig", true) + return readConfigFile(subrepo.FS(), repoConfig, ".plzconfig", true) } func validateSubrepoNameAndPluginConfig(config, repoConfig *Configuration, subrepo *Subrepo) error { diff --git a/src/help/help.go b/src/help/help.go index 33c107380e..679d90e5cf 100644 --- a/src/help/help.go +++ b/src/help/help.go @@ -236,7 +236,8 @@ func getPluginBuildDefs(subrepo *core.Subrepo) map[string]*asp.Statement { p := asp.NewParser(subrepo.State) ret := make(map[string]*asp.Statement) for _, dir := range dirs { - dirEntries, err := iofs.ReadDir(subrepo.FS, dir) + fs := subrepo.FS() + dirEntries, err := iofs.ReadDir(fs, dir) if err != nil { log.Warningf("Failed to read %s: %s", dir, err) } @@ -246,7 +247,7 @@ func getPluginBuildDefs(subrepo *core.Subrepo) map[string]*asp.Statement { } path := filepath.Join(dir, entry.Name()) - bs, err := iofs.ReadFile(subrepo.FS, path) + bs, err := iofs.ReadFile(fs, path) if err != nil { log.Warningf("Failed to read %s: %s", path, err) } diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go index f33b715710..c17b57ca8b 100644 --- a/src/parse/asp/builtins.go +++ b/src/parse/asp/builtins.go @@ -608,7 +608,7 @@ func glob(s *scope, args []pyObject) pyObject { exclude = append(exclude, s.state.Config.Parse.BuildFileName...) if s.globber == nil { if s.pkg.Subrepo != nil { - s.globber = fs.NewGlobber(s.pkg.Subrepo.FS, s.state.Config.Parse.BuildFileName) + s.globber = fs.NewGlobber(s.pkg.Subrepo.FS(), s.state.Config.Parse.BuildFileName) } else { s.globber = fs.NewGlobber(fs.HostFS, s.state.Config.Parse.BuildFileName) } @@ -1293,11 +1293,12 @@ func subrepo(s *scope, args []pyObject) pyObject { if dep != "" { // N.B. The target must be already registered on this package. target = s.pkg.TargetOrDie(s.parseLabelInPackage(dep, s.pkg).Name) + root = target.Label.Name if len(target.Outputs()) == 1 { - root = filepath.Join(target.OutDir(), target.Outputs()[0]) - } else { - // TODO(jpoole): perhaps this should be a fatal error? - root = filepath.Join(target.OutDir(), name) + root = target.Outputs()[0] + } + if target.Local || s.state.RemoteClient == nil { + root = filepath.Join(target.OutDir(), root) } } else if args[PathArgIdx] != None { root = string(args[PathArgIdx].(pyString)) diff --git a/src/parse/parse_step.go b/src/parse/parse_step.go index 094fc05ff0..f4e346b825 100644 --- a/src/parse/parse_step.go +++ b/src/parse/parse_step.go @@ -64,7 +64,7 @@ func parse(state *core.BuildState, label, dependent core.BuildLabel, mode core.P if subrepo != nil && subrepo.Target != nil { // We have got the definition of the subrepo but it depends on something, make sure that has been built. - state.WaitForTargetAndEnsureDownload(subrepo.Target.Label, label, mode.IsPreload()) + state.WaitForBuiltTarget(subrepo.Target.Label, label, mode|core.ParseModeForSubinclude) if !subrepo.Target.State().IsBuilt() { return fmt.Errorf("%v: failed to build subrepo", label) } @@ -164,7 +164,7 @@ func parsePackage(state *core.BuildState, label, dependent core.BuildLabel, subr var fileSystem iofs.FS = fs.HostFS if subrepo != nil { pkg.SubrepoName = subrepo.Name - fileSystem = subrepo.FS + fileSystem = subrepo.FS() } // Only load the internal package for the host repo's state if state.ParentState == nil && packageName == InternalPackageName { @@ -183,6 +183,7 @@ func parsePackage(state *core.BuildState, label, dependent core.BuildLabel, subr return nil, err } defer file.Close() + pkg.Filename = filename if err := state.Parser.ParseReader(pkg, file, &label, &dependent, mode); err != nil { return nil, err diff --git a/src/remote/action.go b/src/remote/action.go index e83004f034..9894e7c52e 100644 --- a/src/remote/action.go +++ b/src/remote/action.go @@ -24,6 +24,7 @@ import ( "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/fs" "github.com/thought-machine/please/src/process" + remotefs "github.com/thought-machine/please/src/remote/fs" ) // uploadAction uploads a build action for a target and returns its digest. @@ -257,6 +258,30 @@ func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildT } continue } + if i, ok := input.(core.SubrepoFileLabel); ok && !target.Local { + for _, p := range i.Paths(c.state.Graph) { + subrepoPath, err := filepath.Rel(target.Subrepo.PackageRoot, p) + if err != nil { + return nil, fmt.Errorf("%v: source file not in subrepo package root (%v): %v", target.Label, p, err) + } + fileNode, dirNode, symlinkNode, err := remotefs.FindNode(target.Subrepo.FS(), subrepoPath) + if err != nil { + return nil, fmt.Errorf("%v: failed to find file in subrepo output: %v", target.Label, err) + } + + dir := b.Dir(filepath.Dir(p)) + if fileNode != nil { + dir.Files = append(dir.Files, fileNode) + } + if dirNode != nil { + dir.Directories = append(dir.Directories, dirNode) + } + if symlinkNode != nil { + dir.Symlinks = append(dir.Symlinks, symlinkNode) + } + } + continue + } if err := c.uploadInput(b, ch, input); err != nil { return nil, err } diff --git a/src/remote/fs/fs.go b/src/remote/fs/fs.go index 7fd2284a0e..d25254c81b 100644 --- a/src/remote/fs/fs.go +++ b/src/remote/fs/fs.go @@ -26,6 +26,14 @@ type Client interface { ReadBlob(ctx context.Context, d digest.Digest) ([]byte, *client.MovedBytesMetadata, error) } +func FindNode(fs iofs.FS, path string) (*pb.FileNode, *pb.DirectoryNode, *pb.SymlinkNode, error) { + casFS, ok := fs.(*CASFileSystem) + if !ok { + return nil, nil, nil, fmt.Errorf("not supported") + } + return casFS.FindNode(path) +} + // CASFileSystem is an fs.FS implemented on top of a Tree proto. This will download files as they are needed from the // CAS when they are opened. type CASFileSystem struct { diff --git a/src/remote/remote.go b/src/remote/remote.go index e568aa5247..bc2aa0d810 100644 --- a/src/remote/remote.go +++ b/src/remote/remote.go @@ -7,6 +7,7 @@ import ( "context" "encoding/hex" "fmt" + iofs "io/fs" "os" "path/filepath" "strings" @@ -37,6 +38,7 @@ import ( "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/fs" "github.com/thought-machine/please/src/metrics" + remotefs "github.com/thought-machine/please/src/remote/fs" ) var log = logging.Log @@ -146,8 +148,8 @@ func New(state *core.BuildState) *Client { c := &Client{ state: state, instance: state.Config.Remote.Instance, - outputs: map[core.BuildLabel]*pb.Directory{}, - subrepoTrees: map[core.BuildLabel]*pb.Tree{}, + outputs: make(map[core.BuildLabel]*pb.Directory, 100), + subrepoTrees: make(map[core.BuildLabel]*pb.Tree, 10), mdStore: newDirMDStore(state.Config.Remote.Instance, time.Duration(state.Config.Remote.CacheDuration)), existingBlobs: map[string]struct{}{ digest.Empty.Hash: {}, @@ -313,6 +315,14 @@ func (c *Client) digestEnum() pb.DigestFunction_Value { } } +func (c *Client) SubrepoFS(target *core.BuildTarget, root string) iofs.FS { + c.outputMutex.RLock() + defer c.outputMutex.RUnlock() + + tree := c.subrepoTrees[target.Label] + return remotefs.New(c.client, tree, root) +} + // Build executes a remote build of the given target. func (c *Client) Build(target *core.BuildTarget) (*core.BuildMetadata, error) { if err := c.CheckInitialised(); err != nil { diff --git a/src/remote/utils.go b/src/remote/utils.go index 854c0f5917..3a786a30c9 100644 --- a/src/remote/utils.go +++ b/src/remote/utils.go @@ -74,37 +74,42 @@ func (c *Client) setOutputs(target *core.BuildTarget, arTree *pb.Tree) error { c.outputMutex.Lock() defer c.outputMutex.Unlock() - o := &pb.Directory{ - Files: arTree.Root.Files, - Symlinks: arTree.Root.Symlinks, - NodeProperties: arTree.Root.NodeProperties, - } - - // A filesystem representing the output tree of the action result - actionResultFS := remotefs.New(c.client, arTree, ".") - - // The action result will contain the raw outputs in their original places. We need to move outputs within - // `output_dirs` to the output root. - for _, dirNode := range arTree.Root.Directories { - if outDir := maybeGetOutDir(dirNode.Name, target.OutputDirectories); outDir != "" { - // Make sure we add all the outputs to the target - if err := c.setOutputDirectoryOuts(target, actionResultFS, outDir); err != nil { - return err + var outputTree *pb.Tree + if len(target.OutputDirectories) == 0 { + outputTree = arTree + } else { + o := &pb.Directory{ + Files: arTree.Root.Files, + Symlinks: arTree.Root.Symlinks, + NodeProperties: arTree.Root.NodeProperties, + Directories: make([]*pb.DirectoryNode, 0, len(arTree.Root.Directories)), + } + // A filesystem representing the output tree of the action result + actionResultFS := remotefs.New(c.client, arTree, ".") + + // The action result will contain the raw outputs in their original places. We need to move outputs within + // `output_dirs` to the output root. + for _, dirNode := range arTree.Root.Directories { + if outDir := maybeGetOutDir(dirNode.Name, target.OutputDirectories); outDir != "" { + // Make sure we add all the outputs to the target + if err := c.setOutputDirectoryOuts(target, actionResultFS, outDir); err != nil { + return err + } + + // Now flatten the contents of the directory into the action result root + dir := actionResultFS.Dir(digest.NewFromProtoUnvalidated(dirNode.Digest)) + o.Directories = append(o.Directories, dir.Directories...) + o.Files = append(o.Files, dir.Files...) + o.Symlinks = append(o.Symlinks, dir.Symlinks...) + } else { + o.Directories = append(o.Directories, dirNode) } - - // Now flatten the contents of the directory into the action result root - dir := actionResultFS.Dir(digest.NewFromProtoUnvalidated(dirNode.Digest)) - o.Directories = append(o.Directories, dir.Directories...) - o.Files = append(o.Files, dir.Files...) - o.Symlinks = append(o.Symlinks, dir.Symlinks...) - } else { - o.Directories = append(o.Directories, dirNode) } - } - outputTree := &pb.Tree{ - Root: o, - Children: arTree.Children, + outputTree = &pb.Tree{ + Root: o, + Children: arTree.Children, + } } if target.IsSubrepo { @@ -152,7 +157,6 @@ func (c *Client) outputTree(ar *pb.ActionResult) (*pb.Tree, error) { downloadErrors.Inc() return nil, wrap(err, "Downloading tree digest for %s [%s]", d.Path, d.TreeDigest.Hash) } - o.Children = append(o.Children, append(tree.Children, tree.Root)...) o.Root.Directories = append(o.Root.Directories, &pb.DirectoryNode{ Name: d.Path, From 8b667f0bbc98134e06a12af2ab86be39f63e2de0 Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Tue, 21 Nov 2023 15:53:35 +0000 Subject: [PATCH 5/8] Don't doubel up on the subrepo arch when parsing labels from subrepos (#2973) --- src/parse/asp/interpreter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parse/asp/interpreter.go b/src/parse/asp/interpreter.go index c640e9e000..0d730f399f 100644 --- a/src/parse/asp/interpreter.go +++ b/src/parse/asp/interpreter.go @@ -314,7 +314,7 @@ func (s *scope) parseLabelInPackage(label string, pkg *core.Package) core.BuildL subrepo = "" } else if s.state.CurrentSubrepo == "" && subrepo == s.state.Config.PluginDefinition.Name { subrepo = "" - } else { + } else if pkg.Subrepo == nil || subrepo != pkg.Subrepo.Arch.String() { subrepo = pkg.SubrepoArchName(subrepo) } return core.BuildLabel{PackageName: p, Name: name, Subrepo: subrepo} From 06412aa22b2239edc333b84cb8d435976360eda8 Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Tue, 21 Nov 2023 17:03:49 +0000 Subject: [PATCH 6/8] Release beta.5 (#2974) --- ChangeLog | 7 +++++++ VERSION | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 9ba31731e8..8a86f11a47 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,13 @@ Version 17.4.0 * Fix reduce builtin (#2925) * Fix issue with completing idents in the language server (#2917) * Improve performance when a large number of input targets are supplied (#2942) + * Virtualise the subrepo filesystem for remote execution to avoid downloading + the whole subrepo to plz-out (#2952, #2954, #2953, #2955, #2960, #2961, + #2962, #2963, #2966, #2968, #2965) + * Fix issue where we would double the architecture subrepo if it was explicitly + specified in a subrepo (#2973) + * Always treat the first argument of `plz run` commands as a build label (#2967) + * Redact secrets from test step logging (#2970) Version 17.3.1 -------------- diff --git a/VERSION b/VERSION index c2e4e0d9a9..337a71806c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -17.4.0-beta.4 +17.4.0-beta.5 From 8bdbec17c6c75668385021d7ba8b98d0f7d0f02b Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Wed, 22 Nov 2023 18:26:15 +0000 Subject: [PATCH 7/8] Fix a couple issues with remotefs (#2975) --- VERSION | 2 +- src/core/subrepo.go | 11 ++++++++--- src/fs/glob.go | 8 ++------ src/remote/action.go | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/VERSION b/VERSION index 337a71806c..61d609d284 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -17.4.0-beta.5 +17.4.0-beta.6 diff --git a/src/core/subrepo.go b/src/core/subrepo.go index a4a16a264b..30d474b0b2 100644 --- a/src/core/subrepo.go +++ b/src/core/subrepo.go @@ -49,10 +49,15 @@ func (s *Subrepo) FS() iofs.FS { // Must be an architecture subrepo return fs.HostFS } - if s.Target == nil || s.Target.Local || s.State.RemoteClient == nil { - return os.DirFS(s.Root) + if s.IsRemoteSubrepo() { + return s.State.RemoteClient.SubrepoFS(s.Target, s.Root) } - return s.State.RemoteClient.SubrepoFS(s.Target, s.Root) + return os.DirFS(s.Root) +} + +// IsRemoteSubrepo returns true when the subrepo sources are remote i.e. not downloaded to plz-out +func (s *Subrepo) IsRemoteSubrepo() bool { + return s.Root != "" && s.Target != nil && !s.Target.Local && s.State.RemoteClient != nil } // SubrepoForArch creates a new subrepo for the given architecture. diff --git a/src/fs/glob.go b/src/fs/glob.go index 4c98ee345a..073e29cc43 100644 --- a/src/fs/glob.go +++ b/src/fs/glob.go @@ -72,7 +72,7 @@ func Glob(fs iofs.FS, buildFileNames []string, rootPath string, includes, exclud // it isn't safe for use in concurrent goroutines. type Globber struct { buildFileNames []string - fs iofs.ReadDirFS + fs iofs.FS walkedDirs map[string]walkedDir } @@ -90,13 +90,9 @@ func Match(glob, path string) (bool, error) { // NewGlobber creates a new Globber. You should call this rather than creating one directly (or use Glob() if you don't care). func NewGlobber(fs iofs.FS, buildFileNames []string) *Globber { - rdfs, ok := fs.(iofs.ReadDirFS) - if !ok { - log.Fatalf("NewGlobber must be constructed with a ReadDirFS") - } return &Globber{ buildFileNames: buildFileNames, - fs: rdfs, + fs: fs, walkedDirs: map[string]walkedDir{}, } } diff --git a/src/remote/action.go b/src/remote/action.go index 9894e7c52e..5b30d94188 100644 --- a/src/remote/action.go +++ b/src/remote/action.go @@ -258,7 +258,7 @@ func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildT } continue } - if i, ok := input.(core.SubrepoFileLabel); ok && !target.Local { + if i, ok := input.(core.SubrepoFileLabel); ok && target.Subrepo.IsRemoteSubrepo() { for _, p := range i.Paths(c.state.Graph) { subrepoPath, err := filepath.Rel(target.Subrepo.PackageRoot, p) if err != nil { From 7dbe4cdaf85c403e82a2bade5e9de200ba8002f8 Mon Sep 17 00:00:00 2001 From: Jonathan Poole Date: Wed, 22 Nov 2023 18:39:31 +0000 Subject: [PATCH 8/8] Update the go plugin to 1.11.4 (#2976) --- docs/BUILD | 2 +- plugins/BUILD | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/BUILD b/docs/BUILD index f19ff35e0c..380ca2a21e 100644 --- a/docs/BUILD +++ b/docs/BUILD @@ -59,7 +59,7 @@ genrule( plugins = { "python": "v1.5.0", "java": "v0.3.0", - "go": "v1.11.3", + "go": "v1.11.4", "cc": "v0.4.0", "shell": "v0.2.0", "go-proto": "v0.2.1", diff --git a/plugins/BUILD b/plugins/BUILD index 96143d70ee..453425034d 100644 --- a/plugins/BUILD +++ b/plugins/BUILD @@ -1,7 +1,7 @@ plugin_repo( name = "go", plugin = "go-rules", - revision = "v1.11.3", + revision = "v1.11.4", ) plugin_repo(