Skip to content

fix: generate macros for all subpackages#16955

Draft
PawelWMS wants to merge 5 commits into3.0-devfrom
pawelwi/macro_gen_fix
Draft

fix: generate macros for all subpackages#16955
PawelWMS wants to merge 5 commits into3.0-devfrom
pawelwi/macro_gen_fix

Conversation

@PawelWMS
Copy link
Copy Markdown
Contributor

@PawelWMS PawelWMS commented Apr 30, 2026

Related to #15578.

Summary

Fixes three related bugs in versionsprocessor that all caused the generated macros.releaseversions file to expose less information than spec authors expect:

  1. Macros were only generated for each spec's default top-level package; every subpackage (%package) was silently dropped.
  2. The macro name was keyed off the spec file's base name rather than the built RPM's name, so subpackages with different names overwrote each other's macros.
  3. Built RPMs whose names contain characters outside [A-Za-z0-9_-] (notably +, .) produced silently truncated macro definitions whose names were unusable.

Bug 1: only the default package was queried

In processSpecFile() (toolkit/tools/versionsprocessor/versionsprocessor.go), the spec query was issued with rpm.QueryHeaderArgument (--srpm):

packages, err := rpm.QuerySPEC(specFile, sourceDir, `%{NAME}: %{evr}\n`, buildArch, defines, rpm.QueryHeaderArgument)

The --srpm flag instructs rpmspec to return only the source package header. Even though the surrounding code loops over packages expecting one entry per built RPM, the query only ever returns a single line for the SRPM's own NEVR. As a result, no %azl_<subpackage>_* macros were emitted for any subpackage declared via %package — only the default package got macros.

This was raised during the original review of #15578 (review comment) but the switch to --builtrpms was not applied, only the query format and regex changes.

Bug 2: subpackage macros used the spec file name

processPackageVersionString derived the macro name from the spec file's base name (e.g. kernel.spec%azl_kernel_*) and then iterated over every queried package. Even after fixing Bug 1, every subpackage of a given spec would still write to the same %azl_<spec>_* macros, repeatedly overwriting the default package's values. Subpackages whose names differ from the spec name (e.g. kernel-headers, ca-certificates-shared, mlnx-ofa_kernel-devel) never produced their own dedicated macros.

Bug 3: package names with + or . produced phantom macros

processPackageVersionString only converted - to _ when forming the macro identifier. Names containing characters outside [A-Za-z0-9_-] (notably +, .) produced silently truncated macro definitions: when RPM's loadMacroFile parses a line like %azl_libsigc++30_version 3.0.7, it accepts only [A-Za-z0-9_] for the macro name, so it stops at the first +. The result is a macro named azl_libsigc with body ++30_version 3.0.7 (with a "missing whitespace before body" warning), and the intended %azl_libsigc++30_* macro is never defined.

A static sweep of the public corpus identified at least 22 names with + (e.g. libsigc++30, gcc-c++, libstdc++, dvd+rw-tools, Text-Tabs+Wrap, python3-prometheus_client+twisted, libpmemobj++-devel) and 2 names with . (rubygem-cool.io, jboss-interceptors-1.2-api). Static grep is a lower bound because some package names are macro-expanded at parse time.

Fix (toolkit changes)

  • Switch from rpm.QuerySPEC(... QueryHeaderArgument) to rpm.QuerySPECForBuiltRPMs. This makes rpmspec return one line per binary RPM that would be built from the spec, including all subpackages with their individual epoch/version/release.
  • Derive each macro name from the actual package name parsed out of the %{nevra} output, instead of from the spec file's base name. Each subpackage now gets its own %azl_<subpackage>_* set of macros.
  • Replace the targeted strings.ReplaceAll(name, "-", "_") substitution with a generic sanitizeMacroIdentPart helper (strings.Map-based) that maps every rune outside [A-Za-z0-9_] to _. This handles +, ., and any other special character that may appear in a built RPM name.
  • Reuse the existing rpmSpecBuiltRPMRegex / RpmSpecBuiltRPMRegex from rpmssnapshot by promoting it into internal/rpm (along with its index constants), and extend it to capture the optional epoch and to split version and release into separate groups.
  • rpmssnapshot now consumes the shared regex and keeps its existing RepoPackage.Version shape (epoch:version-release) by recombining the split groups.
  • processPackageVersionString no longer needs the spec file name or the dist tag: the package name comes from the regex, and the release no longer carries the dist tag (dist is its own capture group now), so the manual strings.Replace(release, distTag, "", 1) is gone.
  • Lifted the per-package error context out of processPackageVersionString and into the caller (processSpecFile), where the spec file name is still in scope.
  • Existing call sites passing --srpm for arch checks etc. are untouched.

After the fix, e.g. ca-certificates.spec yields macros for ca-certificates, ca-certificates-shared, ca-certificates-base, ca-certificates-tools, and ca-certificates-legacy — each with their own %azl_<subpackage>_version / _release (and _epoch when non-zero). And gcc.spec's gcc-c++ subpackage now produces %azl_gcc_c___version / %azl_gcc_c___release instead of a truncated phantom macro.

