Merge "Version bpglob command line arguments" am: 45222ec3ca

Original change: https://android-review.googlesource.com/c/platform/build/blueprint/+/1673987

Change-Id: I207265e94a6ea9b2dcd3456cbfa54773c8e1d04d

bootstrap/bpglob/bpglob.go

@@ -19,23 +19,75 @@
 package main
 
 import (
+	"bytes"
+	"errors"
 	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"strconv"
 	"time"
 
 	"github.com/google/blueprint/pathtools"
 )
 
 var (
-	out = flag.String("o", "", "file to write list of files that match glob")
+	// flagSet is a flag.FlagSet with flag.ContinueOnError so that we can handle the versionMismatchError
+	// error from versionArg.
+	flagSet = flag.NewFlagSet("bpglob", flag.ContinueOnError)
 
-	excludes multiArg
+	out = flagSet.String("o", "", "file to write list of files that match glob")
+
+	excludes     multiArg
+	versionMatch versionArg
 )
 
 func init() {
-	flag.Var(&excludes, "e", "pattern to exclude from results")
+	flagSet.Var(&versionMatch, "v", "version number the command line was generated for")
+	flagSet.Var(&excludes, "e", "pattern to exclude from results")
+}
+
+// bpglob is executed through the rules in build-globs.ninja to determine whether soong_build
+// needs to rerun.  That means when the arguments accepted by bpglob change it will be called
+// with the old arguments, then soong_build will rerun and update build-globs.ninja with the new
+// arguments.
+//
+// To avoid having to maintain backwards compatibility with old arguments across the transition,
+// a version argument is used to detect the transition in order to stop parsing arguments, touch the
+// output file and exit immediately.  Aborting parsing arguments is necessary to handle parsing
+// errors that would be fatal, for example the removal of a flag.  The version number in
+// pathtools.BPGlobArgumentVersion should be manually incremented when the bpglob argument format
+// changes.
+//
+// If the version argument is not passed then a version mismatch is assumed.
+
+// versionArg checks the argument against pathtools.BPGlobArgumentVersion, returning a
+// versionMismatchError error if it does not match.
+type versionArg bool
+
+var versionMismatchError = errors.New("version mismatch")
+
+func (v *versionArg) String() string { return "" }
+
+func (v *versionArg) Set(s string) error {
+	vers, err := strconv.Atoi(s)
+	if err != nil {
+		return fmt.Errorf("error parsing version argument: %w", err)
+	}
+
+	// Force the -o argument to come before the -v argument so that the output file can be
+	// updated on error.
+	if *out == "" {
+		return fmt.Errorf("-o argument must be passed before -v")
+	}
+
+	if vers != pathtools.BPGlobArgumentVersion {
+		return versionMismatchError
+	}
+
+	*v = true
+
+	return nil
 }
 
 type multiArg []string
@@ -54,34 +106,64 @@ func (m *multiArg) Get() interface{} {
 }
 
 func usage() {
-	fmt.Fprintln(os.Stderr, "usage: bpglob -o out glob")
+	fmt.Fprintln(os.Stderr, "usage: bpglob -o out -v version [-e excludes ...] glob")
-	flag.PrintDefaults()
+	flagSet.PrintDefaults()
 	os.Exit(2)
 }
 
 func main() {
-	flag.Parse()
+	// Save the command line flag error output to a buffer, the flag package unconditionally
+	// writes an error message to the output on error, and we want to hide the error for the
+	// version mismatch case.
+	flagErrorBuffer := &bytes.Buffer{}
+	flagSet.SetOutput(flagErrorBuffer)
+
+	err := flagSet.Parse(os.Args[1:])
+
+	if !versionMatch {
+		// A version mismatch error occurs when the arguments written into build-globs.ninja
+		// don't match the format expected by the bpglob binary.  This happens during the
+		// first incremental build after bpglob is changed.  Handle this case by aborting
+		// argument parsing and updating the output file with something that will always cause
+		// the primary builder to rerun.
+		// This can happen when there is no -v argument or if the -v argument doesn't match
+		// pathtools.BPGlobArgumentVersion.
+		writeErrorOutput(*out, versionMismatchError)
+		os.Exit(0)
+	}
+
+	if err != nil {
+		os.Stderr.Write(flagErrorBuffer.Bytes())
+		fmt.Fprintln(os.Stderr, "error:", err.Error())
+		usage()
+	}
 
 	if *out == "" {
 		fmt.Fprintln(os.Stderr, "error: -o is required")
 		usage()
 	}
 
-	if flag.NArg() != 1 {
+	if flagSet.NArg() != 1 {
 		usage()
 	}
 
-	_, err := pathtools.GlobWithDepFile(flag.Arg(0), *out, *out+".d", excludes)
+	_, err = pathtools.GlobWithDepFile(flagSet.Arg(0), *out, *out+".d", excludes)
 	if err != nil {
 		// Globs here were already run in the primary builder without error.  The only errors here should be if the glob
 		// pattern was made invalid by a change in the pathtools glob implementation, in which case the primary builder
 		// needs to be rerun anyways.  Update the output file with something that will always cause the primary builder
 		// to rerun.
-		s := fmt.Sprintf("%s: error: %s\n", time.Now().Format(time.StampNano), err.Error())
+		writeErrorOutput(*out, err)
-		err := ioutil.WriteFile(*out, []byte(s), 0666)
+	}
-		if err != nil {
+}
-			fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
+
-			os.Exit(1)
+// writeErrorOutput writes an error to the output file with a timestamp to ensure that it is
-		}
+// considered dirty by ninja.
+func writeErrorOutput(path string, globErr error) {
+	s := fmt.Sprintf("%s: error: %s\n", time.Now().Format(time.StampNano), globErr.Error())
+	err := ioutil.WriteFile(path, []byte(s), 0666)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
+		os.Exit(1)
 	}
 }

bootstrap/glob.go

@@ -45,7 +45,8 @@ var (
 	// and writes it to $out if it has changed, and writes the directories to $out.d
 	GlobRule = pctx.StaticRule("GlobRule",
 		blueprint.RuleParams{
-			Command:     fmt.Sprintf(`%s -o $out $excludes "$glob"`, globCmd),
+			Command: fmt.Sprintf(`%s -o $out -v %d $excludes "$glob"`,
+				globCmd, pathtools.BPGlobArgumentVersion),
 			CommandDeps: []string{globCmd},
 			Description: "glob $glob",
 

pathtools/glob.go

@@ -25,6 +25,12 @@ import (
 	"github.com/google/blueprint/deptools"
 )
 
+// BPGlobArgumentVersion is used to abort argument parsing early when the bpglob argument format
+// has changed but soong_build hasn't had a chance to rerun yet to update build-globs.ninja.
+// Increment it manually when changing the bpglob argument format.  It is located here because
+// pathtools is the only package that is shared between bpglob and bootstrap.
+const BPGlobArgumentVersion = 1
+
 var GlobMultipleRecursiveErr = errors.New("pattern contains multiple '**'")
 var GlobLastRecursiveErr = errors.New("pattern has '**' as last path element")
 var GlobInvalidRecursiveErr = errors.New("pattern contains other characters between '**' and path separator")