Remove the -v argument from bpglob. am: f12f1d7d1e am: ebe9022a3a am: f941e9c671

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

Change-Id: I36349e0616f8e378359d2d49083b9affa1a64f51

bootstrap/bpglob/bpglob.go

@@ -19,13 +19,10 @@
 package main
 
 import (
-	"bytes"
-	"errors"
 	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
-	"strconv"
 	"time"
 
 	"github.com/google/blueprint/deptools"
@@ -33,63 +30,14 @@ import (
 )
 
 var (
-	// flagSet is a flag.FlagSet with flag.ContinueOnError so that we can handle the versionMismatchError
+	out = flag.String("o", "", "file to write list of files that match glob")
-	// error from versionArg.
-	flagSet = flag.NewFlagSet("bpglob", flag.ContinueOnError)
 
-	out = flagSet.String("o", "", "file to write list of files that match glob")
+	globs []globArg
-
-	versionMatch versionArg
-	globs        []globArg
 )
 
 func init() {
-	flagSet.Var(&versionMatch, "v", "version number the command line was generated for")
+	flag.Var((*patternsArgs)(&globs), "p", "pattern to include in results")
-	flagSet.Var((*patternsArgs)(&globs), "p", "pattern to include in results")
+	flag.Var((*excludeArgs)(&globs), "e", "pattern to exclude from results from the most recent pattern")
-	flagSet.Var((*excludeArgs)(&globs), "e", "pattern to exclude from results from the most recent pattern")
-}
-
-// 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
 }
 
 // A glob arg holds a single -p argument with zero or more following -e arguments.
@@ -127,48 +75,24 @@ func (e *excludeArgs) Set(s string) error {
 }
 
 func usage() {
-	fmt.Fprintln(os.Stderr, "usage: bpglob -o out -v version -p glob [-e excludes ...] [-p glob ...]")
+	fmt.Fprintln(os.Stderr, "usage: bpglob -o out -p glob [-e excludes ...] [-p glob ...]")
-	flagSet.PrintDefaults()
+	flag.PrintDefaults()
 	os.Exit(2)
 }
 
 func main() {
-	// Save the command line flag error output to a buffer, the flag package unconditionally
+	flag.Parse()
-	// 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 flagSet.NArg() > 0 {
+	if flag.NArg() > 0 {
 		usage()
 	}
 
-	err = globsWithDepFile(*out, *out+".d", globs)
+	err := globsWithDepFile(*out, *out+".d", globs)
 	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

bootstrap/glob.go

@@ -54,8 +54,7 @@ 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(`$globCmd -o $out -v %d $args`,
+			Command:     "$globCmd -o $out $args",
-				pathtools.BPGlobArgumentVersion),
 			CommandDeps: []string{"$globCmd"},
 			Description: "glob",
 

pathtools/glob.go

@@ -24,12 +24,6 @@ import (
 	"strings"
 )
 
-// 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 = 2
-
 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")