Skip to content

fix(mozjs140): remove packed Windows test fixture - #18773

Merged
reuben olinsky (reubeno) merged 1 commit into
4.0from
anphel/fix-mozjs140-flagged-files
Sep 11, 2026
Merged

fix(mozjs140): remove packed Windows test fixture#18773
reuben olinsky (reubeno) merged 1 commit into
4.0from
anphel/fix-mozjs140-flagged-files

Conversation

@anphel31

@anphel31 Andrew Phelps (anphel31) commented Sep 9, 2026

Copy link
Copy Markdown
Member

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.rpm and rejects it, blocking signing. The file is a media-sniffing negative test fixture; this package builds SpiderMonkey from js/src only, so nothing in the build reads it and it is not shipped in any binary RPM.

  • An azldev archive overlay (file-remove) drops the fixture and repacks the tarball; the post-overlay hash is pinned via origin = { type = "overlay" }.
  • Two file-search-replace overlays drop the now-dangling references in xpcshell.toml and test_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-only and azldev comp update --check-only both report no drift.

1. Buildazldev comp build -p mozjs140 succeeded, producing 5 RPMs in base/out/:

mozjs140-140.6.0-1.azl4.x86_64.rpm
mozjs140-devel-140.6.0-1.azl4.x86_64.rpm
mozjs140-debuginfo-140.6.0-1.azl4.x86_64.rpm
mozjs140-debugsource-140.6.0-1.azl4.x86_64.rpm
mozjs140-devel-debuginfo-140.6.0-1.azl4.x86_64.rpm

%check ran upstream's jstests.py and jit_test.py suites 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 zero ff-inst matches, and every sibling fixture in mediasniffer/test/unit/data/ is still present, so the file-remove overlay hit only its intended target.

Both file-search-replace results were checked in the tarball, not just offline:

  • xpcshell.toml — still valid TOML, the data/ff-inst.exe entry gone, the rest of support-files intact.
  • test_mediasniffer_ext.js — both the descriptive comment and the test case removed; data/bug1079747.mp4 retains its own correct comment ("MP4 with invalid box size (0) for ftyp"), confirming nothing was stranded or mislabeled.

Zero residual ff-inst / 875769 references in either file.

3. Inspect — RPMs copied into a mock chroot with mock --copyin and listed with rpm -qlp without installing. Neither mozjs140 nor mozjs140-devel contains any ff-inst, .exe, .dll, or mediasniffer path. The runtime package ships only libmozjs-140.so.0{,.0.0}, the README, and the license; -devel ships 392 files. This confirms the fixture was never in a binary RPM either before or after — the change is purely about the .src.rpm the 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:

  • ldd resolves every dependency — 0 not found.
  • LD_BIND_NOW=1 LD_PRELOAD=/usr/lib64/libmozjs-140.so.0 /bin/true exits 0, forcing eager resolution of all relocations and running the library's ELF constructors. LD_DEBUG=files confirms the loader really mapped it and pulled in libicuuc.so.77.
  • SONAME is libmozjs-140.so.0, matching the libmozjs-140.so.0(mozjs_140)(64bit) RPM provides; 454 JS_* symbols are exported.
  • pkg-config --modversion mozjs-140 returns 140.6.0 and jsapi.h is present from -devel.

Copilot AI balanced review requested due to automatic review settings September 9, 2026 05:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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 mozjs140 component definition that removes ff-inst.exe from the upstream source archive and cleans up its test references.
  • Pin the post-overlay (repacked) firefox-140.6.0esr.source.tar.xz via source-files with replace-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.

@anphel31
Andrew Phelps (anphel31) force-pushed the anphel/fix-mozjs140-flagged-files branch from 24d0b73 to 7bf897c Compare September 9, 2026 06:13
Copilot AI review requested due to automatic review settings September 9, 2026 06:13
@anphel31

Copy link
Copy Markdown
Member Author

Two follow-up changes pushed (amended into the single commit, per the squash convention):

1. Overlay metadata category corrected to azl-pruning. The three overlays were tagged azl-security-compliance, which .agents/skills/azldev-overlay-metadata/SKILL.md defines as "Makes FIPS or crypto-policy changes". Removing a file is azl-pruning"Removes content for AZL: unshipped deps, unneeded features, sub-packages, or files."

