Update build-tools to pnpm 11#27509
Conversation
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (251 lines, 11 files), I've queued these reviewers:
How this works
|
| flub exec --no-private --concurrency=1 --releaseGroup $RELEASE_GROUP -- "$PACKAGE_MANAGER pack" | ||
| flub exec --no-private --concurrency=1 --releaseGroup $RELEASE_GROUP -- "mv -t $STAGING_PATH/pack/tarballs/ ./*.tgz" |
There was a problem hiding this comment.
This use of && seemed harmful to me, as it obscured the error (this script did not fail). Claude agreed:
These are separate statements (not chained with
&&) so thatset -eaborts the script on the first
failure. Underset -e, a failing command that is not the last link of an&&chain is exempt from
exit-on-error, so chaining would silently swallow a failedpnpm packand let the script continue.
The && below is also removed for the same reason.
| # the working tree. This cleanup deliberately runs here rather than in a package "postpack" script: under | ||
| # pnpm >=11, `pnpm pack` re-stats every "files" entry after postpack runs and fails with ENOENT if postpack | ||
| # deleted one. Running it here, after all packs complete, avoids that while still cleaning the working tree. | ||
| flub exec --no-private --releaseGroup $RELEASE_GROUP -- "pnpm run --if-present clean:manifest" |
There was a problem hiding this comment.
Due to the issue noted above, this was a bit nasty to root cause, but this fix seems reasonable.
I think pnpm 11 checking for odd changes during pack is good to help ensure packaged content reflects what it looks like was supposed to be in the package, which is nice for auditability/security against attacks which do strange things during publish (like the xz utils attack famously), so I don't think their policy change is a bug, just improved strictness which we violated and needed this change to satisfy.
| type: string | ||
|
|
||
| # The version of the build tools to install, or "repo" to install the one from the repository. | ||
| # If "repo" is selected, this includes a global install of what ever version of pnpm build-tools uses. |
There was a problem hiding this comment.
This isn't new, but it caused some complications in this change as this now installs pnpm 11, which broke tests-tools by overriding its pnpm 10 install.
## Description This code used to simply try and parse an empty string as JSON and give an unexpected end of JSON syntax error if there was some issue, like those caused by include-install-build-tools overwriting the pnpm version causing pnpm to try and install a different version which failed due to network isolation (Noted in #27537 and blocking #27509). Now it actually outputs the errors in such cases.
Description
Update build-tools to pnpm 11
Reviewer Guidance
The review process is outlined on this wiki page.