From af667ca1894db9558b95f64f60542592dd1fde65 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Mon, 6 May 2024 20:40:15 -0700 Subject: [PATCH] [builder] Support for --skip-new-go-module --- .chloggen/builder-skip-go-mod.yaml | 25 +++++++ cmd/builder/README.md | 20 ++++++ cmd/builder/internal/builder/config.go | 34 ++++++--- cmd/builder/internal/builder/config_test.go | 30 +++++++- cmd/builder/internal/builder/main.go | 76 ++++++++++++++++++++- cmd/builder/internal/command.go | 5 ++ 6 files changed, 176 insertions(+), 14 deletions(-) create mode 100644 .chloggen/builder-skip-go-mod.yaml diff --git a/.chloggen/builder-skip-go-mod.yaml b/.chloggen/builder-skip-go-mod.yaml new file mode 100644 index 00000000000..619e29e3489 --- /dev/null +++ b/.chloggen/builder-skip-go-mod.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: builder + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add a --skip-new-go-module flag to skip creating a module in the output directory. + +# One or more tracking issues or pull requests related to the change +issues: [9252] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/cmd/builder/README.md b/cmd/builder/README.md index ec8d18c3808..826505933f7 100644 --- a/cmd/builder/README.md +++ b/cmd/builder/README.md @@ -162,6 +162,26 @@ ocb --skip-generate --skip-get-modules --config=config.yaml ``` 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 +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. + +This mode cannot be used in conjunction with several features that +control the generated `go.mod` file, including `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` diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index e1c4aee1493..4b54471f810 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -19,8 +19,12 @@ import ( const defaultOtelColVersion = "0.106.1" -// ErrMissingGoMod indicates an empty gomod field -var ErrMissingGoMod = errors.New("missing gomod specification for module") +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") +) // Config holds the builder's configuration type Config struct { @@ -29,6 +33,7 @@ type Config struct { SkipGenerate bool `mapstructure:"-"` SkipCompilation bool `mapstructure:"-"` SkipGetModules bool `mapstructure:"-"` + SkipNewGoModule bool `mapstructure:"-"` SkipStrictVersioning bool `mapstructure:"-"` LDFlags string `mapstructure:"-"` Verbose bool `mapstructure:"-"` @@ -116,14 +121,15 @@ func NewDefaultConfig() Config { func (c *Config) Validate() error { var providersError error if c.Providers != nil { - providersError = validateModules("provider", *c.Providers) + providersError = c.validateModules("provider", *c.Providers) } return multierr.Combine( - validateModules("extension", c.Extensions), - validateModules("receiver", c.Receivers), - validateModules("exporter", c.Exporters), - validateModules("processor", c.Processors), - validateModules("connector", c.Connectors), + 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(), providersError, ) } @@ -240,11 +246,21 @@ func (c *Config) ParseModules() error { return nil } -func validateModules(name string, mods []Module) error { +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 { 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 \"%v\" 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/config_test.go b/cmd/builder/internal/builder/config_test.go index 9daf158cb8e..bb99cf849a4 100644 --- a/cmd/builder/internal/builder/config_test.go +++ b/cmd/builder/internal/builder/config_test.go @@ -4,7 +4,6 @@ package builder import ( - "errors" "os" "strings" "testing" @@ -140,10 +139,37 @@ 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.True(t, errors.Is(test.cfg.Validate(), test.err)) + assert.ErrorIs(t, test.cfg.Validate(), test.err) } } diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index 3d36059af36..6368010e700 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -85,18 +85,30 @@ func Generate(cfg Config) error { return fmt.Errorf("failed to create output path: %w", err) } - for _, tmpl := range []*template.Template{ + allTemplates := []*template.Template{ mainTemplate, mainOthersTemplate, mainWindowsTemplate, componentsTemplate, - goModTemplate, - } { + } + + // Add the go.mod template unless that file is skipped. + if !cfg.SkipNewGoModule { + allTemplates = append(allTemplates, goModTemplate) + } + + for _, tmpl := range allTemplates { 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 } @@ -230,6 +242,64 @@ func (c *Config) allComponents() []Module { *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 { + // 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 + } + + // 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 := mod + "@" + ver + + if _, err := runGoCommand(*c, "get", 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") diff --git a/cmd/builder/internal/command.go b/cmd/builder/internal/command.go index a738fb1c1f1..337254129e9 100644 --- a/cmd/builder/internal/command.go +++ b/cmd/builder/internal/command.go @@ -23,6 +23,7 @@ const ( skipGenerateFlag = "skip-generate" skipCompilationFlag = "skip-compilation" skipGetModulesFlag = "skip-get-modules" + skipNewGoModuleFlag = "skip-new-go-module" skipStrictVersioningFlag = "skip-strict-versioning" ldflagsFlag = "ldflags" distributionNameFlag = "name" @@ -84,6 +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.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`) @@ -185,6 +187,9 @@ func applyCfgFromFile(flags *flag.FlagSet, cfgFromFile builder.Config) { if !flags.Changed(skipGetModulesFlag) && cfgFromFile.SkipGetModules { cfg.SkipGetModules = cfgFromFile.SkipGetModules } + if !flags.Changed(skipNewGoModuleFlag) && cfgFromFile.SkipNewGoModule { + cfg.SkipNewGoModule = cfgFromFile.SkipNewGoModule + } if !flags.Changed(skipStrictVersioningFlag) && cfgFromFile.SkipStrictVersioning { cfg.SkipStrictVersioning = cfgFromFile.SkipStrictVersioning }