Apply suggestions from code review

Co-authored-by: Pablo Baeyens <[email protected]>

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.
 

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

cmd/builder/internal/builder/main.go

@@ -264,14 +264,18 @@ 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()
 	if err != nil {
 		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

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`)