From 55ff21c61312a47355ab2dc5635e8dd9adc66e56 Mon Sep 17 00:00:00 2001 From: Andrew Phelps Date: Fri, 18 Sep 2026 11:45:13 -0700 Subject: [PATCH 1/2] fix(pkg): resolve --rpm-file packages per source package, not per name loadRPMFile kept the first mapping for a binary package name and discarded any later entry naming a different source package, warning that it had done so. The result was one entry per packageName, whose component - and therefore publish channel - depended on the order of entries in the file. A binary name produced by more than one component is legitimate. rubygem-bundler is emitted by both 'ruby' (rpm-base) and the standalone 'rubygem-bundler' component (rpm-sdk). A caller that listed both got a single answer chosen by position, so the same package resolved differently in different invocations depending on what else was in the map. That reached production. Control Tower builds its source map from an unordered collection, so its whole-corpus routing pass and its per-batch publish pass received different channels for rubygem-bundler. Publishing placed the RPMs in the repos that rpm-sdk maps to while the presence check expected the ones rpm-base maps to, so the component was re-queued every six hours for weeks, uploading nothing and reporting success. Each (packageName, sourcePackageName) pair is now preserved and reported, so a package with two producers yields one entry per producer, each resolved against its own component. The component is taken from the source package being expanded rather than looked up by name, which removes the order dependence entirely. resolvePackageListResult now receives an already-resolved component name, so both call paths state where the component came from. Only exact duplicate pairs are collapsed. The conflict warning and its TODO are gone: the condition it reported is handled rather than tolerated. Tests: the validation case that asserted first-mapping-wins now asserts one entry per source, and a new case checks that two producers resolve to their own channels and that reversing the order of the map does not change the result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/app/azldev/cmds/pkg/list.go | 90 +++++++++++------------ internal/app/azldev/cmds/pkg/list_test.go | 80 ++++++++++++++++++-- 2 files changed, 118 insertions(+), 52 deletions(-) diff --git a/internal/app/azldev/cmds/pkg/list.go b/internal/app/azldev/cmds/pkg/list.go index 3bf36a6d2..5049f313a 100644 --- a/internal/app/azldev/cmds/pkg/list.go +++ b/internal/app/azldev/cmds/pkg/list.go @@ -296,7 +296,9 @@ func ListPackages(env *azldev.Env, options *ListPackageOptions) ([]PackageListRe } for pkgName := range toResolve { - result, err := resolvePackageListResult(pkgName, compOf, pkgGroupOf, compGroupsOf, proj) + compName := resolveComponentName(pkgName, compOf, proj) + + result, err := resolvePackageListResult(pkgName, compName, pkgGroupOf, compGroupsOf, proj) if err != nil { return nil, err } @@ -324,14 +326,18 @@ func ListPackages(env *azldev.Env, options *ListPackageOptions) ([]PackageListRe // resolvePackageListResult resolves the publish channel and component membership for a single // package and returns a [PackageListResult]. +// +// compName is the component that owns this package in the requested context. Callers resolve it +// themselves — via [resolveComponentName] for project-driven listings, or from the source package +// being expanded for '--rpm-file' listings — because one binary package name can belong to +// different components depending on which source package produced it. func resolvePackageListResult( pkgName string, - compOf map[string]string, + compName string, pkgGroupOf map[string][]string, compGroupsOf map[string][]string, proj *projectconfig.ProjectConfig, ) (PackageListResult, error) { - compName := resolveComponentName(pkgName, compOf, proj) compConfig := resolveComponentConfig(compName, proj) // Apply inherited defaults (project → groups → component) so that @@ -490,9 +496,12 @@ func resolveSourcePackageListResult( // resolveFromRPMFile loads a source map file and resolves all SRPMs and their RPMs. // Returns a flat list of SRPM and RPM entries derived from the file. The returned slice is -// not ordered by contract; callers may sort or otherwise reorder the flattened results. The -// JSON file is parsed once by [loadRPMFile] to produce both the SRPM → RPMs map and the -// authoritative RPM → component index. +// not ordered by contract; callers may sort or otherwise reorder the flattened results. +// +// A binary package name produced by more than one source package yields one RPM entry per +// producing source, each resolved against that source's component. The component is taken from +// the source package being expanded rather than looked up by package name, so the result does +// not depend on the order of entries in the file. func resolveFromRPMFile( fs opctx.FS, path string, @@ -500,7 +509,7 @@ func resolveFromRPMFile( compGroupsOf map[string][]string, proj *projectconfig.ProjectConfig, ) ([]PackageListResult, error) { - srpmMap, rpmCompOf, err := loadRPMFile(fs, path) + srpmMap, err := loadRPMFile(fs, path) if err != nil { return nil, err } @@ -516,7 +525,9 @@ func resolveFromRPMFile( results = append(results, srpmResult) for _, rpmName := range rpmNames { - rpmResult, resolveErr := resolvePackageListResult(rpmName, rpmCompOf, pkgGroupOf, compGroupsOf, proj) + // The SRPM name is the component name by definition, so the producing source is + // the component for every binary it emits. + rpmResult, resolveErr := resolvePackageListResult(rpmName, srpmName, pkgGroupOf, compGroupsOf, proj) if resolveErr != nil { return nil, resolveErr } @@ -530,36 +541,32 @@ func resolveFromRPMFile( // loadRPMFile reads and parses a JSON RPM source map from path on fs. // The file is a JSON array of [rpmSourceEntry] records. -// Returns: -// - srpmMap: source package name → ordered list of binary RPM names it produces -// - rpmCompOf: binary RPM name → source package (component) name +// Returns srpmMap: source package name → ordered list of the binary RPM names it produces. // -// Both maps are built in a single pass over the JSON entries. If the same -// binary package name appears more than once, the first entry wins and later -// entries are skipped, keeping srpmMap and rpmCompOf aligned. Identical -// duplicates are skipped silently; conflicting duplicates (same packageName -// with a different sourcePackageName) emit a warning so operators can detect -// and remediate bad inputs. -func loadRPMFile(fs opctx.FS, path string) (srpmMap map[string][]string, rpmCompOf map[string]string, err error) { +// A binary package name may legitimately appear under more than one source package — several +// components can emit an RPM of the same name, and they may publish to different channels. Each +// such pair is preserved under its own source, so the caller can report one entry per +// (packageName, sourcePackageName). Exact duplicate pairs are collapsed silently. +func loadRPMFile(fs opctx.FS, path string) (srpmMap map[string][]string, err error) { data, readErr := fileutils.ReadFile(fs, path) if readErr != nil { - return nil, nil, fmt.Errorf("reading RPM source map %#q:\n%w", path, readErr) + return nil, fmt.Errorf("reading RPM source map %#q:\n%w", path, readErr) } var entries []rpmSourceEntry if err := json.Unmarshal(data, &entries); err != nil { - return nil, nil, fmt.Errorf("parsing RPM source map %#q:\n%w", path, err) + return nil, fmt.Errorf("parsing RPM source map %#q:\n%w", path, err) } srpmMap = make(map[string][]string) - rpmCompOf = make(map[string]string, len(entries)) + seen := make(map[rpmSourcePair]struct{}, len(entries)) for idx, e := range entries { packageName := e.PackageName sourcePackageName := e.SourcePackageName if packageName == "" { - return nil, nil, fmt.Errorf( + return nil, fmt.Errorf( "invalid RPM source map %#q entry %d:\nmissing non-empty 'packageName'", path, idx, @@ -567,7 +574,7 @@ func loadRPMFile(fs opctx.FS, path string) (srpmMap map[string][]string, rpmComp } if sourcePackageName == "" { - return nil, nil, fmt.Errorf( + return nil, fmt.Errorf( "invalid RPM source map %#q entry %d for package %#q:\nmissing non-empty 'sourcePackageName'", path, idx, @@ -575,35 +582,26 @@ func loadRPMFile(fs opctx.FS, path string) (srpmMap map[string][]string, rpmComp ) } - if existingSource, exists := rpmCompOf[packageName]; exists { - // First mapping wins. Skipping later entries keeps [srpmMap] and - // [rpmCompOf] aligned: each binary RPM appears exactly once in - // [srpmMap] under exactly the source package recorded in [rpmCompOf]. - // - //nolint:godox // intentional temporary workaround documented below. - // TODO: this is a temporary workaround tolerating - // upstream RPM source maps that list the same packageName under different - // sourcePackageName values. Once those duplicates are resolved at the - // source, restore the stricter behavior: error on conflicting - // sourcePackageName, only dedup identical mappings. - if existingSource != sourcePackageName { - slog.Warn( - "RPM source map contains conflicting source package mappings; first mapping wins", - "path", path, - "packageName", packageName, - "keptSourcePackageName", existingSource, - "skippedSourcePackageName", sourcePackageName, - ) - } - + pair := rpmSourcePair{PackageName: packageName, SourcePackageName: sourcePackageName} + if _, exists := seen[pair]; exists { + // An exact repeat carries no extra information; collapse it so a package is not + // reported twice for the same source. continue } + seen[pair] = struct{}{} srpmMap[sourcePackageName] = append(srpmMap[sourcePackageName], packageName) - rpmCompOf[packageName] = sourcePackageName } - return srpmMap, rpmCompOf, nil + return srpmMap, nil +} + +// rpmSourcePair identifies one binary package as produced by one source package. It is the unit +// of uniqueness in an RPM source map: the same packageName under a different sourcePackageName is +// a different fact, not a duplicate. +type rpmSourcePair struct { + PackageName string + SourcePackageName string } // synthesizeDebugPackages augments results with synthetic '-debuginfo' packages (one per diff --git a/internal/app/azldev/cmds/pkg/list_test.go b/internal/app/azldev/cmds/pkg/list_test.go index b7ca6b0a2..95cfcbf84 100644 --- a/internal/app/azldev/cmds/pkg/list_test.go +++ b/internal/app/azldev/cmds/pkg/list_test.go @@ -517,8 +517,9 @@ func TestListPackages_SRPMFile_UsesSRPMChannel(t *testing.T) { } // TestListPackages_RPMFile_Validation exercises the JSON parsing and validation -// error paths in 'loadRPMFile' (invalid JSON, missing fields, conflicting -// mappings) and the silent dedup path for repeated identical mappings. +// error paths in 'loadRPMFile' (invalid JSON, missing fields), the silent dedup +// path for repeated identical mappings, and the one-entry-per-source behaviour +// when a package name has several producers. func TestListPackages_RPMFile_Validation(t *testing.T) { const path = "/test-rpm-map.json" @@ -544,14 +545,14 @@ func TestListPackages_RPMFile_Validation(t *testing.T) { wantErrSub: "missing non-empty 'sourcePackageName'", }, { - // Conflicting source package names must dedup (first mapping wins): - // one SRPM result for "bash" + one RPM result, not two of either. - name: "conflicting source package names dedup", + // A package name produced by two sources is not a duplicate: each pair is + // reported, so two SRPM results and two RPM results. + name: "same package from two sources reported once per source", body: `[ {"packageName":"bash","sourcePackageName":"bash"}, {"packageName":"bash","sourcePackageName":"other"} ]`, - wantResults: 2, + wantResults: 4, }, { // Identical duplicate mappings must not produce duplicate entries: @@ -586,6 +587,73 @@ func TestListPackages_RPMFile_Validation(t *testing.T) { } } +// TestListPackages_RPMFile_MultipleProducersResolvePerSource verifies that a binary package +// name emitted by two components resolves to each component's own publish channel, and that the +// answer does not depend on the order of entries in the source map. +// +// Previously the loader kept the first mapping for a name and discarded the rest, so a caller +// that sent both producers got a single entry whose channel depended on which one happened to +// come first. Callers building the map from an unordered collection therefore got different +// answers for the same package in different batches. +func TestListPackages_RPMFile_MultipleProducersResolvePerSource(t *testing.T) { + const path = "/test-rpm-map.json" + + // Two components emit a binary named "rubygem-bundler", publishing to different channels. + newEnv := func(t *testing.T) *testutils.TestEnv { + t.Helper() + + testEnv := testutils.NewTestEnv(t) + testEnv.Config.Components["ruby"] = projectconfig.ComponentConfig{ + Name: "ruby", + Publish: projectconfig.ComponentPublishConfig{RPMChannel: "rpm-base", SRPMChannel: "rpm-base-srpm"}, + } + testEnv.Config.Components["rubygem-bundler"] = projectconfig.ComponentConfig{ + Name: "rubygem-bundler", + Publish: projectconfig.ComponentPublishConfig{RPMChannel: "rpm-sdk", SRPMChannel: "rpm-sdk-srpm"}, + } + + return testEnv + } + + channelByComponent := func(t *testing.T, body string) map[string]string { + t.Helper() + + testEnv := newEnv(t) + require.NoError(t, fileutils.WriteFile(testEnv.TestFS, path, []byte(body), fileperms.PublicFile)) + + results, err := pkgcmds.ListPackages(testEnv.Env, &pkgcmds.ListPackageOptions{RPMFile: path}) + require.NoError(t, err) + + channels := make(map[string]string) + + for _, r := range results { + if r.PackageName == "rubygem-bundler" && r.Type == pkgcmds.PackageTypeRPM { + channels[r.Component] = r.Channel + } + } + + return channels + } + + bundlerFirst := channelByComponent(t, `[ + {"packageName":"rubygem-bundler","sourcePackageName":"rubygem-bundler"}, + {"packageName":"rubygem-bundler","sourcePackageName":"ruby"} + ]`) + + rubyFirst := channelByComponent(t, `[ + {"packageName":"rubygem-bundler","sourcePackageName":"ruby"}, + {"packageName":"rubygem-bundler","sourcePackageName":"rubygem-bundler"} + ]`) + + want := map[string]string{ + "ruby": "rpm-base", + "rubygem-bundler": "rpm-sdk", + } + + assert.Equal(t, want, bundlerFirst, "each producer should resolve to its own component's channel") + assert.Equal(t, want, rubyFirst, "entry order in the source map must not change the result") +} + // TestListPackages_ComponentGroupsReportedSorted verifies that the // [PackageListResult.ComponentGroups] field reports every component-group the // resolved component belongs to, sorted alphabetically, even when the package From bdae30a71e99fa420bb9c32158f817f749dbe485 Mon Sep 17 00:00:00 2001 From: Andrew Phelps Date: Fri, 18 Sep 2026 13:13:13 -0700 Subject: [PATCH 2/2] docs(pkg): document the --rpm-file row-uniqueness contract The change to report one row per (packageName, sourcePackageName) is user-visible: a binary name produced by several components now appears once per producer. A script keying results by package name alone silently drops a producer, and which one survives depends on iteration order - the same failure this change removes, reintroduced one layer up. Adds a 'Row uniqueness' section to the inspect-package-config how-to with a worked rubygem-bundler example, and corrects the Component-column note, which said Component never means 'the component whose spec produces this package' - true for -a and -p, but the opposite of what it means under --rpm-file. Also states the contract in the command's own help text, and regenerates docs/user/reference/cli/azldev_package_list.md from it. Raised in review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/user/how-to/inspect-package-config.md | 39 +++++++++++++++++++ .../user/reference/cli/azldev_package_list.md | 7 +++- internal/app/azldev/cmds/pkg/list.go | 7 +++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/docs/user/how-to/inspect-package-config.md b/docs/user/how-to/inspect-package-config.md index ecbcfaa8c..3e0ddbf28 100644 --- a/docs/user/how-to/inspect-package-config.md +++ b/docs/user/how-to/inspect-package-config.md @@ -55,6 +55,9 @@ Example output: > per-package entry in its `packages` map — it does **not** mean "the component > whose spec produces this package". Packages that get their configuration only > from the project default or a package-group will show an empty Component. +> +> This differs under `--rpm-file`, where **Component** is the source package that produced +> the row — see [Row uniqueness](#row-uniqueness-a-package-name-may-appear-more-than-once). ## Look Up Specific Packages @@ -88,6 +91,42 @@ The output includes a `type` column to distinguish SRPMs (`srpm`) from binary RP SRPM entries use the component's `srpm-channel`; binary RPM entries use the full publish-channel resolution stack. +### Row uniqueness: a package name may appear more than once + +Rows are unique by **`(packageName, type, component)`**, not by package name alone. + +More than one component can produce a binary RPM of the same name, and those components may +publish to different channels. When a source map lists such a name under several +`sourcePackageName` values, each pair is reported separately, resolved against the component +that produced it: + +```json +[ + { + "packageName": "rubygem-bundler", + "type": "rpm", + "component": "ruby", + "publishChannel": "rpm-base" + }, + { + "packageName": "rubygem-bundler", + "type": "rpm", + "component": "rubygem-bundler", + "publishChannel": "rpm-sdk" + } +] +``` + +Both rows are correct: the binary shipped by `ruby` belongs on `rpm-base`, the one shipped by +the standalone `rubygem-bundler` component belongs on `rpm-sdk`. + +**Scripting consumers must key on the pair.** Loading this output into a map keyed by +`packageName` silently discards one producer, and which one survives depends on iteration +order — the same input can then yield different answers in different runs. + +Exact duplicate pairs — the same `packageName` under the same `sourcePackageName` — are +collapsed to one row, so repeating an entry in the source map does not duplicate output. + > **Note:** `--rpm-file` is mutually exclusive with `-a`, `-p`, and `--synthesize-debug-packages`. ## Machine-Readable Output diff --git a/docs/user/reference/cli/azldev_package_list.md b/docs/user/reference/cli/azldev_package_list.md index e6165273e..dca2b959e 100644 --- a/docs/user/reference/cli/azldev_package_list.md +++ b/docs/user/reference/cli/azldev_package_list.md @@ -14,9 +14,14 @@ or component package overrides). Use --rpm-file to enumerate all source packages (SRPMs) and their binary RPMs from a JSON RPM source map file (an array of {"packageName":"bash","sourcePackageName":"bash"} records). Each SRPM is resolved against the component with the same name; each binary RPM is -resolved using the full publish-channel stack. Results include a 'type' column +resolved against the source package that produced it. Results include a 'type' column ("srpm" or "rpm") to distinguish the two. +Rows are unique by (packageName, type, component), not by package name alone: when a +binary name is produced by several components, each producer is reported separately with +its own publish channel. Scripts must key on the pair — keying on the package name alone +silently drops a producer. + Use -p (or positional args) to look up one or more specific packages by exact name — including packages that are not explicitly configured (they resolve using only project defaults). diff --git a/internal/app/azldev/cmds/pkg/list.go b/internal/app/azldev/cmds/pkg/list.go index 5049f313a..440007917 100644 --- a/internal/app/azldev/cmds/pkg/list.go +++ b/internal/app/azldev/cmds/pkg/list.go @@ -60,9 +60,14 @@ or component package overrides). Use --rpm-file to enumerate all source packages (SRPMs) and their binary RPMs from a JSON RPM source map file (an array of {"packageName":"bash","sourcePackageName":"bash"} records). Each SRPM is resolved against the component with the same name; each binary RPM is -resolved using the full publish-channel stack. Results include a 'type' column +resolved against the source package that produced it. Results include a 'type' column ("srpm" or "rpm") to distinguish the two. +Rows are unique by (packageName, type, component), not by package name alone: when a +binary name is produced by several components, each producer is reported separately with +its own publish channel. Scripts must key on the pair — keying on the package name alone +silently drops a producer. + Use -p (or positional args) to look up one or more specific packages by exact name — including packages that are not explicitly configured (they resolve using only project defaults).