Spec consumer updates

Because the macro names are now keyed off the built RPM's name rather than the spec file's base name, any consumer that referenced a %azl_<spec_basename>_* macro for a spec whose default top-level package is not actually built had to be updated. The only such case in the corpus is mlnx-ofa_kernel-hwe.spec, which produces only mlnx-ofa_kernel-hwe-modules and mlnx-ofa_kernel-hwe-devel (no default mlnx-ofa_kernel-hwe RPM, since the top-level package has no %files section).

The 10 spec consumers that referenced the (no-longer-produced) %azl_mlnx_ofa_kernel_hwe_{version,release} macros now reference %azl_mlnx_ofa_kernel_hwe_modules_{version,release} instead:

  • HWE OOT modules: SPECS/{srp-hwe,iser-hwe,isert-hwe,mlnx-nfsrdma-hwe,xpmem-hwe}.spec
  • Their signed counterparts: SPECS-SIGNED/{srp-hwe-signed,iser-hwe-signed,isert-hwe-signed,mlnx-nfsrdma-hwe-signed,xpmem-hwe-modules-signed}.spec

All other %azl_*_* macros currently in use across the corpus (%azl_kernel_*, %azl_kernel_hwe_*, %azl_mlnx_ofa_kernel_*) refer to packages that do have %files sections in their producing spec files, so they remain valid.

No spec consumer referenced a + or .-bearing macro before this PR (those macros were always phantom), so no other consumer updates were needed for Bug 3.

Documentation updates

  • Reworded the "Dynamic versioning" paragraph in toolkit/docs/how_it_works/3_package_building.md to make it clear that the macro name comes from the built RPM (not the spec file) and to spell out the new sanitization rule, with gcc-c++%azl_gcc_c___version as a worked example.
  • Restructured section 1.3 of .github/instructions/oot-modules.instructions.md into a unified two-step "Macro naming rule" callout (built RPM name first, then sanitization), and updated the cheat-sheet table to use %azl_mlnx_ofa_kernel_hwe_modules_*.

Tests

  • Moved the regex-only tests for RpmSpecBuiltRPMRegex from rpmssnapshot_test.go into rpm_test.go, where the regex now lives. Added a TestRpmSpecBuiltRPMRegexSpecialCharsInName table-driven test confirming the regex correctly parses NEVRAs with + (libsigc++30, gcc-c++, python3-prometheus_client+twisted) and . (rubygem-cool.io, jboss-interceptors-1.2-api) in the name.
  • Updated versionsprocessor_test.go for the new processPackageVersionString signature (NEVRA input, no specFileName/distTag arguments). TestProcessSpecFile_WithSubpackages now verifies distinct %azl_multipkg_*, %azl_multipkg_devel_*, and %azl_multipkg_libs_* macros.
  • Added TestSanitizeMacroIdentPart covering empty strings, alphanumerics, hyphens, plus signs, dots, leading digits, mixed special chars, and non-ASCII Unicode.
  • Added TestProcessSpecFile_SubpackageWithSpecialChars end-to-end test that synthesizes a spec with %package -n libfoo++ and %package -n libfoo.io subpackages, runs processSpecFile, and verifies the emitted %azl_libfoo___* and %azl_libfoo_io_* macros.
  • Extended TestProcessPackageVersionString_TableDriven with +-bearing, .-bearing, and mixed-special-char cases.
  • Updated rpmssnapshot_test.go for the expanded regex (epoch group added, version/release split) and added an epoch-bearing input to TestGenerateResults.
  • All affected packages (internal/rpm, pkg/rpmssnapshot, versionsprocessor) pass with go test. The full toolkit/tools test run is otherwise green; the only failure is internal/pkggraph/TestReferenceDOTFile, which fails identically on origin/3.0-dev and is unrelated to this PR.

@PawelWMS PawelWMS requested a review from a team as a code owner April 30, 2026 06:58
@microsoft-github-policy-service microsoft-github-policy-service Bot added Tools 3.0-dev PRs Destined for AzureLinux 3.0 labels Apr 30, 2026
@PawelWMS PawelWMS marked this pull request as draft April 30, 2026 18:38
%global target_kernel_release %azl_kernel_hwe_release
%global target_mlnx_ofa_kernel_version %azl_mlnx_ofa_kernel_hwe_version
%global target_mlnx_ofa_kernel_release %azl_mlnx_ofa_kernel_hwe_release
%global target_mlnx_ofa_kernel_version %azl_mlnx_ofa_kernel_hwe_modules_version
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to the correct name of the macro after the toolkit update. Not an actual change to the version passed during the build, thus no release bump.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the documentation Improvements or additions to documentation label May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.0-dev PRs Destined for AzureLinux 3.0 documentation Improvements or additions to documentation Packaging Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant