fix(mozjs140): remove packed Windows test fixture - #18773
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change follows an existing in-repo pattern for removing scan-flagged fixtures via archive overlays and updates the lock + rendered outputs consistently.
Pull request overview
Removes a package-signing-scan flagged Windows PE test fixture from the mozjs140 upstream Firefox source tarball via azldev archive overlays, and pins the repacked archive hash so the SRPM no longer contains the flagged bytes.
Changes:
- Introduce a dedicated
mozjs140component definition that removesff-inst.exefrom the upstream source archive and cleans up its test references. - Pin the post-overlay (repacked)
firefox-140.6.0esr.source.tar.xzviasource-fileswithreplace-upstream = true. - Refresh rendered spec/sources and the component lock fingerprint to match the updated inputs.
File summaries
| File | Description |
|---|---|
| base/comps/mozjs140/mozjs140.comp.toml | New per-component config with archive overlays removing the flagged fixture and a pinned post-overlay source hash. |
| base/comps/components.toml | Removes the now-customized mozjs140 from the “unmodified Fedora-imported components” list. |
| locks/mozjs140.lock | Updates input-fingerprint to reflect the new component inputs. |
| specs/m/mozjs140/mozjs140.spec | Bumps release/changelog to capture the component change in rendered output. |
| specs/m/mozjs140/sources | Updates the SHA512 to the repacked, post-overlay source tarball hash. |
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
24d0b73 to
7bf897c
Compare
|
Two follow-up changes pushed (amended into the single commit, per the squash convention): 1. Overlay metadata category corrected to 2. The three overlays moved into a single per-file overlay document. They were three inline This is a pure reorganization: same three overlays, same order, same On the ~130 MiB tarball growthRoot cause is a missing knob rather than anything about this change. Upstream ships Worth noting this component is the first large Three options:
Option 3 works today with existing in-repo precedent, but it moves artifact generation outside the repo (pinned by hash, produced by a script rather than by config), and single-threaded Happy to go with whichever the maintainers prefer — flagging it rather than silently shipping the growth. |
There was a problem hiding this comment.
🟢 Approval recommended
The overlay approach and post-overlay source pinning follow established repository patterns for removing scanner-flagged fixtures, and the lock/rendered updates are consistent with the new component inputs.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
7bf897c to
9ac41ba
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is a narrowly-scoped pruning of unbuilt test fixtures with consistent overlay + hash/lock/spec updates, and only a minor documentation nit identified.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Lite
9ac41ba to
6c6ebd5
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The overlay+source pinning approach matches established repository patterns and the rendered spec/lock updates are consistent with the described change.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
6c6ebd5 to
e56c6c2
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The required build, RPM inspection, and mock-chroot smoke-test verification has not been documented.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Balanced
e56c6c2 to
321baa2
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Required build, RPM inspection, and mock-chroot smoke-test evidence is missing.
Review details
Suppressed comments (1)
base/comps/mozjs140/overlays/0001-remove-packed-windows-test-fixture.overlay.toml:14
- This output-affecting archive overlay is only accompanied by
comp render --check-onlyevidence.AGENTS.md:24-30requires a component build, RPM inspection, and mock-chroot smoke test for overlay/source changes before sign-off; please run and record those checks (or document why a step is impossible).
type = "file-remove"
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
The package-signing scan cannot inspect UPX-packed executables and rejects the .src.rpm over a Win32 test fixture in the Firefox source tarball, toolkit/components/mediasniffer/test/unit/data/ff-inst.exe. Remove it with an archive overlay that repacks and re-pins the tarball, and drop the two references to it in xpcshell.toml and test_mediasniffer_ext.js. The repack uses default compression, which grows the tarball from 613.3 MiB to 743.6 MiB; uncompressed content is unchanged. Test-only: this package builds only js/src, so no binary RPM changes. Signed-off-by: Andrew Phelps <anphel@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
321baa2 to
50c256a
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The overlays are narrowly scoped, generated state is consistent, and build, inspection, and smoke-test validation are documented.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
/azp run |
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. |
Remove a UPX-packed Win32 test fixture (
toolkit/components/mediasniffer/test/unit/data/ff-inst.exe) from the mozjs140 source tarball and drop the two references to it.The package-signing scan flags the packed PE inside the
.src.rpmand rejects it, blocking signing. The file is a media-sniffing negative test fixture; this package builds SpiderMonkey fromjs/srconly, so nothing in the build reads it and it is not shipped in any binary RPM.file-remove) drops the fixture and repacks the tarball; the post-overlay hash is pinned viaorigin = { type = "overlay" }.file-search-replaceoverlays drop the now-dangling references inxpcshell.tomlandtest_mediasniffer_ext.js.Note: the repack uses default compression rather than upstream's
xz -9e, so the tarball grows from 613.3 MiB to 743.6 MiB. Uncompressed content is unchanged.Testing
azldev comp render --check-onlyandazldev comp update --check-onlyboth report no drift.1. Build —
azldev comp build -p mozjs140succeeded, producing 5 RPMs inbase/out/:%checkran upstream'sjstests.pyandjit_test.pysuites against the built engine and passed, so the repacked tarball produces a functionally intact SpiderMonkey.2. Source verification — the repacked tarball in the SRPM hashes to exactly the pinned SHA-512 (
8d88a7f2…aece53e). Listing all 421,874 entries yields zeroff-instmatches, and every sibling fixture inmediasniffer/test/unit/data/is still present, so thefile-removeoverlay hit only its intended target.Both
file-search-replaceresults were checked in the tarball, not just offline:xpcshell.toml— still valid TOML, thedata/ff-inst.exeentry gone, the rest ofsupport-filesintact.test_mediasniffer_ext.js— both the descriptive comment and the test case removed;data/bug1079747.mp4retains its own correct comment ("MP4 with invalid box size (0) forftyp"), confirming nothing was stranded or mislabeled.Zero residual
ff-inst/875769references in either file.3. Inspect — RPMs copied into a mock chroot with
mock --copyinand listed withrpm -qlpwithout installing. Neithermozjs140normozjs140-develcontains anyff-inst,.exe,.dll, ormediasnifferpath. The runtime package ships onlylibmozjs-140.so.0{,.0.0}, the README, and the license;-develships 392 files. This confirms the fixture was never in a binary RPM either before or after — the change is purely about the.src.rpmthe signing scan reads.4. Smoke-test — installed into a mock chroot via
azldev adv mock shell --add-package. mozjs140 ships a shared library with no CLI binary (the spec packages only%{_libdir}/libmozjs-140.so.0*), so validation is a library-load test rather than<binary> --version:lddresolves every dependency — 0not found.LD_BIND_NOW=1 LD_PRELOAD=/usr/lib64/libmozjs-140.so.0 /bin/trueexits 0, forcing eager resolution of all relocations and running the library's ELF constructors.LD_DEBUG=filesconfirms the loader really mapped it and pulled inlibicuuc.so.77.SONAMEislibmozjs-140.so.0, matching thelibmozjs-140.so.0(mozjs_140)(64bit)RPM provides; 454JS_*symbols are exported.pkg-config --modversion mozjs-140returns140.6.0andjsapi.his present from-devel.