From e134a7e3efab2b35ddcffd7dba7cc28d7a13dbbb Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 16:29:52 +0200 Subject: [PATCH 1/9] fix: CompileCommand may return extraneous files In cases where an argument that is not positional ends with `.go`, the `CompileCommand.GoFiles` function might erroneously return that value as a source file to be compiled. This is for example the case when compiling the `github.com/nats-io/nats.go` package, as the `-p` argument has a value ending in `.go`. To address this, this PR introduces a more reliable arguments parser that uses a generated parser (built using the standard `flag` package) based on the usage documentation found in the output of `go tool `, so that the flags are more clearly identified. This replaces the old arguments parser, which is no longer used. --- internal/toolexec/aspect/oncompile.go | 2 +- internal/toolexec/proxy/compile.flags.go | 87 ++++++++ internal/toolexec/proxy/compile.go | 28 ++- internal/toolexec/proxy/compile_test.go | 31 ++- internal/toolexec/proxy/flags.go | 95 -------- internal/toolexec/proxy/flags_test.go | 87 -------- internal/toolexec/proxy/generator/main.go | 255 ++++++++++++++++++++++ internal/toolexec/proxy/link.flags.go | 74 +++++++ internal/toolexec/proxy/link.go | 18 +- internal/toolexec/proxy/link_test.go | 7 +- internal/toolexec/proxy/proxy.go | 26 +-- internal/toolexec/proxy/proxy_test.go | 16 +- 12 files changed, 488 insertions(+), 238 deletions(-) create mode 100644 internal/toolexec/proxy/compile.flags.go delete mode 100644 internal/toolexec/proxy/flags.go delete mode 100644 internal/toolexec/proxy/flags_test.go create mode 100644 internal/toolexec/proxy/generator/main.go create mode 100644 internal/toolexec/proxy/link.flags.go diff --git a/internal/toolexec/aspect/oncompile.go b/internal/toolexec/aspect/oncompile.go index 621fc4cd..69fafb36 100644 --- a/internal/toolexec/aspect/oncompile.go +++ b/internal/toolexec/aspect/oncompile.go @@ -71,7 +71,7 @@ func (w Weaver) OnCompile(cmd *proxy.CompileCommand) error { RootConfig: map[string]string{"httpmode": "wrap"}, Lookup: imports.Lookup, ImportPath: w.ImportPath, - GoVersion: cmd.Flags.GoVersion, + GoVersion: cmd.Flags.Lang, ModifiedFile: func(file string) string { return filepath.Join(orchestrionDir, "src", cmd.Flags.Package, filepath.Base(file)) }, diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go new file mode 100644 index 00000000..ba7209a6 --- /dev/null +++ b/internal/toolexec/proxy/compile.flags.go @@ -0,0 +1,87 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023-present Datadog, Inc.) + +// Code generated by 'go generate' DO NOT EDIT. + +package proxy + +import "flag" + +func (f *compileFlagSet) parse(args []string) ([]string, error) { + flagSet := flag.NewFlagSet("compile", flag.ContinueOnError) + flagSet.Bool("%", false, "debug non-static initializers") + flagSet.Bool("+", false, "compiling runtime") + flagSet.Bool("B", false, "disable bounds checking") + flagSet.Bool("C", false, "disable printing of columns in error messages") + flagSet.String("D", "", "set relative path for local imports") + flagSet.Bool("E", false, "debug symbol export") + flagSet.String("I", "", "add directory to import search path") + flagSet.Bool("K", false, "debug missing line numbers") + flagSet.Bool("L", false, "also show actual source file names in error messages for positions affected by //line directives") + flagSet.Bool("N", false, "disable optimizations") + flagSet.Bool("S", false, "print assembly listing") + flagSet.BoolFunc("V", "print version and exit", func(string) error { + return nil + }) + flagSet.Bool("W", false, "debug parse tree after type checking") + flagSet.Bool("asan", false, "build code compatible with C/C++ address sanitizer") + flagSet.String("asmhdr", "", "write assembly header to file") + flagSet.String("bench", "", "append benchmark times to file") + flagSet.String("blockprofile", "", "write block profile to file") + flagSet.String("buildid", "", "record id as the build id in the export metadata") + flagSet.Int("c", 10, "concurrency during compilation (1 means no concurrency)") + flagSet.Bool("clobberdead", false, "clobber dead stack slots (for debugging)") + flagSet.Bool("clobberdeadreg", false, "clobber dead registers (for debugging)") + flagSet.Bool("complete", false, "compiling complete package (no C or assembly)") + flagSet.String("coveragecfg", "", "read coverage configuration from file") + flagSet.String("cpuprofile", "", "write cpu profile to file") + flagSet.String("d", "", "enable debugging settings; try -d help") + flagSet.Bool("dwarf", true, "generate DWARF symbols") + flagSet.Bool("dwarfbasentries", false, "use base address selection entries in DWARF") + flagSet.Bool("dwarflocationlists", true, "add location lists to DWARF in optimized mode") + flagSet.Bool("dynlink", false, "support references to Go symbols defined in other shared libraries") + flagSet.Bool("e", false, "no limit on number of errors reported") + flagSet.String("embedcfg", "", "read go:embed configuration from file") + flagSet.String("env", "", "add definition of the form key=value to environment") + flagSet.Bool("errorurl", false, "print explanatory URL with error message if applicable") + flagSet.Int("gendwarfinl", 2, "generate DWARF inline info records") + flagSet.String("goversion", "", "required version of the runtime") + flagSet.Bool("h", false, "halt on error") + flagSet.StringVar(&f.ImportCfg, "importcfg", "", "read import configuration from file") + flagSet.String("installsuffix", "", "set pkg directory suffix") + flagSet.Bool("j", false, "debug runtime-initialized variables") + flagSet.String("json", "", "version,file for JSON compiler/optimizer detail output") + flagSet.Bool("l", false, "disable inlining") + flagSet.StringVar(&f.Lang, "lang", "", "Go language version source code expects") + flagSet.String("linkobj", "", "write linker-specific object to file") + flagSet.Bool("linkshared", false, "generate code that will be linked against Go shared libraries") + flagSet.Bool("live", false, "debug liveness analysis") + flagSet.Bool("m", false, "print optimization decisions") + flagSet.String("memprofile", "", "write memory profile to file") + flagSet.String("memprofilerate", "", "set runtime.MemProfileRate to rate") + flagSet.Bool("msan", false, "build code compatible with C/C++ memory sanitizer") + flagSet.String("mutexprofile", "", "write mutex profile to file") + flagSet.Bool("nolocalimports", false, "reject local (relative) imports") + flagSet.StringVar(&f.Output, "o", "", "write output to file") + flagSet.StringVar(&f.Package, "p", "", "set expected package import path") + flagSet.Bool("pack", false, "write to file.a instead of file.o") + flagSet.String("pgoprofile", "", "read profile or pre-process profile from file") + flagSet.Bool("r", false, "debug generated wrappers") + flagSet.Bool("race", false, "enable race detector") + flagSet.Bool("shared", false, "generate code that can be linked into a shared library") + flagSet.Bool("smallframes", false, "reduce the size limit for stack allocated objects") + flagSet.String("spectre", "", "enable spectre mitigations in list (all, index, ret)") + flagSet.Bool("std", false, "compiling standard library") + flagSet.String("symabis", "", "read symbol ABIs from file") + flagSet.Bool("t", false, "enable tracing for debugging the compiler") + flagSet.String("traceprofile", "", "write an execution trace to file") + flagSet.String("trimpath", "", "remove prefix from recorded source file paths") + flagSet.Bool("v", false, "increase debug verbosity") + flagSet.Bool("w", false, "debug type checking") + flagSet.Bool("wb", true, "enable write barrier") + + err := flagSet.Parse(args) + return flagSet.Args(), err +} diff --git a/internal/toolexec/proxy/compile.go b/internal/toolexec/proxy/compile.go index 3a85488e..7930afe1 100644 --- a/internal/toolexec/proxy/compile.go +++ b/internal/toolexec/proxy/compile.go @@ -11,28 +11,35 @@ import ( "strings" ) +//go:generate go run github.com/datadog/orchestrion/internal/toolexec/proxy/generator -command=compile + type compileFlagSet struct { - Package string `ddflag:"-p"` - ImportCfg string `ddflag:"-importcfg"` - Output string `ddflag:"-o"` - TrimPath string `ddflag:"-trimpath"` - GoVersion string `ddflag:"-goversion"` + Package string `ddflag:"-p"` + ImportCfg string `ddflag:"-importcfg"` + Output string `ddflag:"-o"` + Lang string `ddflag:"-lang"` + showVersion bool `ddflag:"-V"` } // CompileCommand represents a go tool `compile` invocation type CompileCommand struct { command Flags compileFlagSet + Files []string // WorkDir is the $WORK directory managed by the go toolchain. WorkDir string } func (*CompileCommand) Type() CommandType { return CommandTypeCompile } +func (c *CompileCommand) ShowVersion() bool { + return c.Flags.showVersion +} + // GoFiles returns the list of Go files passed as arguments to cmd func (cmd *CompileCommand) GoFiles() []string { - files := make([]string, 0, len(cmd.args)) - for _, path := range cmd.args { + files := make([]string, 0, len(cmd.Files)) + for _, path := range cmd.Files { if !strings.HasSuffix(path, ".go") { continue } @@ -47,6 +54,7 @@ func (cmd *CompileCommand) GoFiles() []string { func (cmd *CompileCommand) AddFiles(files []string) { paramIdx := len(cmd.args) cmd.args = append(cmd.args, files...) + cmd.Files = append(cmd.Files, files...) for i, f := range files { cmd.paramPos[f] = paramIdx + i } @@ -57,7 +65,11 @@ func parseCompileCommand(args []string) (Command, error) { return nil, errors.New("unexpected number of command arguments") } cmd := CompileCommand{command: NewCommand(args)} - parseFlags(&cmd.Flags, args[1:]) + pos, err := cmd.Flags.parse(args[1:]) + if err != nil { + return nil, err + } + cmd.Files = pos if cmd.Flags.ImportCfg != "" { // The WorkDir is the parent of the stage directory, which is where the importcfg file is located. diff --git a/internal/toolexec/proxy/compile_test.go b/internal/toolexec/proxy/compile_test.go index 5fbd11d9..d2f8acd9 100644 --- a/internal/toolexec/proxy/compile_test.go +++ b/internal/toolexec/proxy/compile_test.go @@ -6,7 +6,6 @@ package proxy import ( - "reflect" "testing" "github.com/stretchr/testify/require" @@ -15,33 +14,49 @@ import ( func TestParseCompile(t *testing.T) { for name, tc := range map[string]struct { input []string - stage string goFiles []string flags compileFlagSet }{ "version_print": { input: []string{"/path/compile", "-V=full"}, - stage: ".", + flags: compileFlagSet{ + showVersion: true, + }, }, "compile": { - input: []string{"/path/compile", "-o", "/buildDir/b002/a.out", "-p", "mypackage", "-goversion", "go1.42.1337", "-importcfg", "/buildDir/b002/importcfg", "/source/dir/main.go", "/source/dir/file1.go"}, - stage: "b002", + input: []string{"/path/compile", "-o", "/buildDir/b002/a.out", "-p", "mypackage", "-lang=go1.42", "-importcfg", "/buildDir/b002/importcfg", "/source/dir/main.go", "/source/dir/file1.go"}, goFiles: []string{"/source/dir/main.go", "/source/dir/file1.go"}, flags: compileFlagSet{ Package: "mypackage", ImportCfg: "/buildDir/b002/importcfg", Output: "/buildDir/b002/a.out", - GoVersion: "go1.42.1337", + Lang: "go1.42", + }, + }, + "nats.go": { + input: []string{"/path/compile", "-o", "/buildDir/b002/a.out", "-p", "github.com/nats-io/nats.go", "-complete", "/path/to/source/file.go"}, + goFiles: []string{"/path/to/source/file.go"}, + flags: compileFlagSet{ + Package: "github.com/nats-io/nats.go", + Output: "/buildDir/b002/a.out", }, }, } { + if tc.goFiles == nil { + // Simplify comparisons, as goFiles always returns non-nil + tc.goFiles = []string{} + } + + if name != "nats.go" { + continue + } t.Run(name, func(t *testing.T) { cmd, err := parseCompileCommand(tc.input) require.NoError(t, err) require.Equal(t, CommandTypeCompile, cmd.Type()) - require.Equal(t, tc.stage, cmd.Stage()) c := cmd.(*CompileCommand) - require.True(t, reflect.DeepEqual(tc.flags, c.Flags)) + require.Equal(t, tc.flags, c.Flags) + require.EqualValues(t, tc.goFiles, c.GoFiles()) }) } } diff --git a/internal/toolexec/proxy/flags.go b/internal/toolexec/proxy/flags.go deleted file mode 100644 index 8415a8cd..00000000 --- a/internal/toolexec/proxy/flags.go +++ /dev/null @@ -1,95 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc. - -package proxy - -import ( - "fmt" - "reflect" - "strings" -) - -const structTagKey = "ddflag" - -// parseFlags walks through the given arguments and sets the flagSet values -// present in the argument list. Unknown options, not present in the flagSet -// are accepted and skipped. The argument list is not modified. -func parseFlags(flagSet any, args []string) { - flagSetValueMap := makeFlagSetValueMap(flagSet) - - i := 0 - for i < len(args)-1 { - _, shift := parseOption(flagSetValueMap, args[i], args[i+1]) - i += shift - } - - if i < len(args) { - _, _ = parseOption(flagSetValueMap, args[i], "") - } -} - -func makeFlagSetValueMap(flagSet any) map[string]reflect.Value { - v := reflect.ValueOf(flagSet).Elem() - if v.Kind() != reflect.Struct { - panic(fmt.Errorf("flagSet type %T is not a struct", flagSet)) - } - typ := v.Type() - flagSetValueMap := make(map[string]reflect.Value, v.NumField()) - for i := 0; i < typ.NumField(); i++ { - field := typ.Field(i) - if tag, ok := field.Tag.Lookup(structTagKey); ok { - flagSetValueMap[tag] = v.Field(i) - } - } - return flagSetValueMap -} - -// parseOption parses the given current argument and following one according to -// the go Flags syntax. -func parseOption(flagSetValueMap map[string]reflect.Value, arg string, nextArg string) (nonOpt bool, shift int) { - if arg[0] != '-' { - // Not an option, return the value and shift by one. - return true, 1 - } - - // Split the argument by its first `=` character if any, and check the - // syntax being used. - option, value, hasValue := strings.Cut(arg, "=") - flag, exists := flagSetValueMap[option] - - if hasValue { - // `-opt=val` syntax - shift = 1 - if exists { - flag.SetString(value) - } - } else if nextArg != "" && !strings.HasPrefix(nextArg, "-") { - // `-opt val` syntax - value := nextArg - shift = 2 - if exists { - switch flag.Kind() { - case reflect.String: - flag.SetString(value) - case reflect.Bool: - flag.SetBool(true) - shift = 1 - default: - panic(fmt.Errorf("unsupported value kind: %s", flag.Kind())) - } - } - } else { - // `-opt` syntax (no value) - shift = 1 - if exists { - if flag.Kind() != reflect.Bool { - panic(fmt.Sprintf("missing value for %s flag", flag.Kind())) - } - flag.SetBool(true) - } - } - - return -} diff --git a/internal/toolexec/proxy/flags_test.go b/internal/toolexec/proxy/flags_test.go deleted file mode 100644 index a4a387f7..00000000 --- a/internal/toolexec/proxy/flags_test.go +++ /dev/null @@ -1,87 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc. - -package proxy - -import ( - "reflect" - "testing" - - "github.com/stretchr/testify/require" -) - -type testFlagSet struct { - FlagStr string `ddflag:"-flagStr"` - FlagBool bool `ddflag:"-flagBool"` -} - -func TestParseFlags(t *testing.T) { - for name, tc := range map[string]struct { - args []string - expected testFlagSet - panic bool - }{ - "flagStr/plain": { - args: []string{"-flagStr", "test"}, - expected: testFlagSet{ - FlagStr: "test", - }, - }, - "flagStr/assignment": { - args: []string{"-flagStr=test"}, - expected: testFlagSet{ - FlagStr: "test", - }, - }, - "flagStr/assignment-empty": { - args: []string{"-flagStr="}, - }, - "flagBool": { - args: []string{"-flagBool"}, - expected: testFlagSet{ - FlagBool: true, - }, - }, - "combined/1": { - args: []string{"-flagStr", "test", "-flagBool"}, - expected: testFlagSet{ - FlagStr: "test", - FlagBool: true, - }, - }, - "combined/2": { - args: []string{"-flagStr=test", "-flagBool"}, - expected: testFlagSet{ - FlagStr: "test", - FlagBool: true, - }, - }, - "combined/3": { - args: []string{"-flagBool", "-flagStr", "test"}, - expected: testFlagSet{ - FlagStr: "test", - FlagBool: true, - }, - }, - "invalid/flagStr/1": { - args: []string{"-flagStr", "-flagBool"}, - panic: true, - }, - "invalid/flagStr/2": { - args: []string{"-flagStr"}, - panic: true, - }, - } { - t.Run(name, func(t *testing.T) { - defer func() { - r := recover() - require.Equal(t, tc.panic, r != nil) - }() - flags := testFlagSet{} - parseFlags(&flags, tc.args) - require.True(t, reflect.DeepEqual(tc.expected, flags)) - }) - } -} diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go new file mode 100644 index 00000000..ffd5feac --- /dev/null +++ b/internal/toolexec/proxy/generator/main.go @@ -0,0 +1,255 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023-present Datadog, Inc. + +package main + +import ( + "bufio" + "bytes" + "flag" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io" + "log" + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/dave/jennifer/jen" +) + +type flagSpec struct { + Flag string + Value string + Descr []string +} + +func main() { + var command string + flag.StringVar(&command, "command", "", "The command to generate a parser for") + flag.Parse() + + if command == "" { + log.Fatalln("Missing value for required -command flag") + } + + cmd := exec.Command("go", "tool", command) + cmd.Env = append(cmd.Env, "LANG=C") + var buffer bytes.Buffer + cmd.Stderr = &buffer + _ = cmd.Run() // The command is expected to fail... + + reader := bufio.NewReader(&buffer) + // Ensure the first line looks like usage instructions... + if line, _, err := reader.ReadLine(); err != nil { + log.Fatalln(err) + } else if !bytes.HasPrefix(line, []byte("usage: ")) { + log.Fatalf("Unexpected output from command:\n%s\n", string(line)) + } + + var ( + flags []flagSpec + knownFlags = make(map[string]struct{}) + ) + reFlag := regexp.MustCompile(`^ (-[^\s]+)(?: ([^\s]+))?(\t.+)?$`) + for { + line, _, err := reader.ReadLine() + if err == io.EOF { + break + } + if err != nil { + log.Fatalln(err) + } + matches := reFlag.FindSubmatch(line) + if len(matches) == 0 { + text := strings.TrimSpace(string(line)) + if text != "" && len(flags) > 0 { + flags[len(flags)-1].Descr = append(flags[len(flags)-1].Descr, text) + } + continue + } + + spec := flagSpec{ + Flag: string(matches[1]), + Value: string(matches[2]), + } + if descr := strings.TrimSpace(string(matches[3])); descr != "" { + spec.Descr = append(spec.Descr, descr) + } + flags = append(flags, spec) + knownFlags[spec.Flag] = struct{}{} + } + + if len(flags) == 0 { + log.Fatalln("No flags found in command usage, this is unexpected!") + } + + var ( + goFile = requireEnv("GOFILE") + goPackage = requireEnv("GOPACKAGE") + ) + + fset := token.NewFileSet() + src, err := os.ReadFile(goFile) + if err != nil { + log.Fatalln(err) + } + parsed, err := parser.ParseFile(fset, goFile, src, 0) + if err != nil { + log.Fatalln(err) + } + + typeName := fmt.Sprintf("%sFlagSet", command) + capturedFlags := make(map[string]string) + for _, decl := range parsed.Decls { + decl, ok := decl.(*ast.GenDecl) + if !ok || decl.Tok != token.TYPE { + continue + } + for _, spec := range decl.Specs { + spec, ok := spec.(*ast.TypeSpec) + if !ok || spec.Name.Name != typeName { + continue + } + strct := spec.Type.(*ast.StructType) + for _, field := range strct.Fields.List { + tag, err := strconv.Unquote(field.Tag.Value) + if err != nil { + log.Fatalln(err) + } + flag, err := strconv.Unquote(strings.TrimPrefix(tag, "ddflag:")) + if err != nil { + log.Fatalln(err) + } + if _, isKnown := knownFlags[flag]; !isKnown { + log.Fatalf("Unknown captured flag: %q\n", flag) + } + capturedFlags[flag] = field.Names[0].Name + } + } + } + + file := jen.NewFile(goPackage) + file.HeaderComment("// Unless explicitly stated otherwise all files in this repository are licensed") + file.HeaderComment("// under the Apache License Version 2.0.") + file.HeaderComment("// This product includes software developed at Datadog (https://www.datadoghq.com/).") + file.HeaderComment("// Copyright 2023-present Datadog, Inc.)\n") + file.HeaderComment("// Code generated by 'go generate' DO NOT EDIT.") + + file.Func(). + Params(jen.Id("f").Op("*").Id(typeName)). + Id("parse"). + Params(jen.Id("args").Index().String()). + Params( + jen.Index().String(), + jen.Error(), + ). + BlockFunc(func(g *jen.Group) { + g.Id("flagSet"). + Op(":="). + Qual("flag", "NewFlagSet"). + Call( + jen.Lit(command), + jen.Qual("flag", "ContinueOnError"), + ) + + reDefault := regexp.MustCompile(`^(.+?)(?:\s+\(default\s+(.+?)\))?$`) + for _, spec := range flags { + fieldName, captured := capturedFlags[spec.Flag] + + var ( + funcName string + strDefault string + defaultValue *jen.Statement + ) + + descr := strings.Join(spec.Descr, " ") + if matches := reDefault.FindStringSubmatch(descr); matches[2] != "" { + strDefault = matches[2] + descr = matches[1] + } + + switch spec.Value { + case "": + funcName = "Bool" + if strDefault == "" || strDefault == "false" { + defaultValue = jen.False() + } else { + defaultValue = jen.True() + } + case "int": + funcName = "Int" + if strDefault != "" { + val, err := strconv.Atoi(strDefault) + if err != nil { + log.Fatalf("Invalid default value for an int flag: %s\n", strDefault) + } + defaultValue = jen.Lit(val) + } else { + defaultValue = jen.Lit(0) + } + default: + funcName = "String" + defaultValue = jen.Lit(strDefault) + } + + flagName := jen.Lit(spec.Flag[1:]) + + if funcName == "Bool" && spec.Flag == "-V" { + var handler *jen.Statement + if captured { + handler = jen.Func().Params(jen.String()).Error().Block( + jen.Id("f").Dot(fieldName).Op("=").True(), + jen.Return(jen.Nil()), + ) + } else { + handler = jen.Func().Params(jen.String()).Error().Block(jen.Return(jen.Nil())) + } + g.Id("flagSet").Dot("BoolFunc").Call(flagName, jen.Lit(descr), handler) + } else if captured { + g.Id("flagSet").Dot(funcName+"Var").Call(jen.Op("&").Id("f").Dot(fieldName), flagName, defaultValue, jen.Lit(descr)) + } else { + g.Id("flagSet").Dot(funcName).Call(flagName, defaultValue, jen.Lit(descr)) + } + } + g.Line() + + g.Id("err").Op(":=").Id("flagSet").Dot("Parse").Call(jen.Id("args")) + g.Return( + jen.Id("flagSet").Dot("Args").Call(), + jen.Id("err"), + ) + }) + + out, err := os.Create(filepath.Join(goFile, "..", fmt.Sprintf("%s.flags.go", command))) + if err != nil { + log.Fatalln(err) + } + defer out.Close() + + if err := file.Render(out); err != nil { + log.Fatalln(err) + } +} + +func (f *flagSpec) String() string { + if f.Value != "" { + return fmt.Sprintf("%s %s", f.Flag, f.Value) + } + return f.Flag +} + +func requireEnv(name string) string { + val := os.Getenv(name) + if val == "" { + log.Fatalf("Missing environment variable: $%s\n", name) + } + return val +} diff --git a/internal/toolexec/proxy/link.flags.go b/internal/toolexec/proxy/link.flags.go new file mode 100644 index 00000000..3dbd7e8b --- /dev/null +++ b/internal/toolexec/proxy/link.flags.go @@ -0,0 +1,74 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023-present Datadog, Inc.) + +// Code generated by 'go generate' DO NOT EDIT. + +package proxy + +import "flag" + +func (f *linkFlagSet) parse(args []string) ([]string, error) { + flagSet := flag.NewFlagSet("link", flag.ContinueOnError) + flagSet.String("B", "", "add an ELF NT_GNU_BUILD_ID note when using ELF; use \"gobuildid\" to generate it from the Go build ID") + flagSet.String("E", "", "set entry symbol name") + flagSet.String("H", "", "set header type") + flagSet.String("I", "", "use linker as ELF dynamic linker") + flagSet.String("L", "", "add specified directory to library path") + flagSet.String("R", "-1", "set address rounding quantum") + flagSet.Int("T", -1, "set the start address of text symbols") + flagSet.BoolFunc("V", "print version and exit", func(string) error { + f.showVersion = true + return nil + }) + flagSet.String("X", "", "add string value definition of the form importpath.name=value") + flagSet.Bool("a", false, "no-op (deprecated)") + flagSet.Bool("asan", false, "enable ASan interface") + flagSet.Bool("aslr", true, "enable ASLR for buildmode=c-shared on windows") + flagSet.String("benchmark", "", "set to 'mem' or 'cpu' to enable phase benchmarking") + flagSet.String("benchmarkprofile", "", "emit phase profiles to base_phase.{cpu,mem}prof") + flagSet.Bool("bindnow", false, "mark a dynamically linked ELF object for immediate function binding") + flagSet.String("buildid", "", "record id as Go toolchain build id") + flagSet.StringVar(&f.BuildMode, "buildmode", "", "set build mode") + flagSet.Bool("c", false, "dump call graph") + flagSet.String("capturehostobjs", "", "capture host object files loaded during internal linking to specified dir") + flagSet.Bool("checklinkname", true, "check linkname symbol references") + flagSet.Bool("compressdwarf", true, "compress DWARF if possible") + flagSet.String("cpuprofile", "", "write cpu profile to file") + flagSet.Bool("d", false, "disable dynamic executable") + flagSet.Bool("debugnosplit", false, "dump nosplit call graph") + flagSet.Int("debugtextsize", 0, "debug text section max size") + flagSet.Int("debugtramp", 0, "debug trampolines") + flagSet.Bool("dumpdep", false, "dump symbol dependency graph") + flagSet.String("extar", "", "archive program for buildmode=c-archive") + flagSet.String("extld", "", "use linker when linking in external mode") + flagSet.String("extldflags", "", "pass flags to external linker") + flagSet.Bool("f", false, "ignore version mismatch") + flagSet.Bool("g", false, "disable go package data checks") + flagSet.Bool("h", false, "halt on error") + flagSet.StringVar(&f.ImportCfg, "importcfg", "", "read import configuration from file") + flagSet.String("installsuffix", "", "set package directory suffix") + flagSet.String("k", "", "set field tracking symbol") + flagSet.String("libgcc", "", "compiler support lib for internal linking; use \"none\" to disable") + flagSet.String("linkmode", "", "set link mode") + flagSet.Bool("linkshared", false, "link against installed Go shared libraries") + flagSet.String("memprofile", "", "write memory profile to file") + flagSet.String("memprofilerate", "", "set runtime.MemProfileRate to rate") + flagSet.Bool("msan", false, "enable MSan interface") + flagSet.Bool("n", false, "no-op (deprecated)") + flagSet.StringVar(&f.Output, "o", "", "write output to file") + flagSet.String("pluginpath", "", "full path name for plugin") + flagSet.Bool("pruneweakmap", true, "prune weak mapinit refs") + flagSet.String("r", "", "set the ELF dynamic linker search path to dir1:dir2:...") + flagSet.Bool("race", false, "enable race detector") + flagSet.Int("randlayout", 0, "randomize function layout") + flagSet.Bool("s", false, "disable symbol table") + flagSet.Int("strictdups", 0, "sanity check duplicate symbol contents during object file reading (1=warn 2=err).") + flagSet.String("tmpdir", "", "use directory for temporary files") + flagSet.Bool("v", false, "print link trace") + flagSet.Bool("w", false, "disable DWARF generation") + + err := flagSet.Parse(args) + return flagSet.Args(), err +} diff --git a/internal/toolexec/proxy/link.go b/internal/toolexec/proxy/link.go index ecf2094f..9b81bdf7 100644 --- a/internal/toolexec/proxy/link.go +++ b/internal/toolexec/proxy/link.go @@ -10,10 +10,13 @@ import ( "path/filepath" ) +//go:generate go run github.com/datadog/orchestrion/internal/toolexec/proxy/generator -command=link + type linkFlagSet struct { - BuildMode string `ddflag:"-buildmode"` - ImportCfg string `ddflag:"-importcfg"` - Output string `ddflag:"-o"` + BuildMode string `ddflag:"-buildmode"` + ImportCfg string `ddflag:"-importcfg"` + Output string `ddflag:"-o"` + showVersion bool `ddflag:"-V"` } // LinkCommand represents a go tool `link` invocation @@ -28,6 +31,10 @@ func (*LinkCommand) Type() CommandType { return CommandTypeLink } +func (cmd *LinkCommand) ShowVersion() bool { + return cmd.Flags.showVersion +} + func (cmd *LinkCommand) Stage() string { return filepath.Base(filepath.Dir(filepath.Dir(cmd.Flags.Output))) } @@ -37,7 +44,10 @@ func parseLinkCommand(args []string) (Command, error) { return nil, errors.New("unexpected number of command arguments") } flags := &linkFlagSet{} - parseFlags(flags, args[1:]) + _, err := flags.parse(args[1:]) + if err != nil { + return nil, err + } // The WorkDir is the parent of the stage dir, and the ImportCfg file is directly in the stage dir. workDir := filepath.Dir(filepath.Dir(flags.ImportCfg)) diff --git a/internal/toolexec/proxy/link_test.go b/internal/toolexec/proxy/link_test.go index faea9017..7d84608a 100644 --- a/internal/toolexec/proxy/link_test.go +++ b/internal/toolexec/proxy/link_test.go @@ -15,16 +15,16 @@ import ( func TestParseLink(t *testing.T) { for name, tc := range map[string]struct { input []string - stage string flags linkFlagSet }{ "version_print": { input: []string{"/path/link", "-V=full"}, - stage: ".", + flags: linkFlagSet{ + showVersion: true, + }, }, "link": { input: []string{"/path/link", "-o", "/buildDir/b001/exe/a.out", "-importcfg", "/buildDir/b001/importcfg.link", "-buildmode=exe", "/buildDir/b001/_pkg_.a"}, - stage: "b001", flags: linkFlagSet{ ImportCfg: "/buildDir/b001/importcfg.link", Output: "/buildDir/b001/exe/a.out", @@ -36,7 +36,6 @@ func TestParseLink(t *testing.T) { cmd, err := parseLinkCommand(tc.input) require.NoError(t, err) require.Equal(t, CommandTypeLink, cmd.Type()) - require.Equal(t, tc.stage, cmd.Stage()) c := cmd.(*LinkCommand) require.True(t, reflect.DeepEqual(tc.flags, c.Flags)) }) diff --git a/internal/toolexec/proxy/proxy.go b/internal/toolexec/proxy/proxy.go index b5c038d2..2f86dfc1 100644 --- a/internal/toolexec/proxy/proxy.go +++ b/internal/toolexec/proxy/proxy.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "os/exec" - "path/filepath" ) type ( @@ -26,10 +25,7 @@ type ( // Args are all the command arguments, starting from the Go tool command Args() []string ReplaceParam(param string, val string) error - // Stage returns the build stage of the command. Each stage usually associated - // to a specific package and is named using the `bXXX` format, where `X` are numbers. - // Stage b001 is the final stage of the go build process - Stage() string + // Type represents the go tool command type (compile, link, asm, etc.) Type() CommandType @@ -44,11 +40,6 @@ type ( // processors will be invoked. CommandProcessor[T Command] func(T) error - commandFlagSet struct { - Output string `ddflag:"-o"` - Version string `ddflag:"-V"` - } - // command is the default unknown command type // Can be used to compose specific Command implementations command struct { @@ -56,7 +47,6 @@ type ( // paramPos is the index in args of the *value* provided for the parameter stored in the key paramPos map[string]int onClose []func() error - flags commandFlagSet } ) @@ -89,8 +79,6 @@ func NewCommand(args []string) command { cmd.paramPos[v] = pos + 1 } - parseFlags(&cmd.flags, args) - return cmd } @@ -109,10 +97,6 @@ func (cmd *command) Close() error { return nil } -func (cmd *command) ShowVersion() bool { - return cmd.flags.Version == "full" -} - // ReplaceParam will replace any parameter of the command provided it is found // A parameter can be a flag, an option, a value, etc func (cmd *command) ReplaceParam(param string, val string) error { @@ -162,10 +146,6 @@ func MustRunCommand(cmd Command, opts ...RunCommandOption) { panic(err) } -func (cmd *command) Stage() string { - return filepath.Base(filepath.Dir(cmd.flags.Output)) -} - func (*command) Type() CommandType { return CommandTypeOther } @@ -173,3 +153,7 @@ func (*command) Type() CommandType { func (cmd *command) Args() []string { return cmd.args } + +func (*command) ShowVersion() bool { + return false +} diff --git a/internal/toolexec/proxy/proxy_test.go b/internal/toolexec/proxy/proxy_test.go index ff2d2fb3..6920a1be 100644 --- a/internal/toolexec/proxy/proxy_test.go +++ b/internal/toolexec/proxy/proxy_test.go @@ -56,25 +56,21 @@ func TestParseCommand(t *testing.T) { expectedStage string }{ "unknown": { - input: []string{"unknown", "irrelevant"}, - expectedType: proxy.CommandTypeOther, - expectedStage: ".", + input: []string{"unknown", "irrelevant"}, + expectedType: proxy.CommandTypeOther, }, "compile": { - input: []string{"compile", "-o", "b002/a.out", "main.go"}, - expectedType: proxy.CommandTypeCompile, - expectedStage: "b002", + input: []string{"compile", "-o", "b002/a.out", "main.go"}, + expectedType: proxy.CommandTypeCompile, }, "link": { - input: []string{"link", "-o", "b001/out/a.out", "main.go"}, - expectedType: proxy.CommandTypeLink, - expectedStage: "b001", + input: []string{"link", "-o", "b001/out/a.out", "main.go"}, + expectedType: proxy.CommandTypeLink, }, } { t.Run(name, func(t *testing.T) { cmd := proxy.MustParseCommand(tc.input) require.Equal(t, tc.expectedType, cmd.Type()) - require.Equal(t, tc.expectedStage, cmd.Stage()) require.True(t, reflect.DeepEqual(tc.input, cmd.Args())) }) } From eff607958c83ca40f2b478bad5b3da655d81595d Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 16:47:43 +0200 Subject: [PATCH 2/9] Forgot some changes --- internal/cmd/toolexec.go | 2 +- internal/toolexec/proxy/compile.flags.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cmd/toolexec.go b/internal/cmd/toolexec.go index 574533b4..1c0f4727 100644 --- a/internal/cmd/toolexec.go +++ b/internal/cmd/toolexec.go @@ -63,7 +63,7 @@ var Toolexec = &cli.Command{ return err } - log.Tracef("Toolexec final command: %q/n", proxyCmd.Args()) + log.Tracef("Toolexec final command: %q\n", proxyCmd.Args()) return proxy.RunCommand(proxyCmd) }, } diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go index ba7209a6..ae3a70f3 100644 --- a/internal/toolexec/proxy/compile.flags.go +++ b/internal/toolexec/proxy/compile.flags.go @@ -23,6 +23,7 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.Bool("N", false, "disable optimizations") flagSet.Bool("S", false, "print assembly listing") flagSet.BoolFunc("V", "print version and exit", func(string) error { + f.showVersion = true return nil }) flagSet.Bool("W", false, "debug parse tree after type checking") From d740c854f425512cd9ff3202b6c59e646e45d000 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 16:50:35 +0200 Subject: [PATCH 3/9] chore: update generated files --- internal/toolexec/proxy/compile.flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go index ae3a70f3..d781102b 100644 --- a/internal/toolexec/proxy/compile.flags.go +++ b/internal/toolexec/proxy/compile.flags.go @@ -32,7 +32,7 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.String("bench", "", "append benchmark times to file") flagSet.String("blockprofile", "", "write block profile to file") flagSet.String("buildid", "", "record id as the build id in the export metadata") - flagSet.Int("c", 10, "concurrency during compilation (1 means no concurrency)") + flagSet.Int("c", 4, "concurrency during compilation (1 means no concurrency)") flagSet.Bool("clobberdead", false, "clobber dead stack slots (for debugging)") flagSet.Bool("clobberdeadreg", false, "clobber dead registers (for debugging)") flagSet.Bool("complete", false, "compiling complete package (no C or assembly)") @@ -40,7 +40,7 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.String("cpuprofile", "", "write cpu profile to file") flagSet.String("d", "", "enable debugging settings; try -d help") flagSet.Bool("dwarf", true, "generate DWARF symbols") - flagSet.Bool("dwarfbasentries", false, "use base address selection entries in DWARF") + flagSet.Bool("dwarfbasentries", true, "use base address selection entries in DWARF") flagSet.Bool("dwarflocationlists", true, "add location lists to DWARF in optimized mode") flagSet.Bool("dynlink", false, "support references to Go symbols defined in other shared libraries") flagSet.Bool("e", false, "no limit on number of errors reported") From a1fb2769ebd15a1b167e803420fb8d0d200157e3 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 16:58:44 +0200 Subject: [PATCH 4/9] Make linter happy --- internal/toolexec/proxy/compile.flags.go | 2 +- internal/toolexec/proxy/compile.go | 4 ++-- internal/toolexec/proxy/compile_test.go | 4 ++-- internal/toolexec/proxy/generator/main.go | 5 ++++- internal/toolexec/proxy/link.flags.go | 2 +- internal/toolexec/proxy/link.go | 4 ++-- internal/toolexec/proxy/link_test.go | 2 +- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go index d781102b..39a044e6 100644 --- a/internal/toolexec/proxy/compile.flags.go +++ b/internal/toolexec/proxy/compile.flags.go @@ -23,7 +23,7 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.Bool("N", false, "disable optimizations") flagSet.Bool("S", false, "print assembly listing") flagSet.BoolFunc("V", "print version and exit", func(string) error { - f.showVersion = true + f.ShowVersion = true return nil }) flagSet.Bool("W", false, "debug parse tree after type checking") diff --git a/internal/toolexec/proxy/compile.go b/internal/toolexec/proxy/compile.go index 7930afe1..133960dc 100644 --- a/internal/toolexec/proxy/compile.go +++ b/internal/toolexec/proxy/compile.go @@ -18,7 +18,7 @@ type compileFlagSet struct { ImportCfg string `ddflag:"-importcfg"` Output string `ddflag:"-o"` Lang string `ddflag:"-lang"` - showVersion bool `ddflag:"-V"` + ShowVersion bool `ddflag:"-V"` } // CompileCommand represents a go tool `compile` invocation @@ -33,7 +33,7 @@ type CompileCommand struct { func (*CompileCommand) Type() CommandType { return CommandTypeCompile } func (c *CompileCommand) ShowVersion() bool { - return c.Flags.showVersion + return c.Flags.ShowVersion } // GoFiles returns the list of Go files passed as arguments to cmd diff --git a/internal/toolexec/proxy/compile_test.go b/internal/toolexec/proxy/compile_test.go index d2f8acd9..8665d14c 100644 --- a/internal/toolexec/proxy/compile_test.go +++ b/internal/toolexec/proxy/compile_test.go @@ -20,7 +20,7 @@ func TestParseCompile(t *testing.T) { "version_print": { input: []string{"/path/compile", "-V=full"}, flags: compileFlagSet{ - showVersion: true, + ShowVersion: true, }, }, "compile": { @@ -44,7 +44,7 @@ func TestParseCompile(t *testing.T) { } { if tc.goFiles == nil { // Simplify comparisons, as goFiles always returns non-nil - tc.goFiles = []string{} + tc.goFiles = make([]string, 0) } if name != "nats.go" { diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go index ffd5feac..fdb53076 100644 --- a/internal/toolexec/proxy/generator/main.go +++ b/internal/toolexec/proxy/generator/main.go @@ -118,7 +118,10 @@ func main() { if !ok || spec.Name.Name != typeName { continue } - strct := spec.Type.(*ast.StructType) + strct, ok := spec.Type.(*ast.StructType) + if !ok { + log.Fatalf("Expected a struct, got a %T\n", spec) + } for _, field := range strct.Fields.List { tag, err := strconv.Unquote(field.Tag.Value) if err != nil { diff --git a/internal/toolexec/proxy/link.flags.go b/internal/toolexec/proxy/link.flags.go index 3dbd7e8b..56011c51 100644 --- a/internal/toolexec/proxy/link.flags.go +++ b/internal/toolexec/proxy/link.flags.go @@ -19,7 +19,7 @@ func (f *linkFlagSet) parse(args []string) ([]string, error) { flagSet.String("R", "-1", "set address rounding quantum") flagSet.Int("T", -1, "set the start address of text symbols") flagSet.BoolFunc("V", "print version and exit", func(string) error { - f.showVersion = true + f.ShowVersion = true return nil }) flagSet.String("X", "", "add string value definition of the form importpath.name=value") diff --git a/internal/toolexec/proxy/link.go b/internal/toolexec/proxy/link.go index 9b81bdf7..82e02390 100644 --- a/internal/toolexec/proxy/link.go +++ b/internal/toolexec/proxy/link.go @@ -16,7 +16,7 @@ type linkFlagSet struct { BuildMode string `ddflag:"-buildmode"` ImportCfg string `ddflag:"-importcfg"` Output string `ddflag:"-o"` - showVersion bool `ddflag:"-V"` + ShowVersion bool `ddflag:"-V"` } // LinkCommand represents a go tool `link` invocation @@ -32,7 +32,7 @@ func (*LinkCommand) Type() CommandType { } func (cmd *LinkCommand) ShowVersion() bool { - return cmd.Flags.showVersion + return cmd.Flags.ShowVersion } func (cmd *LinkCommand) Stage() string { diff --git a/internal/toolexec/proxy/link_test.go b/internal/toolexec/proxy/link_test.go index 7d84608a..cea2d89c 100644 --- a/internal/toolexec/proxy/link_test.go +++ b/internal/toolexec/proxy/link_test.go @@ -20,7 +20,7 @@ func TestParseLink(t *testing.T) { "version_print": { input: []string{"/path/link", "-V=full"}, flags: linkFlagSet{ - showVersion: true, + ShowVersion: true, }, }, "link": { From 31259706a33fe361e356e36d0f41c8eb2afbfda8 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 17:17:25 +0200 Subject: [PATCH 5/9] Fixup generator --- internal/toolexec/proxy/generator/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go index fdb53076..75f5bfdb 100644 --- a/internal/toolexec/proxy/generator/main.go +++ b/internal/toolexec/proxy/generator/main.go @@ -143,7 +143,7 @@ func main() { file.HeaderComment("// Unless explicitly stated otherwise all files in this repository are licensed") file.HeaderComment("// under the Apache License Version 2.0.") file.HeaderComment("// This product includes software developed at Datadog (https://www.datadoghq.com/).") - file.HeaderComment("// Copyright 2023-present Datadog, Inc.)\n") + file.HeaderComment("// Copyright 2023-present Datadog, Inc.\n") file.HeaderComment("// Code generated by 'go generate' DO NOT EDIT.") file.Func(). From 9ee05170e85a0419d7dca85d37374f8766d344d1 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 18 Sep 2024 17:19:58 +0200 Subject: [PATCH 6/9] chore: update generated files --- internal/toolexec/proxy/compile.flags.go | 2 +- internal/toolexec/proxy/link.flags.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go index 39a044e6..6991b48e 100644 --- a/internal/toolexec/proxy/compile.flags.go +++ b/internal/toolexec/proxy/compile.flags.go @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed // under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc.) +// Copyright 2023-present Datadog, Inc. // Code generated by 'go generate' DO NOT EDIT. diff --git a/internal/toolexec/proxy/link.flags.go b/internal/toolexec/proxy/link.flags.go index 56011c51..e06e894f 100644 --- a/internal/toolexec/proxy/link.flags.go +++ b/internal/toolexec/proxy/link.flags.go @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed // under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc.) +// Copyright 2023-present Datadog, Inc. // Code generated by 'go generate' DO NOT EDIT. From 24f9a3813310e4d1719489e61a5e4d35fe1e7746 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 19 Sep 2024 17:09:54 +0200 Subject: [PATCH 7/9] Implemented feedback --- internal/toolexec/proxy/compile.flags.go | 14 ++--- internal/toolexec/proxy/generator/main.go | 76 +++++++++++++++++++---- internal/toolexec/proxy/link.flags.go | 14 ++--- internal/toolexec/proxy/link.go | 3 +- 4 files changed, 78 insertions(+), 29 deletions(-) diff --git a/internal/toolexec/proxy/compile.flags.go b/internal/toolexec/proxy/compile.flags.go index 6991b48e..fdf8df7b 100644 --- a/internal/toolexec/proxy/compile.flags.go +++ b/internal/toolexec/proxy/compile.flags.go @@ -10,7 +10,7 @@ package proxy import "flag" func (f *compileFlagSet) parse(args []string) ([]string, error) { - flagSet := flag.NewFlagSet("compile", flag.ContinueOnError) + flagSet := flag.NewFlagSet("compile version go1.23", flag.ContinueOnError) flagSet.Bool("%", false, "debug non-static initializers") flagSet.Bool("+", false, "compiling runtime") flagSet.Bool("B", false, "disable bounds checking") @@ -32,22 +32,22 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.String("bench", "", "append benchmark times to file") flagSet.String("blockprofile", "", "write block profile to file") flagSet.String("buildid", "", "record id as the build id in the export metadata") - flagSet.Int("c", 4, "concurrency during compilation (1 means no concurrency)") + flagSet.Int("c", 0, "concurrency during compilation (1 means no concurrency)") flagSet.Bool("clobberdead", false, "clobber dead stack slots (for debugging)") flagSet.Bool("clobberdeadreg", false, "clobber dead registers (for debugging)") flagSet.Bool("complete", false, "compiling complete package (no C or assembly)") flagSet.String("coveragecfg", "", "read coverage configuration from file") flagSet.String("cpuprofile", "", "write cpu profile to file") flagSet.String("d", "", "enable debugging settings; try -d help") - flagSet.Bool("dwarf", true, "generate DWARF symbols") - flagSet.Bool("dwarfbasentries", true, "use base address selection entries in DWARF") - flagSet.Bool("dwarflocationlists", true, "add location lists to DWARF in optimized mode") + flagSet.Bool("dwarf", false, "generate DWARF symbols") + flagSet.Bool("dwarfbasentries", false, "use base address selection entries in DWARF") + flagSet.Bool("dwarflocationlists", false, "add location lists to DWARF in optimized mode") flagSet.Bool("dynlink", false, "support references to Go symbols defined in other shared libraries") flagSet.Bool("e", false, "no limit on number of errors reported") flagSet.String("embedcfg", "", "read go:embed configuration from file") flagSet.String("env", "", "add definition of the form key=value to environment") flagSet.Bool("errorurl", false, "print explanatory URL with error message if applicable") - flagSet.Int("gendwarfinl", 2, "generate DWARF inline info records") + flagSet.Int("gendwarfinl", 0, "generate DWARF inline info records") flagSet.String("goversion", "", "required version of the runtime") flagSet.Bool("h", false, "halt on error") flagSet.StringVar(&f.ImportCfg, "importcfg", "", "read import configuration from file") @@ -81,7 +81,7 @@ func (f *compileFlagSet) parse(args []string) ([]string, error) { flagSet.String("trimpath", "", "remove prefix from recorded source file paths") flagSet.Bool("v", false, "increase debug verbosity") flagSet.Bool("w", false, "debug type checking") - flagSet.Bool("wb", true, "enable write barrier") + flagSet.Bool("wb", false, "enable write barrier") err := flagSet.Parse(args) return flagSet.Args(), err diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go index 75f5bfdb..c03c790a 100644 --- a/internal/toolexec/proxy/generator/main.go +++ b/internal/toolexec/proxy/generator/main.go @@ -8,6 +8,7 @@ package main import ( "bufio" "bytes" + "errors" "flag" "fmt" "go/ast" @@ -40,7 +41,49 @@ func main() { log.Fatalln("Missing value for required -command flag") } - cmd := exec.Command("go", "tool", command) + var ( + goFile = requireEnv("GOFILE") + goPackage = requireEnv("GOPACKAGE") + ) + + cmd := exec.Command("go", "tool", command, "-V=full") + cmd.Env = append(cmd.Env, "LANG=C") + var fullVersion bytes.Buffer + cmd.Stdout = &fullVersion + if err := cmd.Run(); err != nil { + log.Fatalln(err) + } + + version := strings.TrimSpace(fullVersion.String()) + version = version[:strings.LastIndexByte(version, '.')] + + outFile := filepath.Join(goFile, "..", fmt.Sprintf("%s.flags.go", command)) + if content, err := os.ReadFile(outFile); err == nil { + newMajorS, newMinorS, _ := strings.Cut(strings.Fields(version)[2][2:], ".") + newMajor, _ := strconv.Atoi(newMajorS) + newMinor, _ := strconv.Atoi(newMinorS) + + // versionTagRe matches the " version goX.Y" tag that is expected to be present in generated files. + // Example: https://regex101.com/r/MiYpBy/1 + versionTagRe := regexp.MustCompile(fmt.Sprintf(`(?m)"%s version go(\d+)\.(\d+)"`, command)) + if matches := versionTagRe.FindSubmatch(content); len(matches) > 0 { + curMajor, _ := strconv.Atoi(string(matches[1])) + curMinor, _ := strconv.Atoi(string(matches[2])) + + if curMajor > newMajor || (curMajor == newMajor && curMinor > newMinor) { + if os.Getenv("CI") != "" { + log.Fatalf("Generate must be run with go%d.%d or newer (was run with %d.%d)\n", curMajor, curMinor, newMajor, newMinor) + } else { + log.Printf("Skipping generation of %q, as it was generated against a more recent version of the go %s tool (%d.%d >= %d.%d)\n", outFile, command, curMajor, curMinor, newMajor, newMinor) + return + } + } + } + } else if !errors.Is(err, os.ErrNotExist) { + log.Fatalln(err) + } + + cmd = exec.Command("go", "tool", command) cmd.Env = append(cmd.Env, "LANG=C") var buffer bytes.Buffer cmd.Stderr = &buffer @@ -48,17 +91,21 @@ func main() { reader := bufio.NewReader(&buffer) // Ensure the first line looks like usage instructions... - if line, _, err := reader.ReadLine(); err != nil { + line, _, err := reader.ReadLine() + if err != nil { log.Fatalln(err) - } else if !bytes.HasPrefix(line, []byte("usage: ")) { + } + if !bytes.HasPrefix(line, []byte("usage: ")) { log.Fatalf("Unexpected output from command:\n%s\n", string(line)) } var ( flags []flagSpec knownFlags = make(map[string]struct{}) + // reFlag captures the flag and it's argument name (if present), as well as the documentation string that may + // follow it. Example: https://regex101.com/r/vqsECV/1 + reFlag = regexp.MustCompile(`^ (-[^\s]+)(?: ([^\s]+))?(\t.+)?$`) ) - reFlag := regexp.MustCompile(`^ (-[^\s]+)(?: ([^\s]+))?(\t.+)?$`) for { line, _, err := reader.ReadLine() if err == io.EOF { @@ -91,11 +138,6 @@ func main() { log.Fatalln("No flags found in command usage, this is unexpected!") } - var ( - goFile = requireEnv("GOFILE") - goPackage = requireEnv("GOPACKAGE") - ) - fset := token.NewFileSet() src, err := os.ReadFile(goFile) if err != nil { @@ -138,13 +180,16 @@ func main() { } } } + if len(capturedFlags) == 0 { + log.Fatalf("Expected fields annotated with the `ddflag:\"-flag\"` tag in the %s struct\n", typeName) + } file := jen.NewFile(goPackage) file.HeaderComment("// Unless explicitly stated otherwise all files in this repository are licensed") file.HeaderComment("// under the Apache License Version 2.0.") file.HeaderComment("// This product includes software developed at Datadog (https://www.datadoghq.com/).") file.HeaderComment("// Copyright 2023-present Datadog, Inc.\n") - file.HeaderComment("// Code generated by 'go generate' DO NOT EDIT.") + file.HeaderComment("// Code generated by 'go generate' DO NOT EDIT.\n") file.Func(). Params(jen.Id("f").Op("*").Id(typeName)). @@ -159,10 +204,11 @@ func main() { Op(":="). Qual("flag", "NewFlagSet"). Call( - jen.Lit(command), + jen.Lit(version), jen.Qual("flag", "ContinueOnError"), ) + // reDefault captures default values found in flag descriptions. See: https://regex101.com/r/jDEwWQ/1 reDefault := regexp.MustCompile(`^(.+?)(?:\s+\(default\s+(.+?)\))?$`) for _, spec := range flags { fieldName, captured := capturedFlags[spec.Flag] @@ -171,6 +217,7 @@ func main() { funcName string strDefault string defaultValue *jen.Statement + zeroValue *jen.Statement ) descr := strings.Join(spec.Descr, " ") @@ -182,6 +229,7 @@ func main() { switch spec.Value { case "": funcName = "Bool" + zeroValue = jen.False() if strDefault == "" || strDefault == "false" { defaultValue = jen.False() } else { @@ -189,6 +237,7 @@ func main() { } case "int": funcName = "Int" + zeroValue = jen.Lit(0) if strDefault != "" { val, err := strconv.Atoi(strDefault) if err != nil { @@ -200,6 +249,7 @@ func main() { } default: funcName = "String" + zeroValue = jen.Lit("") defaultValue = jen.Lit(strDefault) } @@ -219,7 +269,7 @@ func main() { } else if captured { g.Id("flagSet").Dot(funcName+"Var").Call(jen.Op("&").Id("f").Dot(fieldName), flagName, defaultValue, jen.Lit(descr)) } else { - g.Id("flagSet").Dot(funcName).Call(flagName, defaultValue, jen.Lit(descr)) + g.Id("flagSet").Dot(funcName).Call(flagName, zeroValue, jen.Lit(descr)) } } g.Line() @@ -231,7 +281,7 @@ func main() { ) }) - out, err := os.Create(filepath.Join(goFile, "..", fmt.Sprintf("%s.flags.go", command))) + out, err := os.Create(outFile) if err != nil { log.Fatalln(err) } diff --git a/internal/toolexec/proxy/link.flags.go b/internal/toolexec/proxy/link.flags.go index e06e894f..8ae2a2fa 100644 --- a/internal/toolexec/proxy/link.flags.go +++ b/internal/toolexec/proxy/link.flags.go @@ -10,14 +10,14 @@ package proxy import "flag" func (f *linkFlagSet) parse(args []string) ([]string, error) { - flagSet := flag.NewFlagSet("link", flag.ContinueOnError) + flagSet := flag.NewFlagSet("link version go1.23", flag.ContinueOnError) flagSet.String("B", "", "add an ELF NT_GNU_BUILD_ID note when using ELF; use \"gobuildid\" to generate it from the Go build ID") flagSet.String("E", "", "set entry symbol name") flagSet.String("H", "", "set header type") flagSet.String("I", "", "use linker as ELF dynamic linker") flagSet.String("L", "", "add specified directory to library path") - flagSet.String("R", "-1", "set address rounding quantum") - flagSet.Int("T", -1, "set the start address of text symbols") + flagSet.String("R", "", "set address rounding quantum") + flagSet.Int("T", 0, "set the start address of text symbols") flagSet.BoolFunc("V", "print version and exit", func(string) error { f.ShowVersion = true return nil @@ -25,7 +25,7 @@ func (f *linkFlagSet) parse(args []string) ([]string, error) { flagSet.String("X", "", "add string value definition of the form importpath.name=value") flagSet.Bool("a", false, "no-op (deprecated)") flagSet.Bool("asan", false, "enable ASan interface") - flagSet.Bool("aslr", true, "enable ASLR for buildmode=c-shared on windows") + flagSet.Bool("aslr", false, "enable ASLR for buildmode=c-shared on windows") flagSet.String("benchmark", "", "set to 'mem' or 'cpu' to enable phase benchmarking") flagSet.String("benchmarkprofile", "", "emit phase profiles to base_phase.{cpu,mem}prof") flagSet.Bool("bindnow", false, "mark a dynamically linked ELF object for immediate function binding") @@ -33,8 +33,8 @@ func (f *linkFlagSet) parse(args []string) ([]string, error) { flagSet.StringVar(&f.BuildMode, "buildmode", "", "set build mode") flagSet.Bool("c", false, "dump call graph") flagSet.String("capturehostobjs", "", "capture host object files loaded during internal linking to specified dir") - flagSet.Bool("checklinkname", true, "check linkname symbol references") - flagSet.Bool("compressdwarf", true, "compress DWARF if possible") + flagSet.Bool("checklinkname", false, "check linkname symbol references") + flagSet.Bool("compressdwarf", false, "compress DWARF if possible") flagSet.String("cpuprofile", "", "write cpu profile to file") flagSet.Bool("d", false, "disable dynamic executable") flagSet.Bool("debugnosplit", false, "dump nosplit call graph") @@ -59,7 +59,7 @@ func (f *linkFlagSet) parse(args []string) ([]string, error) { flagSet.Bool("n", false, "no-op (deprecated)") flagSet.StringVar(&f.Output, "o", "", "write output to file") flagSet.String("pluginpath", "", "full path name for plugin") - flagSet.Bool("pruneweakmap", true, "prune weak mapinit refs") + flagSet.Bool("pruneweakmap", false, "prune weak mapinit refs") flagSet.String("r", "", "set the ELF dynamic linker search path to dir1:dir2:...") flagSet.Bool("race", false, "enable race detector") flagSet.Int("randlayout", 0, "randomize function layout") diff --git a/internal/toolexec/proxy/link.go b/internal/toolexec/proxy/link.go index 82e02390..1d270da4 100644 --- a/internal/toolexec/proxy/link.go +++ b/internal/toolexec/proxy/link.go @@ -44,8 +44,7 @@ func parseLinkCommand(args []string) (Command, error) { return nil, errors.New("unexpected number of command arguments") } flags := &linkFlagSet{} - _, err := flags.parse(args[1:]) - if err != nil { + if _, err := flags.parse(args[1:]); err != nil { return nil, err } From 642083f04909e8d3dac193c6851be8e6b5208ce7 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 19 Sep 2024 17:13:26 +0200 Subject: [PATCH 8/9] Sanitize command --- internal/toolexec/proxy/generator/main.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go index c03c790a..34eb80aa 100644 --- a/internal/toolexec/proxy/generator/main.go +++ b/internal/toolexec/proxy/generator/main.go @@ -34,7 +34,15 @@ type flagSpec struct { func main() { var command string - flag.StringVar(&command, "command", "", "The command to generate a parser for") + flag.Func("command", "The command to generate a parser for", func(val string) error { + switch val { + case "compile", "link": + command = val + default: + return fmt.Errorf("unsupported command: %q", val) + } + return nil + }) flag.Parse() if command == "" { From c1e743ecb8ed1335ff767b18bf005bbfcbaefd7c Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 19 Sep 2024 17:26:11 +0200 Subject: [PATCH 9/9] Make linter happy --- internal/toolexec/proxy/generator/main.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/toolexec/proxy/generator/main.go b/internal/toolexec/proxy/generator/main.go index 34eb80aa..1db20d9c 100644 --- a/internal/toolexec/proxy/generator/main.go +++ b/internal/toolexec/proxy/generator/main.go @@ -81,10 +81,9 @@ func main() { if curMajor > newMajor || (curMajor == newMajor && curMinor > newMinor) { if os.Getenv("CI") != "" { log.Fatalf("Generate must be run with go%d.%d or newer (was run with %d.%d)\n", curMajor, curMinor, newMajor, newMinor) - } else { - log.Printf("Skipping generation of %q, as it was generated against a more recent version of the go %s tool (%d.%d >= %d.%d)\n", outFile, command, curMajor, curMinor, newMajor, newMinor) - return } + log.Printf("Skipping generation of %q, as it was generated against a more recent version of the go %s tool (%d.%d >= %d.%d)\n", outFile, command, curMajor, curMinor, newMajor, newMinor) + return } } } else if !errors.Is(err, os.ErrNotExist) {