Add Yarn 2+ support to `pack` #1071

mattwebbio · 2022-12-30T00:39:15Z

Fixes #759

Closed #989 and updated this branch to support the deprecation of qq introduced in #1035.

strophy · 2022-12-31T05:24:29Z

Interesting, this one fails for me with:

oclif: gathering workspace for dashmate to /home/strophy/Code/dashpay/platform/packages/dashmate/tmp/dashmate
    Error: Command failed: yarn workspaces focus --production

    Code: 1

But I think you are on the right track again here, I'll close #1060 in favour of this. I see your point on the other PR that c.workspace() returns a workspace temp dir, this is not what I wanted. Something like c.config.name might be more appropriate? I'm trying to run the command in the monorepo root while specifying the focus package like this:

await exec(`yarn workspaces focus ${c.config.name} --production`, { cwd: yarnRoot })

But I'm not sure if this is standard yarn usage or something different in our monorepo, and also not sure if yarn.lock still needs to be copied first.

mattwebbio · 2022-12-31T16:49:44Z

If you were to run yarn workspaces focus --production by hand in /home/strophy/Code/dashpay/platform/packages/dashmate/tmp/dashmate, what's the output? Are you using Yarn 2 or Yarn 3? 🤔

strophy · 2023-01-02T00:48:10Z

Interesting, this reveals the underlying error:

dashmate/tmp/dashmate on  chore/update-chain-id [$] ❯ yarn workspaces focus --production
➤ YN0000: ┌ Resolution step
➤ YN0001: │ Error: @dashevo/dashpay-contract@workspace:*: Workspace not found (@dashevo/dashpay-contract@workspace:*)
    at ze.getWorkspaceByDescriptor (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:439:3273)
    at md.getCandidates (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:390:29963)
    at wd.getCandidates (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:391:1281)
    at wd.getCandidates (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:391:1281)
    at /home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:439:7764
    at Lg (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:390:11154)
    at ce (/home/strophy/.cache/node/corepack/yarn/3.2.2/yarn.js:439:7744)
➤ YN0000: └ Completed in 0s 414ms
➤ YN0000: Failed with errors in 0s 420ms

Same error with both yarn 3.2.2 and 3.3.0. Maybe a problem with the workspace?

peniqliotuv · 2023-01-20T23:29:40Z

Is this still in-progress?

mattwebbio · 2023-01-20T23:42:25Z

@peniqliotuv

Is this still in-progress?

This is working in an oclif CLI monorepo at @appfolio, but I've not had an opportunity to figure out why it isn't working for @strophy. On a previous PR also trying to fix this same issue, an engineer at Salesforce made it pretty clear that this is not a priority for them to test, review, and merge. As a result, it seems like it may be a waste of my time to continue pushing this forward.

peniqliotuv · 2023-01-21T01:25:33Z

@peniqliotuv

Is this still in-progress?

This is working in an oclif CLI monorepo at @appfolio, but I've not had an opportunity to figure out why it isn't working for @strophy. On a previous PR also trying to fix this same issue, an engineer at Salesforce made it pretty clear that this is not a priority for them to test, review, and merge. As a result, it seems like it may be a waste of my time to continue pushing this forward.

Got it - I'd hate to have to fork over oclif just to get support for this. We are using a monorepo with yarn berry - the only solution now is to create a nested .yarnrc and run yarn set version classic. This also restricts us from using our first-party libraries within our CLI. I'm surprised there aren't more use cases of this... but also Yarn berry isn't quite popular

strophy · 2023-01-21T01:33:53Z

Yep, sounds like you are experiencing the same problem as us @peniqliotuv

@mattwebbio I discussed this with our lead engineer last week and he determined that the error I pasted above is due to oclif needing to gather some more things from the workspace into the tmp directory, either as copies or symlinks. We will continue trying to figure out a fix based on this PR.

strophy · 2023-01-28T00:51:04Z

@mattwebbio we traced the problem above to quirks in the way our specific monorepo is set up. With additional steps on our side this PR is now tested working for us. @RodEsp Can we please get it merged?

shumkov · 2023-02-04T09:47:08Z

Yeah, it works for me as well. Please merge.

peniqliotuv · 2023-02-22T21:30:41Z

Any update on this?

peniqliotuv · 2023-02-22T21:32:13Z

Also I do believe for this to work, you'll need workspace-tools installed. (see: https://yarnpkg.com/cli/workspaces/focus)

src/tarballs/build.ts

Co-authored-by: Jerry Tsui <tiger.tsui@gmail.com>

peniqliotuv · 2023-03-13T17:28:10Z

@WillieRuemmele / @mdonnalley sorry for the direct ping, saw you were active maintainers

strophy · 2023-03-15T05:44:25Z

Or @peternhale can you help maybe?

(@peniqliotuv for background info I was getting good feedback from him when I first raised this issue way back in 2021)

peniqliotuv · 2023-03-20T16:54:00Z

