build: honor pre-computed ELF metadata during ld.so.cache generation - #2454
Conversation
smoser
left a comment
There was a problem hiding this comment.
Nice speedup to chase, and I like that a miss falls back to parsing so it degrades safely. My concern is mostly about where the data lives rather than the mechanics, so I'd like to settle that before this lands.
The xattr is a global side effect, not a private channel
The strip in buildImage only runs in apko's image build. Nothing else strips:
pkg/apk/apk/install.go:171appliesSCHILY.xattr.*on every install, and real apk-tools does the same. A plainapk addon a live system leavesuser.elf.metadataon every shared library, permanently.- melange's build environments are apk installs into a workspace, not apko image builds, so they carry it too.
- anything embedding
pkg/apkas a library gets it unless it calls the strip itself.
So "consumed and removed at image assembly" holds for exactly one consumer out of several. That's the part I'd want addressed regardless of direction.
Alternatives
A file in the control tar. I checked apk-tools: apk_extract_v2_entry (src/extract_v2.c) silently ignores any dot-prefixed control entry that isn't .PKGINFO, .INSTALL, or a known script name — no warning, and never written to disk. .melange.yaml already rides along this way, which is why updateScriptsTar has its 0555 filter (pkg/apk/apk/installed.go:190). expandapk.APKExpanded.ControlFS already exposes the control tar as an indexed FS on every install, so reading it is nearly free. That buys us one blob parsed once per package instead of N xattr set/get/remove plus a whole-tree walk, no strip step at all, nothing leaking into non-apko installs, and coverage by the package signature. Two constraints: the name must start with . (a non-dot control entry makes apk-tools return -APKE_V2PKG_FORMAT), and apk-tools 3's ADB format has no arbitrary-blob slot, so this is v2-only absent an upstream schema field.
Outside the apk entirely. What neither in-package option gives us is the ability to fix or extend this without rebuilding every package — and both of them need a melange producer change and a world rebuild before a single library sees the fast path. A sidecar we regenerate by rescanning the repo needs neither, and can be generated from already-published apks today.
Key it on file content rather than name-version and the usual staleness problem goes away too: every regular file in a data tar already carries APK-TOOLS.checksum.SHA1 as a PAX record, and we already read it (pkg/apk/apk/installed.go:116 writes it as the Z: line). So file sha1 -> {machine, sonames | not-dynamic} is a pure memo table. The key is the content, so an entry can't describe the wrong file; it dedupes across packages and versions; nothing lands in an image; a miss falls back to parsing. On the apko side it's also the simplest of the three consumers — a map lookup on a digest we already have, versus threading a per-package path->facts map from install into the build context. The work is a repo-side generator plus one more signed artifact next to APKINDEX, which is a shape we already have, and it's separable from apko.
Given the producer/consumer interface will take iteration wherever we put it, I lean toward the option that decouples the data's lifecycle from the package's.
Smaller things, independent of the above
The symlink resolution in stampedElfInfo looks redundant, and is weaker than what the fs already does. memFS.getNodeCountLinks (pkg/apk/fs/memfs.go:62) already follows symlinks on every component including the last, recursively up to maxLinks, and GetXattr goes through it; dirFS.GetXattr delegates to its overrides memFS. So GetXattr("usr/lib/libfoo.so.1", ...) already lands on the target. The hand-rolled version does a single Readlink, so a two-hop chain resolves to an intermediate path and then works only because getNode cleans up after it — two resolvers that can only ever diverge. I'd drop the isLink branch, the Readlink assertion, and the dir parameter.
Note this isn't a corner case: entries are only emitted when the dirent name equals the SONAME (internal/ldso-cache/ldsocache.go:352-353), so for a normal libfoo.so.1 -> libfoo.so.1.2.3 layout the dirent we scan is the symlink. If the branch stays, it needs tests for a two-hop chain and a symlink pointing into another directory.
stripELFMetadata walks the entire image with a GetXattr per regular file on every build, including builds where nothing was ever stamped, to undo something whose extent was already known — install.go:171 is the single place the xattr is ever set. Recording those paths, or even just an "anything stamped?" flag, makes this O(stamped) or a no-op.
user.elf.metadata is unvendored, in a namespace shared with every other tool on the system. Something like user.dev.chainguard.elfmeta costs nothing and can't collide. Worth noting in the package doc that user.* is only settable on regular files on Linux — which is implicitly why the stamp has to live on the target rather than the symlink.
The stamp becomes an unverified authority for cache contents. The fallback covers missing and malformed, not wrong: a bad SONAME in a stamp yields a bad ld.so.cache silently. Signed packages make that a defensible boundary and it applies to the control-file option equally, so it's not an argument between them — but it's worth being explicit about, and it's the one thing a content-keyed table would let us actually verify.
Unrelated to all of the above: +1 on finishLibInfo dropping the duplicated no-SONAME block. That was dead code.
The ld.so.cache scan needs only each library's machine and SONAMEs, but learns them today by opening and ELF-parsing every shared library in the image. Let a package build pre-compute those facts instead: pkg/elfmeta defines a vendored xattr (user.dev.chainguard.elfmeta) a producer such as melange stamps onto each built library — including an explicit "checked, not a dynamic object" form — and the scan reads the stamp in place of the parse. Any miss or malformed value falls back to parsing, so unstamped packages are unaffected and the path degrades safely. The stamp is ordinary package metadata: it rides the data section's PAX records like any other xattr, is covered by the package signature, and persists wherever the package's files do — installed systems and built images alike. No consumer strips or rewrites it, so every installer (apko, apk-tools, pkg/apk embedders) treats it identically. It lives on regular files only — Linux permits user.* xattrs on neither symlinks nor special files — and symlinked dirents resolve to their target's stamp through the filesystem's own GetXattr resolution. ParseLibFilename moves to pkg/elfmeta (re-exported here) so producers can share the eligibility test, and finishLibInfo folds the previously duplicated no-SONAME fallback into one place for the parsed and pre-computed paths alike. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c86bef9 to
4c89b14
Compare
|
The leak point is right, and the fix I've taken is to remove the special case rather than extend it: nothing strips the xattr anymore. Worth being transparent about where the strip came from — I was originally stripping so a downstream equivalence test could hold stamped and unstamped builds byte-identical; I've since adjusted that test to compare with the stamp waived, and with that gone the strip had no reason to exist. The xattr is now ordinary package metadata: applied by whatever installs the package, persisting wherever the package's files do, on installed systems and in images alike, and the package doc now says this plainly. On where the data lives: I want to keep it on the file. The stamp rides the data section's PAX records, so it's covered by the package signature — the same trust envelope the control-file option buys — and it travels with the file through any xattr-preserving transform without a side artifact to generate, distribute, or go stale. Your content-keyed table's real advantage is coverage of already-published packages without a rebuild, and nothing here precludes it: the consumer is structured as stamp-then-parse-fallback, and a digest-keyed memo table could slot in as another source later. I'd rather treat that as a separable follow-up than the vehicle. The control-tar option I'd pass on: v2-only, leans on apk-tools silently ignoring unknown dot entries, and still needs the producer change and world rebuild anyway. Taken as-is: the symlink resolution is dropped — you're right that GetXattr already resolves, and the stamped test's symlink case now exercises exactly that; the xattr is renamed to On authority: made explicit in the package doc — a stamp is exactly as authoritative as the package carrying it. A producer that stamps wrong facts mis-describes contents it could equally have shipped wrong outright; signing covers both identically. A derive-and-compare self-check at stamp time is a good producer-side guard, and I'd put it in melange when the emitter lands. |
Generating ld.so.cache costs a full ELF parse per shared library, and all the cache needs from each one is its machine and SONAMEs — facts the package build already had in hand. The new pkg/elfmeta package gives producers (melange, or any packaging tool) the pieces to pre-compute them: Extract derives the facts from an ELF, Encode/Decode define the wire form, and Xattr names the extended attribute that carries it — riding tar PAX records and the build filesystem like any other file metadata. A stamp may also record "checked, not a dynamic object", sparing consumers the parse that would rediscover that.
The cache scan consults the stamp before opening a file (resolving symlinks to the real file first) and falls back to parsing whenever a file is unstamped or a stamp is malformed, so mixed-provenance filesystems keep working and adopt the fast path package by package. Once the filesystem is assembled the stamps are stripped unconditionally — the hint is build-time input, and images never carry it, so stamped and unstamped packages produce identical images.
ParseLibFilename moves to pkg/elfmeta (re-exported from the internal package) so producers can stamp by the same name policy ldconfig scans by.