fix(release): drop dead dist/vendor copy; publish the verified tarball; cache cargo#331
Open
ig-ant wants to merge 1 commit into
Open
fix(release): drop dead dist/vendor copy; publish the verified tarball; cache cargo#331ig-ant wants to merge 1 commit into
ig-ant wants to merge 1 commit into
Conversation
…l; cache cargo dist/vendor/ is dead weight in the published package. Both resolvers already find binaries at the package-root vendor/ location: getSrtWinPath() via repoRoot()+vendor/srt-win/<arch>/, and getLocalSeccompPaths() via its baseDir/../../vendor/seccomp/<arch>/ candidate. The dist/vendor/ candidate in the seccomp resolver predates vendor/ being in package.json files[] — it stays in code for downstream-bundler layouts, but populating dist/vendor in OUR tarball serves nothing. prepublishOnly is now just clean+build; ~10MB of duplicate srt-win.exe and ~1.6MB of duplicate apply-seccomp dropped from every release. Publish the verified tarball: `npm pack` does not run prepublishOnly, so the tarball-contents check was inspecting a different shape than `npm publish` (directory mode) would ship. Run prepublishOnly explicitly, pack, verify, then publish THAT tarball via `npm publish "$PKG"` (tarball mode skips lifecycle scripts so no rebuild happens between verify and publish). Tarball listing is captured once and grepped in-memory. New assertion: dist/vendor/ is absent. verify-package: only the host-arch (x64) srt-win binary is exercised; drop build-seccomp from needs and the three unused artifact downloads. Presence of the others is covered by the tarball check. Cargo cache: Swatinem/rust-cache@v2 with shared-key=srt-win on the Windows legs of integration-tests.yml (warms the cache on main pushes) AND build-srt-win in release.yml (restores from main via the default-branch fallback). Placed after dtolnay/rust-toolchain so the cache key derives from the installed rustc.
ig-ant
force-pushed
the
ig/release-followup
branch
from
9e8d552 to
44c6704
Compare
ig-ant
changed the title
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
1 participant
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.
Follow-up to #321.
dist/vendor/ is dead weight
prepublishOnlycopiedvendor/{seccomp,srt-win}intodist/vendor/, so the published package shipped two copies of every prebuilt binary — ~10MB of duplicatesrt-win.exeand ~1.6MB of duplicateapply-seccompper release.Neither resolver needs the
dist/vendor/copy in an installed package:getSrtWinPath()resolves<pkgroot>/vendor/srt-win/<arch>/viarepoRoot()and has nodist/vendorcandidate at all.getLocalSeccompPaths()candidate Is there a way to not default-allow read access to the whole system? #2 (baseDir/../../vendor/seccomp/<arch>/) resolves<pkgroot>/vendor/seccomp/fromdist/sandbox/*.js. Its candidate DNS exfiltration is currently possible by default on MacOS #3 (dist/vendor/seccomp/) predatesvendor/being infiles[]— it stays in code for downstream-bundler layouts (wherebaseDirisn't<pkg>/dist/sandbox/), but populating it in our tarball serves nothing.prepublishOnlyis now justnpm run clean && npm run build.Publish the verified tarball
npm packdoes not runprepublishOnly— only directory-modenpm publishdoes. So #321's tarball-contents check was inspecting a different shape than actually shipped (nodist/vendor/, which is why it didn't catch the duplicate-binary bug).Now:
npm run prepublishOnly→npm pack→ verify →npm publish "$PKG". Publishing a tarball skips lifecycle scripts, so the verified artifact is byte-identical to what publishes. Tarball listing is captured once and grepped in-memory (wastar -tzf×7). New assertion:dist/vendor/absent.verify-package trimmed
Only the host-arch (x64)
srt-win.exeis exercised onwindows-latest; dropbuild-seccompfromneeds:and the three unused artifact downloads. Presence of the others in the tarball is covered by the publish job's contents check.Cargo build cache
Swatinem/rust-cache@v2withshared-key: srt-winon the Windows legs of bothintegration-tests.yml(warms the cache onmainpushes; speeds the ~5-8min cargo build on every PR) andrelease.ymlbuild-srt-win(restores frommainvia the default-branch cache fallback — without the shared key, the tag-ref release would never see a saved cache). Placed afterdtolnay/rust-toolchain@stableso the cache key derives from the installed rustc.