From f50ffba9cf37a247e9e87b42a4391b72e3348e36 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Wed, 8 May 2024 11:12:19 -0700 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Pablo Baeyens --- cmd/builder/README.md | 12 +++++------- cmd/builder/internal/builder/config.go | 2 +- cmd/builder/internal/builder/main.go | 10 +++++++--- cmd/builder/internal/command.go | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cmd/builder/README.md b/cmd/builder/README.md index 826505933f7..18397df575b 100644 --- a/cmd/builder/README.md +++ b/cmd/builder/README.md @@ -164,11 +164,10 @@ to only execute the compilation step. ### Avoiding the use of a new go.mod file -There is an additional option that controls one aspect of the build -process, which specifically allows skipping the use of a new `go.mod` -file. When the `--skip-new-go-module` command-line flag is supplied, -the build process issues a `go get` command for each component, -relying on the Go toolchain to update the enclosing Go module +You can optionally skip creating a new `go.mod` file. This is helpful when +using a monorepo setup with a shared go.mod file. When the `--skip-new-go-module` +command-line flag is supplied, the build process issues a `go get` command for +each component, relying on the Go toolchain to update the enclosing Go module appropriately. This command will avoid downgrading a dependency in the enclosing @@ -177,8 +176,7 @@ module, according to and will instead issue a log indicating that the component was not upgraded. -This mode cannot be used in conjunction with several features that -control the generated `go.mod` file, including `replaces`, `excludes`, +`--skip-new-go-module` is incompatible with `replaces`, `excludes`, and the component `path` override. For each of these features, users are expected to modify the enclosing `go.mod` directly. diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index e98ca1dde5f..2fd933a1ad8 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -254,7 +254,7 @@ func (c *Config) validateModules(name string, mods []Module) error { return fmt.Errorf("%s module at index %v: %w", name, i, ErrMissingGoMod) } if mod.Path != "" && c.SkipNewGoModule { - return fmt.Errorf("%w cannot modify mod.path \"%v\" combined with --skip-new-go-module; please modify the enclosing go.mod file directly", ErrIncompatibleConfigurationValues, mod.Path) + return fmt.Errorf("%w cannot modify mod.path %q combined with --skip-new-go-module; please modify the enclosing go.mod file directly", ErrIncompatibleConfigurationValues, mod.Path) } } return nil diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index 6368010e700..3af9fdd8775 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -264,6 +264,11 @@ func (c *Config) updateModules() error { } func (c *Config) updateGoModule(modspec string) error { + mod, ver, found := strings.Cut(modspec, " ") + if !found { + return fmt.Errorf("ill-formatted modspec %q: missing space separator", modspec) + } + // Re-parse the go.mod file on each iteration, since it can // change each time. modulePath, dependencyVersions, err := c.readGoModFile() @@ -271,7 +276,6 @@ func (c *Config) updateGoModule(modspec string) error { return err } - mod, ver, _ := strings.Cut(modspec, " ") if mod == modulePath { // this component is part of the same module, nothing to update. return nil @@ -292,9 +296,9 @@ func (c *Config) updateGoModule(modspec string) error { } // upgrading or changing version - updatespec := mod + "@" + ver + updatespec := "-require=" + mod + "@" + ver - if _, err := runGoCommand(*c, "get", updatespec); err != nil { + if _, err := runGoCommand(*c, "mod", "edit", updatespec); err != nil { return err } return nil diff --git a/cmd/builder/internal/command.go b/cmd/builder/internal/command.go index c7581e84d28..abe9d3b6fc3 100644 --- a/cmd/builder/internal/command.go +++ b/cmd/builder/internal/command.go @@ -85,7 +85,7 @@ configuration is provided, ocb will generate a default Collector. cmd.Flags().BoolVar(&cfg.SkipGenerate, skipGenerateFlag, false, "Whether builder should skip generating go code (default false)") cmd.Flags().BoolVar(&cfg.SkipCompilation, skipCompilationFlag, false, "Whether builder should only generate go code with no compile of the collector (default false)") cmd.Flags().BoolVar(&cfg.SkipGetModules, skipGetModulesFlag, false, "Whether builder should skip updating go.mod and retrieve Go module list (default false)") - cmd.Flags().BoolVar(&cfg.SkipNewGoModule, skipNewGoModuleFlag, false, "Whether builder should skip generating a new go.mod file, use enclosing Go module instead (default false)") + cmd.Flags().BoolVar(&cfg.SkipNewGoModule, skipNewGoModuleFlag, false, "Whether builder should skip generating a new go.mod file, using the enclosing Go module instead (default false)") cmd.Flags().BoolVar(&cfg.SkipStrictVersioning, skipStrictVersioningFlag, false, "Whether builder should skip strictly checking the calculated versions following dependency resolution") cmd.Flags().BoolVar(&cfg.Verbose, verboseFlag, false, "Whether builder should print verbose output (default false)") cmd.Flags().StringVar(&cfg.LDFlags, ldflagsFlag, "", `ldflags to include in the "go build" command`)