ci: fix corrupted distributions when merging parallel build artifacts - #514
Draft
bhimrazy wants to merge 3 commits into
Draft
ci: fix corrupted distributions when merging parallel build artifacts#514bhimrazy wants to merge 3 commits into
bhimrazy wants to merge 3 commits into
Conversation
The `merge-artifacts` job downloaded every `*-build-*` artifact with
`merge-multiple: true` into a single `dist/` folder. Build jobs running on
several OSes produce identically named files for a platform-independent
package, so the artifacts were extracted concurrently onto the same paths and
the writes interleaved, yielding a corrupted archive.
This surfaced as a flaky `check-package-extras / pkg-check` failure that passed
on re-run, since whether the interleaving damaged the payload was a race:
FileNotFoundError: .../src/lightning_utilities/install/requirements.py
ERROR: Failed to build lightning_utilities-0.15.3.tar.gz
Download each artifact into its own sub-folder instead and flatten them
sequentially, keeping the first of any duplicate name and reporting the rest.
`twine check` did not catch this: it stops at the first unreadable tar header
and treats it as end-of-archive, so it validated the intact prefix and never
reached the gzip CRC. Verify the archives end to end as well.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #514 +/- ##
=====================================
Coverage ? 72%
=====================================
Files ? 17
Lines ? 749
Branches ? 0
=====================================
Hits ? 540
Misses ? 209
Partials ? 0 🚀 New features to boost your workflow:
|
`download-artifact` only places each artifact in its own sub-folder when the pattern resolves to more than one; a lone artifact is extracted directly into the path. Globbing `dist-parts/*/*` therefore found nothing for single-build callers such as `check-package`, leaving `dist/` empty and failing on an opaque `twine check: error: the following arguments are required: dist`. Collect files at any depth so both layouts work, and fail with an explicit message when no distribution is found at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the flaky
check-package-extras / pkg-checkfailure that shows up on most PRs and goes away on re-run.The
merge-artifactsjob downloads every*-build-*artifact withmerge-multiple: trueinto a singledist/folder. When the build matrix has more than one OS, each build job produces identically named files (this package ispy3-none-any, so all builds emit the same sdist/wheel names).download-artifactextracts them concurrently onto the same paths, the writes interleave, and the result is a corrupted archive.That is why only
check-package-extrasfails: it is the only caller with a two-OS build matrix.check-package, which uses the same workflow with a single build OS, never fails.Evidence
The artifacts from run 28387440212 are still downloadable, so this can be checked directly:
lightning_utilities-0.15.3.tar.gzgzip -t…-build-06dc1de8c…(33560 B)…-build-10c3c2665…(33553 B)65bbf534…(33560 B)invalid compressed data--crc errorEach build artifact is individually intact; the merged one matches neither and is corrupt. Enumerating it stops after 30 members and
src/lightning_utilities/install/requirements.pyis missing — exactly what pip reported:The wheel in that same merged artifact is byte-identical to
build-1, i.e. one file won its race cleanly while the other was shredded.Changes
1. Do not extract multiple artifacts into one folder. Drop
merge-multiple: trueso each lands in its own sub-folder, then flatten sequentially, keeping the first of any duplicate name and logging the ones dropped. Files with distinct names (e.g. real platform-specific wheels) are all preserved.2. Actually verify archive integrity.
twine checkpassed on the corrupted tarball above — it stops at the first unreadable tar header and treats it as end-of-archive, so it validated the intact prefix and never reached the gzip CRC. Added a full decompress of any sdist and atestzip()of any wheel.Testing
The shell logic was exercised locally against the real artifacts downloaded from the failing run:
build-0, duplicates loggeddist/→ still fails loudlyNote this is a reusable workflow, so the second change had to stay safe for callers that do not produce an sdist.
Notes
check-package-extrasto a single build OS inci-use-checks.yamlwould remove the duplicate at the source and save some CI time. Deliberately not done here: it would only fix this repo and leave the same hazard for any other caller of this reusable workflow that uses a multi-OS build matrix. Happy to add it as a follow-up.bad checksumvariant seen in run 28387450718, wheretwine checkdid fail, is consistent with the same cause at a different corruption offset, but that run died before uploading a merged artifact so it could not be byte-verified.