I really appreciate this wonderful library but I'm really disappointed by the level of responsiveness for outstanding issues, especially when it was communicated that from the release of @oclif/core v2, the maintainers would be more active in the community. I wish there was a discord or something of sorts where community contributors could collaborate and ask/answer questions.

peniqliotuv · 2023-04-05T20:41:52Z

Hi, any update on this? @peternhale

WillieRuemmele · 2023-05-03T14:18:25Z

src/tarballs/build.ts

@@ -66,7 +66,21 @@ export async function build(c: BuildConfig, options: {
    const yarnRoot = findYarnWorkspaceRoot(c.root) || c.root
    if (fs.existsSync(path.join(yarnRoot, 'yarn.lock'))) {
      await fs.copy(path.join(yarnRoot, 'yarn.lock'), path.join(c.workspace(), 'yarn.lock'))
-      await exec('yarn --no-progress --production --non-interactive', {cwd: c.workspace()})
+
+      const yarnVersion = (await exec('yarn -v')).stdout.charAt(0)


Hey @mattwebbio - sorry for lack of comms is yarn guaranteed to be installed? This would throw a command not found error if not

The same issue would happen today on the production version of oclif today. I agree that we should add a check, but if possible, that should be another PR

WillieRuemmele · 2023-05-04T19:50:26Z

Hey @mattwebbio -

I'm trying to verify these changes today but can't get it working.

I've done the following:

check out your fork/branch
yarn set version berry
yarn --version => 3.5.1
yarn && yarn build

./bin/run pack:tarballs
oclif: gathering workspace for oclif to /Users/william.ruemmele/projects/oclif/oclif-fork/tmp/oclif
Error: Command failed: yarn workspaces focus --production

    at ChildProcess.exithandler (node:child_process:419:12)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:489:12)
    at maybeClose (node:internal/child_process:1091:16)
    at Socket.<anonymous> (node:internal/child_process:449:11)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
    at Pipe.<anonymous> (node:net:322:12)

using yarn 2.4.3 fails with the same error
using yarn 1.22.19 works

we should be able to run the ./bin/dev pack:tarballs command on the oclif/oclif repo, did I miss a step, or do something wrong? I don't think anyone on the team has adopted yarn 1+ (clearly 😜) so could be a user error too

amccarthy1

I was able to reproduce @WillieRuemmele 's issue with the steps provided, and confirmed that this is because of a missingworkspace-tools plugin. There's a catch block that's supposed to identify this and print a clearer message, but it looks like it's matching on the wrong pattern. Added a comment for a guard that works for me. Another alternative would be to print this error message for any "Command failed" errors, but that would likely catch more than just missing plugins.

amccarthy1 · 2023-05-08T15:21:57Z

src/tarballs/build.ts

+          if (error instanceof Error && error.message.includes('Command not found')) {
+            throw new Error('Missing workspace tools. Run `yarn plugin import workspace-tools`.')
+          }


I wasn't able to get this conditional to correctly identify Command not found errors; it seems like the resulting error's message is just Command failed, but it has an extra property stdout which includes the "Command not found" substring.

This is consistent with the NodeJS docs on this usage:

In case of an error (including any error resulting in an exit code other than 0), a rejected promise is returned, with the same error object given in the callback, but with two additional properties stdout and stderr.

It's a bit verbose, but this does catch it for me:

const isCommandNotFoundError = (error: unknown): boolean => { type ErrorWithStdout = Error & { stdout: string; } const typedError = error as ErrorWithStdout if (typedError instanceof Error && typeof typedError.stdout === 'string') { return typedError.stdout.includes('Command not found') } return false }

Hi @amccarthy1 - I tried adding a simplified version of your check

const isCommandNotFoundError = (error: unknown): boolean => { type ErrorWithStdout = Error & { stdout: string; } const typedError = error as ErrorWithStdout return typedError.stdout.includes('Command not found') }

this worked for yarn1 and yarn3 🎉

when I was testing with yarn2, and was told to yarn plugin import workspace-tools that command failed. I did quite a bit of researching but wasn't able to find a command or version that would install that plugin... if you can figure out a way to install workspace-tools in yarn 2 we could add that to the error message... otherwise I think it'd be ok to continue as is.

Hey @mattwebbio -

I'm trying to verify these changes today but can't get it working.

I've done the following:

check out your fork/branch

yarn set version berry

yarn --version => 3.5.1

yarn && yarn build

./bin/run pack:tarballs oclif: gathering workspace for oclif to /Users/william.ruemmele/projects/oclif/oclif-fork/tmp/oclif Error: Command failed: yarn workspaces focus --production at ChildProcess.exithandler (node:child_process:419:12) at ChildProcess.emit (node:events:513:28) at ChildProcess.emit (node:domain:489:12) at maybeClose (node:internal/child_process:1091:16) at Socket.<anonymous> (node:internal/child_process:449:11) at Socket.emit (node:events:513:28) at Socket.emit (node:domain:489:12) at Pipe.<anonymous> (node:net:322:12)

using yarn 2.4.3 fails with the same error using yarn 1.22.19 works

we should be able to run the ./bin/dev pack:tarballs command on the oclif/oclif repo, did I miss a step, or do something wrong? I don't think anyone on the team has adopted yarn 1+ (clearly 😜) so could be a user error too

Hi @WillieRuemmele - so I don't think that the oclif repository will actually work with Yarn 2.
I tried the steps above and couldn't even run yarn install (got this issue installing Typescript).
However, there does seem to be an issue with yarn >=2 <=3 when running plugin import workspace-tools. I've opened an issue here.

Something we could do is explicitly not support yarn 2, which seems reasonable considering that yarn >= 2 doesn't work at all today, and this issue is blocking other folks.

WillieRuemmele · 2023-06-12T20:55:09Z

Hey everyone and @mattwebbio , @peniqliotuv @amccarthy1 - I created my own PR with those changes we talked about - just merged it and should be publishing as I type this

fix: Support Yarn 2+ with pack

e191b59

salesforce-cla bot added the cla:signed label Dec 30, 2022

mattwebbio mentioned this pull request Dec 30, 2022

fix: support yarn 2+ with pack #1060

Closed

peniqliotuv suggested changes Mar 8, 2023

View reviewed changes

src/tarballs/build.ts Outdated Show resolved Hide resolved

fix: add detailed error if workspace tools are missing

a27d9db

Co-authored-by: Jerry Tsui <tiger.tsui@gmail.com>

mattwebbio force-pushed the addYarnThreeSupport branch from fab0127 to a27d9db Compare March 8, 2023 17:34

peniqliotuv approved these changes Mar 10, 2023

View reviewed changes

strophy approved these changes Mar 12, 2023

View reviewed changes

WillieRuemmele reviewed May 3, 2023

View reviewed changes

amccarthy1 reviewed May 8, 2023

View reviewed changes

WillieRuemmele mentioned this pull request Jun 12, 2023

fix: add yarn 1/3 support #1149

Merged

WillieRuemmele closed this in #1149 Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Yarn 2+ support to `pack` #1071

Add Yarn 2+ support to `pack` #1071

mattwebbio commented Dec 30, 2022

strophy commented Dec 31, 2022

mattwebbio commented Dec 31, 2022

strophy commented Jan 2, 2023

peniqliotuv commented Jan 20, 2023

mattwebbio commented Jan 20, 2023

peniqliotuv commented Jan 21, 2023

strophy commented Jan 21, 2023

strophy commented Jan 28, 2023

shumkov commented Feb 4, 2023

peniqliotuv commented Feb 22, 2023

peniqliotuv commented Feb 22, 2023

peniqliotuv commented Mar 13, 2023

strophy commented Mar 15, 2023

peniqliotuv commented Mar 20, 2023 •

edited

peniqliotuv commented Apr 5, 2023 •

edited

WillieRuemmele May 3, 2023 •

edited

peniqliotuv May 16, 2023

WillieRuemmele commented May 4, 2023

amccarthy1 left a comment

amccarthy1 May 8, 2023

WillieRuemmele May 15, 2023

peniqliotuv May 18, 2023 •

edited

WillieRuemmele commented Jun 12, 2023

Add Yarn 2+ support to pack #1071

Add Yarn 2+ support to pack #1071

Conversation

mattwebbio commented Dec 30, 2022

strophy commented Dec 31, 2022

mattwebbio commented Dec 31, 2022

strophy commented Jan 2, 2023

peniqliotuv commented Jan 20, 2023

mattwebbio commented Jan 20, 2023

peniqliotuv commented Jan 21, 2023

strophy commented Jan 21, 2023

strophy commented Jan 28, 2023

shumkov commented Feb 4, 2023

peniqliotuv commented Feb 22, 2023

peniqliotuv commented Feb 22, 2023

peniqliotuv commented Mar 13, 2023

strophy commented Mar 15, 2023

peniqliotuv commented Mar 20, 2023 • edited

peniqliotuv commented Apr 5, 2023 • edited

WillieRuemmele May 3, 2023 • edited

Choose a reason for hiding this comment

peniqliotuv May 16, 2023

Choose a reason for hiding this comment

WillieRuemmele commented May 4, 2023

amccarthy1 left a comment

Choose a reason for hiding this comment

amccarthy1 May 8, 2023

Choose a reason for hiding this comment

WillieRuemmele May 15, 2023

Choose a reason for hiding this comment

peniqliotuv May 18, 2023 • edited

Choose a reason for hiding this comment

WillieRuemmele commented Jun 12, 2023

Add Yarn 2+ support to `pack` #1071

Add Yarn 2+ support to `pack` #1071

peniqliotuv commented Mar 20, 2023 •

edited

peniqliotuv commented Apr 5, 2023 •

edited

WillieRuemmele May 3, 2023 •

edited

peniqliotuv May 18, 2023 •

edited