plz run parallel/sequential accepts inline args (#2916)

* Change run parallel args

* linter

* Fix test

* bump

* Still honour the old ones

docs/BUILD

@@ -57,9 +57,9 @@ genrule(
 
 # Plugin versions to pull the docs from
 plugins = {
-    "python": "v1.4.1",
+    "python": "v1.5.0",
     "java": "v0.3.0",
-    "go": "v1.9.0",
+    "go": "v1.9.1",
     "cc": "v0.4.0",
     "shell": "v0.2.0",
     "go-proto": "v0.2.1",

src/please.go

@@ -189,18 +189,18 @@ var opts struct {
 			NumTasks       int                `short:"n" long:"num_tasks" default:"10" description:"Maximum number of subtasks to run in parallel"`
 			Output         process.OutputMode `long:"output" default:"default" choice:"default" choice:"quiet" choice:"group_immediate" description:"Allows to control how the output should be handled."`
 			PositionalArgs struct {
-				Targets []core.AnnotatedOutputLabel `positional-arg-name:"target" description:"Targets to run"`
+				Targets TargetsOrArgs `positional-arg-name:"target" required:"true" description:"Target to run"`
 			} `positional-args:"true" required:"true"`
-			Args   cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the called processes."`
+			Args   cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the target. Deprecated, pass them directly as arguments (after -- if needed)"`
 			Detach bool          `long:"detach" description:"Detach from the parent process when all children have spawned"`
 		} `command:"parallel" description:"Runs a sequence of targets in parallel"`
 		Sequential struct {
 			Quiet          bool               `short:"q" long:"quiet" description:"Suppress output from successful subprocesses."`
 			Output         process.OutputMode `long:"output" default:"default" choice:"default" choice:"quiet" choice:"group_immediate" description:"Allows to control how the output should be handled."`
 			PositionalArgs struct {
-				Targets []core.AnnotatedOutputLabel `positional-arg-name:"target" description:"Targets to run"`
+				Targets TargetsOrArgs `positional-arg-name:"target" required:"true" description:"Target to run"`
 			} `positional-args:"true" required:"true"`
-			Args cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the called processes."`
+			Args cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the target. Deprecated, pass them directly as arguments (after -- if needed)"`
 		} `command:"sequential" description:"Runs a sequence of targets sequentially."`
 		Args struct {
 			Target core.AnnotatedOutputLabel `positional-arg-name:"target" required:"true" description:"Target to run"`
@@ -595,19 +595,21 @@ 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 {
-		if success, state := runBuild(unannotateLabels(opts.Run.Parallel.PositionalArgs.Targets), true, false, false); success {
+		annotated, unannotated, args := opts.Run.Parallel.PositionalArgs.Targets.Separate()
+		if success, state := runBuild(unannotated, true, false, false); success {
 			var dir string
 			if opts.Run.WD != "" {
 				dir = getAbsolutePath(opts.Run.WD, originalWorkingDirectory)
 			}
-			ls := state.ExpandOriginalMaybeAnnotatedLabels(opts.Run.Parallel.PositionalArgs.Targets)
 			output := opts.Run.Parallel.Output
-			os.Exit(run.Parallel(context.Background(), state, ls, opts.Run.Parallel.Args.AsStrings(), opts.Run.Parallel.NumTasks, output, opts.Run.Remote, opts.Run.Env, opts.Run.Parallel.Detach, opts.Run.InTempDir, dir))
+			args = append(args, opts.Run.Sequential.Args.AsStrings()...)
+			os.Exit(run.Parallel(context.Background(), state, annotated, args, opts.Run.Parallel.NumTasks, output, opts.Run.Remote, opts.Run.Env, opts.Run.Parallel.Detach, opts.Run.InTempDir, dir))
 		}
 		return 1
 	},
 	"run.sequential": func() int {
-		if success, state := runBuild(unannotateLabels(opts.Run.Sequential.PositionalArgs.Targets), true, false, false); success {
+		annotated, unannotated, args := opts.Run.Sequential.PositionalArgs.Targets.Separate()
+		if success, state := runBuild(unannotated, true, false, false); success {
 			var dir string
 			if opts.Run.WD != "" {
 				dir = getAbsolutePath(opts.Run.WD, originalWorkingDirectory)
@@ -617,9 +619,8 @@ var buildFunctions = map[string]func() int{
 				log.Warningf("--quiet has been deprecated in favour of --output=quiet and will be removed in v17.")
 				output = process.Quiet
 			}
-
-			ls := state.ExpandOriginalMaybeAnnotatedLabels(opts.Run.Sequential.PositionalArgs.Targets)
-			os.Exit(run.Sequential(state, ls, opts.Run.Sequential.Args.AsStrings(), output, opts.Run.Remote, opts.Run.Env, opts.Run.InTempDir, dir))
+			args = append(args, opts.Run.Sequential.Args.AsStrings()...)
+			os.Exit(run.Sequential(state, annotated, args, output, opts.Run.Remote, opts.Run.Env, opts.Run.InTempDir, dir))
 		}
 		return 1
 	},
@@ -1210,7 +1211,15 @@ type TargetsOrArgs []TargetOrArg
 func (l TargetsOrArgs) Separate() (annotated []core.AnnotatedOutputLabel, unannotated []core.BuildLabel, args []string) {
 	for _, arg := range l {
 		if l, _ := arg.label.Label(); l.IsEmpty() {
-			args = append(args, arg.arg)
+			if arg.arg == "-" {
+				labels := plz.ReadAndParseStdinLabels()
+				unannotated = append(unannotated, labels...)
+				for _, label := range labels {
+					annotated = append(annotated, core.AnnotatedOutputLabel{BuildLabel: label})
+				}
+			} else {
+				args = append(args, arg.arg)
+			}
 		} else {
 			annotated = append(annotated, arg.label)
 			unannotated = append(unannotated, arg.label.BuildLabel)
@@ -1435,14 +1444,6 @@ func initBuild(args []string) string {
 	return command
 }
 
-func unannotateLabels(als []core.AnnotatedOutputLabel) []core.BuildLabel {
-	labels := make([]core.BuildLabel, len(als))
-	for i, al := range als {
-		labels[i] = al.BuildLabel
-	}
-	return labels
-}
-
 func writeGoTraceFile() {
 	if err := runtime.StartTrace(); err != nil {
 		log.Fatalf("failed to start trace: %v", err)

src/plz/plz.go

@@ -239,16 +239,23 @@ func ReadStdinLabels(labels []core.BuildLabel) []core.BuildLabel {
 	ret := []core.BuildLabel{}
 	for _, l := range labels {
 		if l == core.BuildLabelStdin {
-			for s := range flags.ReadStdin() {
-				ret = append(ret, core.ParseBuildLabels([]string{s})...)
-			}
+			ret = append(ret, ReadAndParseStdinLabels()...)
 		} else {
 			ret = append(ret, l)
 		}
 	}
 	return ret
 }
 
+// ReadAndParseStdinLabels unconditionally reads stdin and parses it into build labels.
+func ReadAndParseStdinLabels() []core.BuildLabel {
+	ret := []core.BuildLabel{}
+	for s := range flags.ReadStdin() {
+		ret = append(ret, core.ParseBuildLabels([]string{s})...)
+	}
+	return ret
+}
+
 // A limiter allows only a certain number of concurrent tasks
 // TODO(peterebden): We have about four of these now, commonise this somewhere
 type limiter chan struct{}