feat: add noarch package architecture support - #2645
Open
lioneloh wants to merge 12 commits into
Open
Conversation
Add support for a "noarch" architecture, mirroring Alpine APKBUILD's arch=noarch: a package that does not depend on host-specific binary code (Python programs, Java bytecode, etc.) is compiled once using the host's native architecture and its apk output is replicated and indexed into every requested per-architecture output directory as arch=noarch, instead of being rebuilt once per --arch. Implemented per docs/plans/noarch-architecture.md: - Step 1 (pkg/config): reuse target-architecture: [noarch] as a third sentinel alongside "all"; enforce it must be the sole entry via a hard parse error; add Package.IsNoArch(). - Step 2 (pkg/build, pkg/cli): force the guest/toolchain arch to runtime.GOARCH for a noarch package regardless of --arch; collapse BuildCmd/TestCmd to construct exactly one *build.Build/*build.Test per invocation instead of one per requested arch; fix a pre-existing gap where `melange test` silently skipped every noarch package. - Step 3 (pkg/build): add Build.PackageArch() (returns "noarch" for metadata/output while the real build arch stays untouched) and Build.ReplicateArchs/replicateTargets(); build once into a staging directory (a unique per-process os.MkdirTemp dir, not a fixed path, to avoid collisions between concurrent invocations sharing an --out-dir) and copy the result into every target arch's output directory; generalize the inline APKINDEX generation to loop over the replication targets (behaviorally identical to before for non-noarch builds). - Step 4 (pkg/index): accept arch=noarch packages into any --arch-scoped `melange index` run via a small matchesExpectedArch helper, instead of rejecting them on a strict string mismatch. - Step 5 (pkg/linter): new "noarch" linter (default: Require) that flags any ELF binary found in a package declared noarch; a guaranteed no-op for every other package. Each step built with a worker/reviewer subagent pass; the reviewer caught and the worker fixed two real bugs along the way (the melange-test skip gap in step 2, and a staging-directory collision/leak-on-failure pair in step 3). Remaining per the plan: step 6 (SBOM arch, likely already covered by step 3's PackageArch() wiring), step 7 (rebuild.go host-arch handling for arch=noarch), and step 8 (end-to-end tests).
Step 6 of docs/plans/noarch-architecture.md (SBOM/PURL arch) turned out to already be fully satisfied by step 3's Build.PackageArch() wiring: BuildPackage's SBOM GeneratorContext.Arch already uses PackageArch(), which flows into both the SPDX package's Arch field and its PURL arch qualifier, and the SBOM is embedded inside the apk's data section (not a sidecar file), so it's already covered by step 3's copy-replication. No change needed there. Reviewing turned up one adjacent, previously-missed spot: the FDO package-metadata linker template (embedded via --package-metadata / -Xlinker and the .note.package ELF note, consumed by SBOM/CVE scanners for statically-linked or vendored code) still used .Arch.ToAPK for its "architecture" field. Since the template's root object is *Build itself, .PackageArch is a direct drop-in replacement. This can't affect a *passing* noarch build in practice, since the new "noarch" linter (step 5) already rejects any ELF binary in a package declared noarch -- but it's an in-scope consistency fix, so apply it here. Verified by exercising the template (not just compiling it, since text/template method resolution isn't caught by go build/vet): TestCreateFdoNoteHeader's literal "architecture":"x86_64" assertion still passes unchanged for a normal build.
Both were verified via reviewer-only investigation rather than a worker pass, since neither required a code change: - Step 6 (SBOM arch) was already fully satisfied by step 3's Build.PackageArch() wiring; only a small adjacent fix (compiler_config.go's FDO package-metadata template) was needed, already committed separately. - Step 7 (rebuild.go) is already handled end-to-end by step 2's shared build.New noarch-override, which fires identically for RebuildCmd's call path (via WithConfiguration setting the real embedded original config before the override runs) as it does for a normal `melange build --arch`. Record the trace for both in the plan doc.
Add e2e-tests/noarch-build-test.yaml: a minimal, self-contained, greeter-style package declaring target-architecture: [noarch], with a real test: pipeline assertion (not a no-op) following the existing tester-blob pattern. Uses the existing -build-test filename convention (no run-tests/README changes) so CI runs it exactly like every other e2e test, on the real melange binary against a real runner. Its content is pure shell script (no compiled/ELF binaries), so it also proves the new "noarch" linter doesn't false-positive on legitimate content. Verified the fixture parses through melange's real config.ParseConfiguration (not just generic YAML syntax): IsNoArch() is true, and both the build and test pipelines parse with the expected step counts. This is item 1 of the two-part step 8 e2e plan; item 2 (multi-arch replication fan-out, requiring a small run-tests change) follows separately.
…em 2)
Add e2e-tests/noarch-multiarch-build.yaml plus a small, additive
e2e-tests/run-tests change: a new *-multiarch-build filename
convention (matched before the existing generic *-build case) that
builds a noarch package with --arch=x86_64,aarch64 in one invocation,
then asserts both packages/x86_64/ and packages/aarch64/ received the
apk and their own APKINDEX.tar.gz.
This closes the one gap unit tests structurally couldn't reach: the
real CLI -> BuildCmd -> BuildPackage -> real runner -> real apk/index
write path for the "one build, N replicated arch directories" fan-out
added in an earlier step. Every other existing e2e-tests/*.yaml is
unaffected -- arch_flag defaults to ${ARCH} exactly as before unless a
file matches the new suffix, and the vrc success/failure control flow
is behaviorally identical to the prior one-line form.
Costs nothing extra in CI: ReplicateArchs (grepped, confirmed) is never
consumed by any container/runner/kernel-selection code, since the real
guest build for a noarch package always runs on the CI host's actual
native architecture regardless of how many arches are requested -- the
second "aarch64" arch here is purely a directory-copy target, no QEMU
emulation or kernel fetch involved.
Per review: the assertion hardcodes the exact fixture filename
(noarch-multiarch-test-1.0-r0.apk) rather than a glob. Kept as-is
deliberately -- it fails loud (not silently) if the fixture and script
ever drift out of sync, and a looser glob would risk passing against
stale leftover artifacts from a prior run.
…nd-break BuildCmd used to always call build.New with WithArch(archs[0]) first, learning only afterwards (via bc.Configuration.Package.IsNoArch()) whether to break out of the per-arch loop. Since archs defaults to apko_types.AllArchs when --arch is omitted, and AllArchs[0] is "386", a plain `melange build` on a noarch package logged a misleading "ignoring requested architecture x86 for noarch package" -- the user never requested x86, it was purely an artifact of loop ordering. Extract New's config-resolution block (config-file auto-detection, the ConfigFile/ConfigFileRepositoryURL/ConfigFileRepositoryCommit validation, and the config.ParseConfiguration call) into Build.resolveConfiguration, called by New at the exact same point the inline block used to run -- a pure extraction, New's observable behavior and error precedence are unchanged. Add a narrow exported build.PeekIsNoArch(ctx, opts...) that resolves just enough configuration to answer the one question BuildCmd needs before deciding how many build contexts to construct, without any of New's heavier setup. BuildCmd now branches explicitly: noarch builds exactly one context with the real host arch supplied from the start (runtime.GOARCH, not archs[0]), everything else keeps the original per-arch loop (including ErrSkipThisArch handling) untouched. Scoped to BuildCmd only, per discussion -- TestCmd/NewTest have an analogous loop-and-break pattern with the same log-message inaccuracy, left as a separate, intentional follow-up.
…loop replicateTargets() only ever reads Build.ReplicateArchs when Configuration.Package.IsNoArch() is true. The non-noarch branch of BuildCmd only runs when build.PeekIsNoArch already returned false for this exact configuration, so every *build.Build constructed there is guaranteed non-noarch -- ReplicateArchs can never be consulted for them. Setting it was inert; drop it so the branch doesn't imply replication behavior that never applies here.
Same shape as the earlier BuildCmd refactor (560dd5c): NewTest always parsed config unconditionally and only learned via Configuration.Package.IsNoArch() after the fact whether to override Arch to the host arch, and TestCmd looped over archs breaking early once it hit a noarch config -- the same misleading "ignoring requested architecture x86" log bug as BuildCmd had, since archs[0] defaults to AllArchs[0] ("386") when --arch is omitted. Extract NewTest's config-loading into Test.resolveConfiguration (simpler than Build's -- no file auto-detection, no ConfigFileRepositoryURL/Commit validation, those fields don't exist on Test). Add build.PeekIsNoArchTest mirroring PeekIsNoArch. TestCmd now peeks first and branches explicitly: noarch builds exactly one *build.Test with the real host arch supplied last (after baseOpts, so it always wins over anything baseOpts might set -- matching BuildCmd's option ordering), everything else keeps the original per-arch loop. Also remove dead code found along the way: TestCmd's errors.Is(err, build.ErrSkipThisArch) handling around each build.NewTest call could never trigger -- that sentinel is defined and returned only by build.New's TargetArchitecture filter, which NewTest never implemented. The actual "is this arch in scope" decision for melange test happens later, per already-constructed *Test, inside TestPackage's own inarchs check. Explicitly left alone: TestPackage's inarchs logic has a separate, pre-existing, unrelated quirk where target-architecture: ["all"] causes apko_types.ParseArchitecture("all") to never equal a real arch, silently skipping all tests for that deprecated sentinel value. Not introduced or touched by this change.
A noarch package's own .PKGINFO correctly and permanently says arch = noarch -- that's provenance about how it was built. But an APKINDEX.tar.gz describes one specific architecture's repository, and every entry in e.g. packages/x86_64/APKINDEX.tar.gz should report arch: x86_64, not the literal string "noarch", so apk clients resolving against that arch-specific index see an ordinary matching architecture without needing any noarch-aware special-casing. In pkg/index/index.go's UpdateIndex, after the existing matchesExpectedArch filter passes (filtering semantics unchanged), rewrite the in-memory *apk.Package's Arch field from "noarch" to idx.ExpectedArch when one was given. Standalone `melange index *.apk` with no --arch flag has no concrete arch to stamp, so it's left as literally "noarch" as before. This only mutates the in-memory struct used for index serialization -- the apk file on disk and its embedded .PKGINFO are never touched. This alone couldn't fix a real `melange build`, though: the inline per-arch index generation in pkg/build/build.go's BuildPackage never set index.WithExpectedArch at all, so idx.ExpectedArch was always empty there. Added index.WithExpectedArch(arch.ToAPK()) to that loop's options -- a no-op for non-noarch builds (replicateTargets() returns only the package's own single real arch there, which already matches trivially), and the fix that actually makes every per-arch index directory generated by a real build correctly stamp its noarch packages now. Tested by generalizing the existing real-fixture-mangling test helper (mangleApk -> mangleApkControl) to byte-rewrite libcap-2.69-r0.apk's actual embedded control section (arch = aarch64 -> arch = noarch, same-length swap, no control-section corruption), producing a genuine noarch apk run through the real UpdateIndex end-to-end -- not a synthetic struct test.
Author
|
could resolve #1002 |
Member
|
I approved the workflow to run and added some engineers to review in more detail. I also pinged them on our internal slack |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add support for building and indexing noarch packages.
How it works
noarchis set on package.noarcharchitecture inPKGINFOand SBOM.noarchwith corresponding concrete architecture in eachAPKINDEX.This reproduces existing Alpine
lddtreebehavior:https://pkgs.alpinelinux.org/package/edge/main/x86/lddtree
Validation
go test ./pkg/build ./pkg/index ./pkg/config ./pkg/linter ./pkg/cligit diff --check origin/main...HEADUploaded logs from restic and gzip build from wolfi os ditribution
melange_restic.txt
melange_gzip.txt
AI assistance
PR prepared with assistance from Claude Sonnet 5 and GPT-5.6-Luna.