Revert "[builder] Support for --skip-new-go-module (open-telemetry#10098)" (open-telemetry#10978)

This reverts commit a0331ed.

Author: codeboten

.chloggen/builder-skip-go-mod.yaml

@@ -1,7 +1,5 @@
 include ../../Makefile.Common
 
-GOTEST_TIMEOUT=360s
-
 .PHONY: ocb
 ocb:
 	CGO_ENABLED=0 $(GOCMD) build -trimpath -o ../../bin/ocb_$(GOOS)_$(GOARCH) .

cmd/builder/Makefile

@@ -157,24 +157,6 @@ ocb --skip-generate --skip-get-modules --config=config.yaml
 ```
 to only execute the compilation step.
 
-### Avoiding the use of a new go.mod file
-
-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
-module, according to
-[`semver.Compare()`](https://pkg.go.dev/golang.org/x/mod/semver#Compare),
-and will instead issue a log indicating that the component was not
-upgraded.
-
-`--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.
-
 ### Strict versioning checks
 
 The builder checks the relevant `go.mod`

cmd/builder/README.md

@@ -31,7 +31,7 @@ require (
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/rogpeppe/go-internal v1.12.0 // indirect
+	github.com/rogpeppe/go-internal v1.10.0 // indirect
 	golang.org/x/sys v0.21.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

cmd/builder/go.mod

@@ -19,12 +19,8 @@ import (
 
 const defaultOtelColVersion = "0.107.0"
 
-var (
-	// ErrMissingGoMod indicates an empty gomod field
-	ErrMissingGoMod = errors.New("missing gomod specification for module")
-	// ErrIncompatibleConfigurationValues indicates that there is configuration that cannot be combined
-	ErrIncompatibleConfigurationValues = errors.New("cannot combine configuration values")
-)
+// ErrMissingGoMod indicates an empty gomod field
+var ErrMissingGoMod = errors.New("missing gomod specification for module")
 
 // Config holds the builder's configuration
 type Config struct {
@@ -33,7 +29,6 @@ type Config struct {
 	SkipGenerate         bool   `mapstructure:"-"`
 	SkipCompilation      bool   `mapstructure:"-"`
 	SkipGetModules       bool   `mapstructure:"-"`
-	SkipNewGoModule      bool   `mapstructure:"-"`
 	SkipStrictVersioning bool   `mapstructure:"-"`
 	LDFlags              string `mapstructure:"-"`
 	Verbose              bool   `mapstructure:"-"`
@@ -121,15 +116,14 @@ func NewDefaultConfig() Config {
 func (c *Config) Validate() error {
 	var providersError error
 	if c.Providers != nil {
-		providersError = c.validateModules("provider", *c.Providers)
+		providersError = validateModules("provider", *c.Providers)
 	}
 	return multierr.Combine(
-		c.validateModules("extension", c.Extensions),
-		c.validateModules("receiver", c.Receivers),
-		c.validateModules("exporter", c.Exporters),
-		c.validateModules("processor", c.Processors),
-		c.validateModules("connector", c.Connectors),
-		c.validateFlags(),
+		validateModules("extension", c.Extensions),
+		validateModules("receiver", c.Receivers),
+		validateModules("exporter", c.Exporters),
+		validateModules("processor", c.Processors),
+		validateModules("connector", c.Connectors),
 		providersError,
 	)
 }
@@ -246,21 +240,11 @@ func (c *Config) ParseModules() error {
 	return nil
 }
 
-func (c *Config) validateFlags() error {
-	if c.SkipNewGoModule && (len(c.Replaces) != 0 || len(c.Excludes) != 0) {
-		return fmt.Errorf("%w excludes or replaces with --skip-new-go-module; please modify the enclosing go.mod file directly", ErrIncompatibleConfigurationValues)
-	}
-	return nil
-}
-
-func (c *Config) validateModules(name string, mods []Module) error {
+func validateModules(name string, mods []Module) error {
 	for i, mod := range mods {
 		if mod.GoMod == "" {
 			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 %q combined with --skip-new-go-module; please modify the enclosing go.mod file directly", ErrIncompatibleConfigurationValues, mod.Path)
-		}
 	}
 	return nil
 }

cmd/builder/go.sum

@@ -4,6 +4,7 @@
 package builder
 
 import (
+	"errors"
 	"os"
 	"strings"
 	"testing"
@@ -139,37 +140,10 @@ func TestMissingModule(t *testing.T) {
 			},
 			err: ErrMissingGoMod,
 		},
-		{
-			cfg: Config{
-				Logger:          zap.NewNop(),
-				SkipNewGoModule: true,
-				Extensions: []Module{{
-					GoMod: "some-module",
-					Path:  "invalid",
-				}},
-			},
-			err: ErrIncompatibleConfigurationValues,
-		},
-		{
-			cfg: Config{
-				Logger:          zap.NewNop(),
-				SkipNewGoModule: true,
-				Replaces:        []string{"", ""},
-			},
-			err: ErrIncompatibleConfigurationValues,
-		},
-		{
-			cfg: Config{
-				Logger:          zap.NewNop(),
-				SkipNewGoModule: true,
-				Excludes:        []string{"", ""},
-			},
-			err: ErrIncompatibleConfigurationValues,
-		},
 	}
 
 	for _, test := range configurations {
-		assert.ErrorIs(t, test.cfg.Validate(), test.err)
+		assert.True(t, errors.Is(test.cfg.Validate(), test.err))
 	}
 }
 

cmd/builder/internal/builder/config.go

@@ -13,6 +13,7 @@ import (
 	"slices"
 	"strings"
 	"text/template"
+	"time"
 
 	"go.uber.org/zap"
 	"golang.org/x/mod/modfile"
@@ -24,6 +25,7 @@ var (
 	ErrGoNotFound      = errors.New("go binary not found")
 	ErrDepNotFound     = errors.New("dependency not found in go mod file")
 	ErrVersionMismatch = errors.New("mismatch in go.mod and builder configuration versions")
+	errDownloadFailed  = errors.New("failed to download go modules")
 	errCompileFailed   = errors.New("failed to compile the OpenTelemetry Collector distribution")
 	skipStrictMsg      = "Use --skip-strict-versioning to temporarily disable this check. This flag will be removed in a future minor version"
 )
@@ -84,30 +86,18 @@ func Generate(cfg Config) error {
 		return fmt.Errorf("failed to create output path: %w", err)
 	}
 
-	allTemplates := []*template.Template{
+	for _, tmpl := range []*template.Template{
 		mainTemplate,
 		mainOthersTemplate,
 		mainWindowsTemplate,
 		componentsTemplate,
-	}
-
-	// Add the go.mod template unless that file is skipped.
-	if !cfg.SkipNewGoModule {
-		allTemplates = append(allTemplates, goModTemplate)
-	}
-
-	for _, tmpl := range allTemplates {
+		goModTemplate,
+	} {
 		if err := processAndWrite(cfg, tmpl, tmpl.Name(), cfg); err != nil {
 			return fmt.Errorf("failed to generate source file %q: %w", tmpl.Name(), err)
 		}
 	}
 
-	// when not creating a new go.mod file, update modules one-by-one in the
-	// enclosing go module.
-	if err := cfg.updateModules(); err != nil {
-		return err
-	}
-
 	cfg.Logger.Info("Sources created", zap.String("path", cfg.Distribution.OutputPath))
 	return nil
 }
@@ -155,7 +145,7 @@ func GetModules(cfg Config) error {
 	}
 
 	if cfg.SkipStrictVersioning {
-		return nil
+		return downloadModules(cfg)
 	}
 
 	// Perform strict version checking.  For each component listed and the
@@ -195,7 +185,22 @@ func GetModules(cfg Config) error {
 		}
 	}
 
-	return nil
+	return downloadModules(cfg)
+}
+
+func downloadModules(cfg Config) error {
+	cfg.Logger.Info("Getting go modules")
+	failReason := "unknown"
+	for i := 1; i <= cfg.downloadModules.numRetries; i++ {
+		if _, err := runGoCommand(cfg, "mod", "download"); err != nil {
+			failReason = err.Error()
+			cfg.Logger.Info("Failed modules download", zap.String("retry", fmt.Sprintf("%d/%d", i, cfg.downloadModules.numRetries)))
+			time.Sleep(cfg.downloadModules.wait)
+			continue
+		}
+		return nil
+	}
+	return fmt.Errorf("%w: %s", errDownloadFailed, failReason)
 }
 
 func processAndWrite(cfg Config, tmpl *template.Template, outFile string, tmplParams any) error {
@@ -221,68 +226,6 @@ func (c *Config) allComponents() []Module {
 		c.Extensions, c.Connectors, *c.Providers)
 }
 
-func (c *Config) updateModules() error {
-	if !c.SkipNewGoModule {
-		return nil
-	}
-
-	// Build the main service dependency
-	coremod, corever := c.coreModuleAndVersion()
-	corespec := coremod + " " + corever
-
-	if err := c.updateGoModule(corespec); err != nil {
-		return err
-	}
-
-	for _, comp := range c.allComponents() {
-		if err := c.updateGoModule(comp.GoMod); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-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
-	}
-
-	if mod == modulePath {
-		// this component is part of the same module, nothing to update.
-		return nil
-	}
-
-	// check for exact match
-	hasVer, ok := dependencyVersions[mod]
-	if ok && hasVer == ver {
-		c.Logger.Info("Component version match", zap.String("module", mod), zap.String("version", ver))
-		return nil
-	}
-
-	scomp := semver.Compare(hasVer, ver)
-	if scomp > 0 {
-		// version in enclosing module is newer, do not change
-		c.Logger.Info("Not upgrading component, enclosing module is newer.", zap.String("module", mod), zap.String("existing", hasVer), zap.String("config_version", ver))
-		return nil
-	}
-
-	// upgrading or changing version
-	updatespec := "-require=" + mod + "@" + ver
-
-	if _, err := runGoCommand(*c, "mod", "edit", updatespec); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (c *Config) readGoModFile() (string, map[string]string, error) {
 	var modPath string
 	stdout, err := runGoCommand(*c, "mod", "edit", "-print")