2. The three overlays moved into a single per-file overlay document. They were three inline [[components.mozjs140.overlays]] entries each repeating identical metadata. The skill calls that out directly: "if you find yourself stamping the same metadata on several inline overlays, that is a signal they are one logical change — move them into a single overlay file with one [metadata] block", and recommends the per-file layout for all new work. They are now in base/comps/mozjs140/overlays/0001-remove-packed-windows-test-fixture.overlay.toml with one top-level [metadata] block, auto-loaded via the project-wide overlay-files glob. mozjs140.comp.toml retains only the source-files pin.

This is a pure reorganization: same three overlays, same order, same archive/file/regex values, so the repacked tarball and its pinned SHA-512 are unchanged.


On the ~130 MiB tarball growth

Root cause is a missing knob rather than anything about this change. Upstream ships firefox-140.6.0esr.source.tar.xz at xz -9e (64 MiB dictionary, ratio 0.180); azldev's archive-overlay repack emits at the liblzma default preset (8 MiB dictionary, ratio 0.218). Dictionary size accounts for essentially the whole delta — it is a preset difference, not a blocking/threading difference.

Worth noting this component is the first large .tar.xz to go through the overlay-repack path. The only other origin = { type = "overlay" } pin in the tree is minizip-ng, a small .tar.gz, where a ~20% ratio difference is invisible. So this is newly-exposed rather than a regression.

Three options:

  1. Accept it. Simplest, but a 21% size increase on an already-large SRPM is a real cost.
  2. Add an xz preset option to the archive-overlay repack in azldev (e.g. per-component compression = { preset = "9e" }). This is the clean fix and would benefit every future large-archive overlay. Requires an azldev change.
  3. Repack out-of-band, following the merged gnome-autoar precedent. base/comps/gnome-autoar/modify_source.py pipes tar --sort=name --mtime=... --owner=0 --group=0 --numeric-owner into xz -T1 -9e and serves the result via origin = { type = "download", uri = ... } from the staging lookaside. That reproduces upstream's compression exactly and would land at or slightly below the original 613 MiB.

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 -9e over a 3.4 GiB tree is slow. Option 2 is the better long-term answer if we expect more large archives to need overlays.

Happy to go with whichever the maintainers prefer — flagging it rather than silently shipping the growth.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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

Comment thread base/comps/mozjs140/overlays/0001-remove-packed-windows-test-fixture.overlay.toml Outdated
Copilot AI review requested due to automatic review settings September 9, 2026 20:55
@anphel31
Andrew Phelps (anphel31) force-pushed the anphel/fix-mozjs140-flagged-files branch from 7bf897c to 9ac41ba Compare September 9, 2026 20:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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

Comment thread base/comps/mozjs140/mozjs140.comp.toml Outdated
@anphel31
Andrew Phelps (anphel31) marked this pull request as ready for review September 9, 2026 21:00
@anphel31
Andrew Phelps (anphel31) requested a review from a team as a code owner September 9, 2026 21:00
Copilot AI review requested due to automatic review settings September 9, 2026 21:02
@anphel31
Andrew Phelps (anphel31) force-pushed the anphel/fix-mozjs140-flagged-files branch from 9ac41ba to 6c6ebd5 Compare September 9, 2026 21:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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

Copilot AI review requested due to automatic review settings September 10, 2026 23:11
@anphel31
Andrew Phelps (anphel31) force-pushed the anphel/fix-mozjs140-flagged-files branch from 6c6ebd5 to e56c6c2 Compare September 10, 2026 23:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Copilot AI review requested due to automatic review settings September 10, 2026 23:27
@anphel31 Andrew Phelps (anphel31) changed the title fix: mozjs140 - remove flagged files fix(mozjs140): remove packed Windows test fixture Sep 10, 2026
@anphel31
Andrew Phelps (anphel31) force-pushed the anphel/fix-mozjs140-flagged-files branch from e56c6c2 to 321baa2 Compare September 10, 2026 23:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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-only evidence. AGENTS.md:24-30 requires 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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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

@anphel31

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
2 pipeline(s) were filtered out due to trigger conditions.

@reubeno
reuben olinsky (reubeno) merged commit 53bc117 into 4.0 Sep 11, 2026
12 checks passed
@reubeno
reuben olinsky (reubeno) deleted the anphel/fix-mozjs140-flagged-files branch September 11, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants