From 2177ebe64c4c55ab30b5e145cd8740f829b218f8 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Thu, 23 Jul 2026 17:50:59 +0000 Subject: [PATCH] feat: allow component definitions to merge across config files Remove the restriction that required each component name to be unique across all config files. Components now support field-level merge like distros: if the same component name appears in multiple files, later files' non-empty fields override earlier ones. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/user/explanation/config-system.md | 12 +- internal/projectconfig/loader.go | 27 +++- internal/projectconfig/loader_test.go | 206 ++++++++++++++++++++++++- 3 files changed, 226 insertions(+), 19 deletions(-) diff --git a/docs/user/explanation/config-system.md b/docs/user/explanation/config-system.md index 57a6e57c..721e9063 100644 --- a/docs/user/explanation/config-system.md +++ b/docs/user/explanation/config-system.md @@ -73,16 +73,22 @@ When multiple files define the same top-level section, azldev applies section-sp | Section | Merge behavior | Duplicates across files | |---------|---------------|------------------------| -| `components` | Additive (union of all component definitions) | **Error** — each component name must be unique across all files | +| `components` | Additive with field-level merge | **Allowed** — fields from later files override matching fields in earlier files | | `component-groups` | Additive (union of all group definitions) | **Error** — each group name must be unique across all files | | `images` | Additive (union of all image definitions) | **Error** — each image name must be unique across all files | | `distros` | Additive with field-level merge | **Allowed** — fields from later files override matching fields in earlier files | | `project` | Field-level override | N/A (single struct, not a map) | | `tools` | Field-level override | N/A (single struct, not a map) | -### Components, Component Groups, and Images +### Components -These are strict-union maps: each name may appear in exactly one config file across the entire include tree. If two files both define `[components.curl]`, azldev reports an error. This prevents accidental shadowing and makes it clear where each definition lives. +Component definitions are merged additively. If the same component name (e.g., `curl`) appears in multiple files, later files' non-empty fields override earlier ones. This allows splitting a component definition across files — for example, defining the component's general properties in one file and overriding specific details in another. + +> **Note:** Slice fields (like `overlays`) are **appended**, not replaced, following the same merge behavior used by component configuration inheritance. + +### Component Groups and Images + +These are strict-union maps: each name may appear in exactly one config file across the entire include tree. If two files both define `[component-groups.my-group]` or `[images.my-image]`, azldev reports an error. This prevents accidental shadowing and makes it clear where each definition lives. ### Distros diff --git a/internal/projectconfig/loader.go b/internal/projectconfig/loader.go index 31971f90..460c8314 100644 --- a/internal/projectconfig/loader.go +++ b/internal/projectconfig/loader.go @@ -19,8 +19,6 @@ import ( ) var ( - // ErrDuplicateComponents is returned when duplicate conflicting component definitions are found. - ErrDuplicateComponents = errors.New("duplicate component") // ErrDuplicateComponentGroups is returned when duplicate conflicting component group definitions are found. ErrDuplicateComponentGroups = errors.New("duplicate component group") // ErrDuplicateImages is returned when duplicate conflicting image definitions are found. @@ -214,18 +212,31 @@ func mergeComponentGroups(resolvedCfg *ProjectConfig, loadedCfg *ConfigFile) err } // mergeComponents merges component definitions from a loaded config file into -// the resolved config. Duplicate component names are not allowed. +// the resolved config. Components support additive merging: if a component +// already exists, its fields are updated from the new definition. func mergeComponents(resolvedCfg *ProjectConfig, loadedCfg *ConfigFile) error { for componentName, component := range loadedCfg.Components { - if _, ok := resolvedCfg.Components[componentName]; ok { - return fmt.Errorf("%w: %s", ErrDuplicateComponents, componentName) - } - // Fill out fields not explicitly serialized. component.Name = componentName component.SourceConfigFile = loadedCfg - resolvedCfg.Components[componentName] = *(component.WithAbsolutePaths(loadedCfg.dir)) + resolved := component.WithAbsolutePaths(loadedCfg.dir) + for i := range resolved.OverlayFiles { + resolved.OverlayFiles[i] = makeAbsolute(loadedCfg.dir, resolved.OverlayFiles[i]) + } + + if existing, ok := resolvedCfg.Components[componentName]; ok { + err := existing.MergeUpdatesFrom(resolved) + if err != nil { + return fmt.Errorf("failed to merge component %#q:\n%w", componentName, err) + } + + // Preserve the latest source config file reference. + existing.SourceConfigFile = loadedCfg + resolvedCfg.Components[componentName] = existing + } else { + resolvedCfg.Components[componentName] = *resolved + } } return nil diff --git a/internal/projectconfig/loader_test.go b/internal/projectconfig/loader_test.go index 67d781ce..c7bd136d 100644 --- a/internal/projectconfig/loader_test.go +++ b/internal/projectconfig/loader_test.go @@ -311,7 +311,7 @@ log-dir = "artifacts/logs" assert.Equal(t, "/project/subdir/artifacts/logs", config.Project.LogDir) } -func TestLoadAndResolveProjectConfig_DuplicateComponents(t *testing.T) { +func TestLoadAndResolveProjectConfig_MergeComponents(t *testing.T) { testFiles := []struct { path string contents string @@ -320,9 +320,15 @@ func TestLoadAndResolveProjectConfig_DuplicateComponents(t *testing.T) { includes = ["include.toml"] [components.abc] +[components.abc.spec] +type = "upstream" +upstream-commit = "aaa1111" `}, {"/project/include.toml", ` [components.abc] +[components.abc.spec] +type = "upstream" +upstream-commit = "bbb2222" `}, } @@ -334,8 +340,11 @@ includes = ["include.toml"] require.NoError(t, fileutils.WriteFile(ctx.FS(), testFile.path, []byte(testFile.contents), fileperms.PrivateFile)) } - _, err := loadAndResolveProjectConfig(ctx.FS(), false, testFiles[0].path) - require.ErrorIs(t, err, ErrDuplicateComponents) + config, err := loadAndResolveProjectConfig(ctx.FS(), false, testFiles[0].path) + require.NoError(t, err) + + // The included file is loaded after the parent, so its values override. + assert.Equal(t, "bbb2222", config.Components["abc"].Spec.UpstreamCommit) } func TestLoadAndResolveProjectConfig_DuplicateComponentGroups(t *testing.T) { @@ -429,15 +438,21 @@ dist-git-branch = "TenPointZero" assert.Len(t, config.Distros, 1) } -func TestLoadAndResolveProjectConfig_DuplicateComponentsAcrossFiles(t *testing.T) { - // Two separate config files both defining the same component should error. - // Unlike distros, components do not support merging across config files. +func TestLoadAndResolveProjectConfig_MergeComponentsAcrossFiles(t *testing.T) { + // Two separate config files both defining the same component should merge. + // Later files' non-empty fields override earlier files' fields. const configContents1 = ` [components.foo] +[components.foo.spec] +type = "upstream" +upstream-commit = "aaa1111" ` const configContents2 = ` [components.foo] +[components.foo.spec] +type = "upstream" +upstream-commit = "bbb2222" ` configPath1 := testConfigPath @@ -447,8 +462,183 @@ func TestLoadAndResolveProjectConfig_DuplicateComponentsAcrossFiles(t *testing.T require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath1, []byte(configContents1), fileperms.PrivateFile)) require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath2, []byte(configContents2), fileperms.PrivateFile)) - _, err := loadAndResolveProjectConfig(ctx.FS(), false, configPath1, configPath2) - require.ErrorIs(t, err, ErrDuplicateComponents) + config, err := loadAndResolveProjectConfig(ctx.FS(), false, configPath1, configPath2) + require.NoError(t, err) + + // The second file's upstream-commit should override the first. + assert.Equal(t, "bbb2222", config.Components["foo"].Spec.UpstreamCommit) +} + +func TestLoadAndResolveProjectConfig_MergeComponentsPreservesEarlierFields(t *testing.T) { + // When the later file only sets some fields, earlier fields should be preserved. + const configContents1 = ` +[components.curl] +[components.curl.spec] +type = "upstream" +upstream-commit = "abc1234" + +[components.curl.build] +with = ["ssl"] + +[components.curl.release] +calculation = "auto" +` + + // Second file only overrides upstream-commit; build and release should survive. + const configContents2 = ` +[components.curl] +[components.curl.spec] +type = "upstream" +upstream-commit = "def5678" +` + + configPath1 := testConfigPath + configPath2 := filepath.Join("/project", "extra.toml") + + ctx := testctx.NewCtx() + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath1, []byte(configContents1), fileperms.PrivateFile)) + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath2, []byte(configContents2), fileperms.PrivateFile)) + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, configPath1, configPath2) + require.NoError(t, err) + + comp := config.Components["curl"] + + // Overridden field should use the later value. + assert.Equal(t, "def5678", comp.Spec.UpstreamCommit) + + // Fields not set in the later file should be preserved from the earlier file. + assert.Equal(t, []string{"ssl"}, comp.Build.With) + assert.Equal(t, ReleaseCalculationAuto, comp.Release.Calculation) +} + +func TestLoadAndResolveProjectConfig_MergeComponentsSourceConfigFile(t *testing.T) { + // SourceConfigFile should point to the last file that contributed to the component. + testFiles := []struct { + path string + contents string + }{ + {testConfigPath, ` +includes = ["sub/include.toml"] + +[components.abc] +[components.abc.spec] +type = "upstream" +upstream-commit = "aaa1111" +`}, + {"/project/sub/include.toml", ` +[components.abc] +[components.abc.spec] +type = "upstream" +upstream-commit = "bbb2222" +`}, + } + + ctx := testctx.NewCtx() + + for _, testFile := range testFiles { + require.NoError(t, fileutils.MkdirAll(ctx.FS(), filepath.Dir(testFile.path))) + require.NoError(t, fileutils.WriteFile(ctx.FS(), testFile.path, []byte(testFile.contents), fileperms.PrivateFile)) + } + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, testFiles[0].path) + require.NoError(t, err) + + comp := config.Components["abc"] + require.NotNil(t, comp.SourceConfigFile) + + // SourceConfigFile should reference the last file that contributed. + assert.Equal(t, "/project/sub", comp.SourceConfigFile.dir) +} + +func TestLoadAndResolveProjectConfig_MergeComponentsMultipleComponents(t *testing.T) { + // When two files define different components, both should be present. + // When they also share a component, that component should be merged. + const configContents1 = ` +[components.alpha] +[components.alpha.spec] +type = "upstream" +upstream-commit = "aaa1111" + +[components.beta] +[components.beta.spec] +type = "upstream" +upstream-commit = "bbb2222" +` + + const configContents2 = ` +[components.alpha] +[components.alpha.spec] +type = "upstream" +upstream-commit = "ccc3333" + +[components.gamma] +[components.gamma.spec] +type = "upstream" +` + + configPath1 := testConfigPath + configPath2 := filepath.Join("/project", "extra.toml") + + ctx := testctx.NewCtx() + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath1, []byte(configContents1), fileperms.PrivateFile)) + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath2, []byte(configContents2), fileperms.PrivateFile)) + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, configPath1, configPath2) + require.NoError(t, err) + + // All three components should be present. + assert.Len(t, config.Components, 3) + assert.Contains(t, config.Components, "alpha") + assert.Contains(t, config.Components, "beta") + assert.Contains(t, config.Components, "gamma") + + // alpha should have the merged (overridden) commit. + assert.Equal(t, "ccc3333", config.Components["alpha"].Spec.UpstreamCommit) + + // beta should be unchanged. + assert.Equal(t, "bbb2222", config.Components["beta"].Spec.UpstreamCommit) +} + +func TestLoadAndResolveProjectConfig_MergeComponentsOverlaysAppended(t *testing.T) { + // Overlays are slice fields — MergeUpdatesFrom with mergo.WithAppendSlice + // should append later overlays to earlier ones. + const configContents1 = ` +[components.pkg] +[components.pkg.spec] +type = "upstream" + +[[components.pkg.overlays]] +type = "spec-add-tag" +tag = "BuildRequires" +value = "openssl-devel" +` + + const configContents2 = ` +[components.pkg] + +[[components.pkg.overlays]] +type = "spec-add-tag" +tag = "Requires" +value = "libcurl" +` + + configPath1 := testConfigPath + configPath2 := filepath.Join("/project", "extra.toml") + + ctx := testctx.NewCtx() + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath1, []byte(configContents1), fileperms.PrivateFile)) + require.NoError(t, fileutils.WriteFile(ctx.FS(), configPath2, []byte(configContents2), fileperms.PrivateFile)) + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, configPath1, configPath2) + require.NoError(t, err) + + comp := config.Components["pkg"] + + // Both overlays should be present (appended, not replaced). + require.Len(t, comp.Overlays, 2) + assert.Equal(t, "BuildRequires", comp.Overlays[0].Tag) + assert.Equal(t, "Requires", comp.Overlays[1].Tag) } func TestLoadAndResolveProjectConfig_ComponentGroupWithMembers(t *testing.T) {