Attempt at an alternative corepack implementation. #1656
Conversation
|
@nathanhammond is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
Hrm, this would handle CI, but doesn't handle running E2E locally. Then again, our local E2E already doesn't do the right thing so this is maybe "same same"? |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| function setupCorepack(): string { | ||
| execa.sync("corepack", ["enable"]); | ||
| const corepack_path = execa.sync("which", ["corepack"]).stdout; | ||
| return path.dirname(corepack_path); | ||
| } |
There was a problem hiding this comment.
If we're getting rid of this, then we should add a section to CONTRIBUTING.md that alerts users that they need to make sure their $PATH is correctly setup.
For example, with my setup this change causes make e2e to hard fail (I do have corepack enabled):
olszewski@chriss-mbp cli % echo $PATH
/Users/olszewski/Library/pnpm:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/olszewski/.nvm/versions/node/v16.16.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/olszewski/.nvm/versions/node/v16.16.0/bin:/usr/local/go/bin:/Users/olszewski/go/bin:/usr/local/go/bin:/Users/olszewski/go/bin
olszewski@chriss-mbp cli % make e2e
npm install -g corepack@latest
npm WARN config global `--global`, `--local` are deprecated. Use `--location=global` instead.
changed 1 package, and audited 2 packages in 440ms
found 0 vulnerabilities
corepack enable
pnpm install --filter=cli
.. | +5 +
.. | Progress: resolved 5, reused 5, downloaded 0, added 0, done
node -r esbuild-register scripts/e2e/e2e.ts
Error: Command failed with exit code 1: yarn install
error Couldn't find any versions for "b" that matches "workspace:*"
yarn install v1.22.19
[1/4] Resolving packages...
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
at makeError (/Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/execa@5.1.1/node_modules/execa/lib/error.js:60:11)
at Function.module.exports.sync (/Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/execa@5.1.1/node_modules/execa/index.js:194:17)
at Monorepo.linkPackages (/Users/olszewski/code/vercel/turborepo/cli/scripts/monorepo.ts:162:17)
at Object.<anonymous> (/Users/olszewski/code/vercel/turborepo/cli/scripts/e2e/e2e.ts:52:8)
at Module._compile (node:internal/modules/cjs/loader:1105:14)
at Module._compile (/Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/esbuild-register@3.3.3_esbuild@0.14.49/node_modules/esbuild-register/dist/node.js:2258:26)
at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
at Object.newLoader [as .ts] (/Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/esbuild-register@3.3.3_esbuild@0.14.49/node_modules/esbuild-register/dist/node.js:2262:9)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12) {
shortMessage: 'Command failed with exit code 1: yarn install',
command: 'yarn install',
escapedCommand: 'yarn install',
exitCode: 1,
signal: undefined,
signalDescription: undefined,
stdout: 'yarn install v1.22.19\n' +
'[1/4] Resolving packages...\n' +
'info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.',
stderr: `error Couldn't find any versions for "b" that matches "workspace:*"`,
failed: true,
timedOut: false,
isCanceled: false,
killed: false
}
make: *** [e2e] Error 1
olszewski@chriss-mbp cli % which yarn
/opt/homebrew/bin/yarn
olszewski@chriss-mbp cli % which pnpm
/opt/homebrew/bin/pnpm
olszewski@chriss-mbp cli % pnpm -v
7.8.0
There was a problem hiding this comment.
Summary of a conversation that happened in Slack:
We decided that we should stay ahead of where things are going with corepack and begin adoption of it. There exist weird $PATH precedence issues that are not fun and it's still behind corepack enable, but we believe that those issues will fade as adoption of corepack increases and it moves toward being the default for node dev environments.
It's imperfect, it will make things a little weird for contributors during the transition, but we believe that it's worth the churn and add or name to the "adopters of corepack" list.
| @@ -66,7 +66,11 @@ fmt-go: $(GO_FILES) go.mod go.sum | |||
| install: | ./package.json | |||
| pnpm install --filter=cli | |||
|
|
|||
| e2e: install turbo | |||
| corepack: | |||
| npm install -g corepack@latest | |||
There was a problem hiding this comment.
This line isn't needed since corepack ships with Node.js
There was a problem hiding this comment.
The version shipped with Node.js currently includes this bug which causes the first run of the e2e tests on a machine to fail
| corepack enable | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install |
There was a problem hiding this comment.
It would be cool if this was corepack -- install. That way a change to packageManager wouldn't require another change here
There was a problem hiding this comment.
We've got a secondary pnpm caching setup above this in the file that is also dependent upon this. This is what happens when we're semi-early adopters.
There was a problem hiding this comment.
(corepack is not (yet?) in the business of delegating to package managers for certain functionality.)
| npm install -g corepack@latest | ||
| corepack enable | ||
|
|
||
| e2e: corepack install turbo |
There was a problem hiding this comment.
i'm sure i missed this in a slack thread somewhere, but is this essentially pnpm install turbo ?
There was a problem hiding this comment.
Nope! It reads hilariously close to that; but it's actually the enumeration of tasks that need to be run in advance of running the e2e task.
There was a problem hiding this comment.
oh... TIL. this is how Makefiles declare dependencies?
I don't actually know if this will work, but I was trying to make the system itself responsible for maintaining the executable paths so that we didn't need to account for it in our e2e scripts.