fix: gnome-autoar - remove flagged files - #18626
fix: gnome-autoar - remove flagged files#18626Andrew Phelps (anphel31) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Repackages gnome-autoar to remove encrypted test fixtures that block package signing.
Changes:
- Adds a deterministic source-repacking script.
- Replaces the upstream archive with the sanitized artifact.
- Refreshes generated sources, release, and lock metadata.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
base/comps/components.toml |
Moves the customized component out of the inline list. |
base/comps/gnome-autoar/gnome-autoar.comp.toml |
Configures the replacement source archive. |
base/comps/gnome-autoar/modify_source.sh |
Removes fixtures and matching test cases. |
locks/gnome-autoar.lock |
Refreshes the input fingerprint. |
specs/g/gnome-autoar/gnome-autoar.spec |
Bumps the rendered release. |
specs/g/gnome-autoar/sources |
Records the replacement archive checksum. |
Suppressed comments (1)
base/comps/gnome-autoar/modify_source.sh:129
- The checksum manifest names
${ORIGINAL_NAME}, but that path is the cached upstream tarball; the modified output is${ORIGINAL_NAME}.modified. As written,sha512sum -c "${ORIGINAL_NAME}.sha512"always checks the old archive against the new hash and fails. Point the manifest at the file whose hash was computed.
echo "${MODIFIED_SHA512} ${ORIGINAL_NAME}" > "${ORIGINAL_NAME}.sha512"
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| VERSION="0.4.5" | ||
| ORIGINAL_NAME="gnome-autoar-${VERSION}.tar.xz" | ||
|
|
||
| # Upstream Source0 (download.gnome.org) SHA512, from specs/g/gnome-autoar/sources. |
There was a problem hiding this comment.
Fixed. Clarified the comment so it's explicit that UPSTREAM_SHA512 is the pristine upstream download.gnome.org checksum used to verify the download before repacking — and explicitly not the modified/served hash that now lives in specs/g/gnome-autoar/sources. That was the only reference to specs/.../sources; I didn't find a second occurrence at line 129 (the tail of the script only computes and prints the modified hash + the upload command, which is correct).
| [[components.gnome-autoar.source-files]] | ||
| filename = "gnome-autoar-0.4.5.tar.xz" | ||
| hash = "855c016959c216b233c5a7c07c8a96f8beeb74b55154fdc054f67768c4b4935b7dc35d16dcd8aa8feee9b5ec283acaf92dd0ea686ebcd9a2a00cc6a2753da4a1" | ||
| hash-type = "SHA512" | ||
| origin = { type = "download", uri = "https://azltempstaginglookaside.blob.core.windows.net/repo/pkgs_modified/gnome-autoar/gnome-autoar-0.4.5.tar.xz/sha512/855c016959c216b233c5a7c07c8a96f8beeb74b55154fdc054f67768c4b4935b7dc35d16dcd8aa8feee9b5ec283acaf92dd0ea686ebcd9a2a00cc6a2753da4a1/gnome-autoar-0.4.5.tar.xz" } | ||
| replace-upstream = true |
There was a problem hiding this comment.
Verified via the modify_source.sh post-conditions and azldev comp render --check-only (no drift): the script checks the upstream SHA512 before editing, removes the encrypted fixtures, drops the three test_encrypted* cases and their g_test_add_func registrations, and asserts siblings (e.g. test_readonly_directory) survive. meson uses only selective -Werror= flags (no -Werror=unused-function and no blanket werror), so removing the cases doesn't break compilation. A full RPM/mock build isn't runnable in my environment; CI performs it.
bd9c314 to
6ccc9dd
Compare
| cd "${WORKDIR}" | ||
|
|
||
| echo "[1/6] Downloading ${ORIGINAL_NAME}" | ||
| [[ -f "${ORIGINAL_NAME}" ]] || curl -fsSL --retry 3 -o "${ORIGINAL_NAME}" "${UPSTREAM_URL}" |
There was a problem hiding this comment.
Fixed. The download now writes to ${ORIGINAL_NAME}.part and is renamed to the final name only on success, so an interrupted transfer can't leave a truncated file that the -f guard would silently reuse.
| ) | xz -T1 -9e > "${ORIGINAL_NAME}.modified" | ||
|
|
||
| MODIFIED_SHA512=$(sha512sum "${ORIGINAL_NAME}.modified" | awk '{print $1}') | ||
| echo "${MODIFIED_SHA512} ${ORIGINAL_NAME}" > "${ORIGINAL_NAME}.sha512" |
There was a problem hiding this comment.
Fixed. The sidecar now records the modified file's name (${ORIGINAL_NAME}.modified) written to ${ORIGINAL_NAME}.modified.sha512, so sha512sum -c verifies against the file that was actually hashed.
6ccc9dd to
6dc41e8
Compare
| EXTRACT_DIR="extracted" | ||
| rm -rf "${EXTRACT_DIR}" | ||
| mkdir "${EXTRACT_DIR}" | ||
| tar -xf "${ORIGINAL_NAME}" -C "${EXTRACT_DIR}" |
There was a problem hiding this comment.
Good catch. I pinned umask 022 before extraction so the recorded file modes no longer depend on the caller's environment. One caveat: the whole script is now Python and xz output still isn't guaranteed identical across liblzma versions, so the docstring no longer claims cross-toolchain byte-reproducibility — the artifact published to the lookaside (pinned by SHA-512 in specs/g/gnome-autoar/sources) is the source of truth the build downloads.
gnome-autoar ships three encrypted extract-test fixtures (tests/files/extract/test-encrypted*/input/arextract.zip) that fail the package-signing scan and block signing. %check runs %meson_test, so removing the fixtures also drops the three meson cases that read them (test_encrypted, test_encrypted_request_passphrase, test_encrypted_wrong_passphrase and their registrations); test-only, not shipped in any binary RPM. An azldev archive overlay cannot be used here because the tarball also ships an absolute-target symlink fixture (test-symlink-parent/reference/arextract -> /tmp) that azldev's overlay extractor rejects; instead modify_source.sh repacks the tarball out-of-band and it is served via origin=download.
6dc41e8 to
72895f8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Suppressed comments (1)
base/comps/gnome-autoar/modify_source.py:120
- Because Ruff enables all rules and does not ignore D103 (
ruff.toml:8-14), this newly added public function failsruff checkwithout a docstring.
def main() -> None:
| if not original.exists(): | ||
| # No atomic-rename dance: the SHA-512 check below rejects any partial or | ||
| # corrupt download, so a re-run simply re-fetches it. | ||
| run(["curl", "-fsSL", "--retry", "3", "-o", ORIGINAL_NAME, UPSTREAM_URL]) |
| def run(cmd: list[str], **kwargs) -> subprocess.CompletedProcess: | ||
| return subprocess.run(cmd, check=True, **kwargs) |
| return subprocess.run(cmd, check=True, **kwargs) | ||
|
|
||
|
|
||
| def sha512_of(path: Path) -> str: |
| xz = subprocess.Popen( | ||
| ["xz", "-T1", "-9e"], stdin=tar.stdout, stdout=out | ||
| ) | ||
| tar.stdout.close() # allow tar to receive SIGPIPE if xz exits |
Tobias Brick (tobiasb-ms)
left a comment
Tobias Brick (tobiasb-ms)
left a comment
There was a problem hiding this comment.
There are a couple other outstanding questions that need to be resolved, but the code currently looks good.
| if extract_dir.exists(): | ||
| shutil.rmtree(extract_dir) | ||
| extract_dir.mkdir() | ||
| run(["tar", "-xf", ORIGINAL_NAME, "-C", str(extract_dir)]) |
There was a problem hiding this comment.
nit(non-blocking): Did you consider using the tarfile standard library? It supports this extraction and of the repackaging (though I'm not certain about guaranteed reproducibility).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
base/comps/gnome-autoar/modify_source.py:138
- Writing
curloutput directly to the cache filename makes interrupted runs poison the next run: if the process is terminated beforesubprocess.runreturns, the partial file remains,original.exists()skips the download, and checksum verification exits. Download to a.partfile and rename it only after success.
["curl", "-fsSL", "--retry", "3", "-o", ORIGINAL_NAME, UPSTREAM_URL],
| @@ -0,0 +1,224 @@ | |||
| #!/usr/bin/env python3 | |||
Tobias Brick (tobiasb-ms)
left a comment
Tobias Brick (tobiasb-ms)
left a comment
There was a problem hiding this comment.
Changes look good, but please squash the three commits into one, with a conventional commit message (the first commit has a good message). We do rebase merges, so all of the commits in a PR end up in the history, and all of these are one logical change.
While you're at it, look at the last commit, which seems to be an empty merge from 4.0. You should be rebasing rather than merging, but since it's empty it seems kind of funky and per above paragraph you should be squashing it out anyway.
Remove three encrypted extract-test fixtures from gnome-autoar and drop the three meson test cases that read them.
gnome-autoar ships three encrypted extract-test fixtures (
tests/files/extract/test-encrypted*/input/arextract.zip, password-protected zips). The package-signing scan can't inspect encrypted archives and rejects the .src.rpm, blocking signing. The files/cases are test-only and not shipped in any binary RPM.%checkruns%meson_test, so removing the fixtures also drops the three meson cases that read them (test_encrypted,test_encrypted_request_passphrase,test_encrypted_wrong_passphraseand their registrations).Why not an azldev archive overlay (as used for the other packages)? The tarball also ships an absolute-target symlink fixture (
tests/files/extract/test-symlink-parent/reference/arextract -> /tmp) that azldev's overlay extractor rejects while extracting the whole archive — before overlays apply. Instead,modify_source.shrepacks the tarball out-of-band and it's served viaorigin=download.azldev comp render --check-onlyreports no drift.