fix: mozjs140 - remove flagged files - #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
| type = "file-search-replace" | ||
| archive = "firefox-140.6.0esr.source.tar.xz" | ||
| file = "toolkit/components/mediasniffer/test/unit/xpcshell.toml" | ||
| regex = '\n "data/ff-inst\.exe",' |
There was a problem hiding this comment.
issue(blocking): These regexes look quite brittle, with hard-coded whitespace chars (ditto for the one below). What's the simplest regex that could be used instead to remove the required entries?
There was a problem hiding this comment.
I'm looking into alternatives
There was a problem hiding this comment.
Good call — replaced both with whole-line anchored patterns, which drops the hard-coded indentation and the exact comment wording:
# xpcshell.toml
regex = '(?m)^.*ff-inst\.exe.*\n'
# test_mediasniffer_ext.js
regex = '(?m)^.*(?:bug 875769|ff-inst\.exe).*\n'(?m)^...\n matches the same way as the existing patterns in util-linux and rpmlint. Since Go's RE2 never lets . cross a newline, each match is confined to a single line, and (?m) is what makes ^ a line anchor rather than start-of-file.
The second one still needs the bug 875769 alternation: the fixture's entry is preceded by a comment line describing it, and file-search-replace is a global replace, so the alternation takes out both lines. Removing only the entry would strand that comment above the unrelated bug1079747.mp4 case and mislabel it. I added a note in the file explaining that.
Verification — against the upstream files at FIREFOX_140_6_0esr_RELEASE, old and new patterns produce byte-identical output:
| file | matches | lines | sha256 |
|---|---|---|---|
xpcshell.toml |
1 → 1 | 24 → 23 | feb0ac63… (both) |
test_mediasniffer_ext.js |
1 → 2 | 144 → 142 | 893be5d7… (both) |
Zero residual ff-inst / 875769 references either way, and 875769 occurs exactly once in the file so the alternation can't over-match. End-to-end, azldev comp prepare-sources repacks to the same pinned SHA-512, so specs/m/mozjs140/sources is unchanged. locks/mozjs140.lock is refreshed since the component config changed; render --check-only and update --check-only both report CHANGED=false.
If the hard-coded bug number is still more than you want, the alternative is '(?m)^.*bug 875769.*\n.*ff-inst\.exe.*\n' — it requires the two lines to stay adjacent, so it fails loudly on upstream drift instead of silently stranding the comment. Also verified byte-identical. Happy to switch if you prefer that trade-off.
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
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 deterministic repack emits a single-block xz stream, so the tarball grows from 613.3 MiB to 743.6 MiB. Uncompressed content is unchanged.
azldev comp render --check-onlyreports no drift.