Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

upgrade to Yarn 4 #912

Merged
merged 6 commits into from
Aug 22, 2024
Merged

upgrade to Yarn 4 #912

merged 6 commits into from
Aug 22, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jan 8, 2024 •
edited
Loading

Agoric/agoric-sdk#8461 is failing on integration with this repo

This PR upgrades Yarn from v1 to v4 and fixes issues that emerged with the stricter package resolution.

It also formats with Prettier to reduce code whitespace churn.

Review by commit is recommended

@turadg turadg requested review from dckc and 0xpatrickdev January 8, 2024 18:36
@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Jan 8, 2024 •
edited
Loading

Deploying documentation with  Cloudflare Pages  Cloudflare Pages

Latest commit: 27b7c08
Status: ✅  Deploy successful!
Preview URL: https://1dcdba8e.documentation-7tp.pages.dev
Branch Preview URL: https://ta-yarn-4.documentation-7tp.pages.dev

View logs

dckc
dckc reviewed Jan 8, 2024
View reviewed changes
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to hear more about endo versions before approving

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@dckc dckc mentioned this pull request Jan 8, 2024
Open
dckc
dckc previously approved these changes Jan 8, 2024
View reviewed changes
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm content to deal with endo version stuff separately

Agoric/agoric-sdk#8621 remains open

@turadg
Copy link
Member Author

turadg commented Jan 8, 2024

This is failing agoric install because that command uses the --link-folder option and thus assumes Yarn version 1 for downstream projects.

$ yarn workspaces focus [--json] [--production] [-A,--all] ...
$ yarn workspaces foreach [--from #0] [-A,--all] [-R,--recursive] [-W,--worktree] [-v,--verbose] [-p,--parallel] [-i,--interlaced] [-j,--jobs #0] [-t,--topological] [--topological-dev] [--include #0] [--exclude #0] [--no-private] [--since] [-n,--dry-run] <commandName> ...
agoric: (Error#1)
Error#1: yarn install updates failed in .
  at yarnInstallEachWorktree (file:///home/runner/work/documentation/documentation/agoric-sdk/packages/agoric-cli/src/install.js:73:15)
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async installMain (file:///home/runner/work/documentation/documentation/agoric-sdk/packages/agoric-cli/src/install.js:279:3)
  at async Command.parseAsync (/home/runner/work/documentation/documentation/agoric-sdk/node_modules/commander/lib/command.js:935:5)
  at async main (file:///home/runner/work/documentation/documentation/agoric-sdk/packages/agoric-cli/src/main.js:400:5)

This seems like an unnecessary coupling to me. I could make it check for the Yarn version but I wonder if @michaelfig has any better ideas.

dckc
dckc reviewed Feb 3, 2024
View reviewed changes
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turadg notes it's failing in some way...

@dckc dckc dismissed their stale review February 3, 2024 17:15

seems to be failing

@michaelfig
Copy link
Member

seems to be failing

It would be great to port agoric install to whatever corepack would use, but I don't have much experience outside of Yarn 1, so it would take some time to gather the requisite knowledge to do it. I'd expect to see that put in priority on a project board somewhere.

@turadg turadg marked this pull request as draft March 19, 2024 16:30
@turadg turadg mentioned this pull request Apr 9, 2024
Open
turadg
turadg commented Aug 22, 2024
View reviewed changes
snippets/zoe/contracts/test-callSpread.js
@@ -9,8 +9,8 @@ import '@agoric/zoe/exported.js';
import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
import { AmountMath } from '@agoric/ertp';

import { setup } from '@agoric/zoe/test/unitTests/setupBasicMints.js';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newer versions of @agoric/zoe don't bundle tests path (thankfully)

@turadg turadg force-pushed the ta/yarn-4 branch 2 times, most recently from e6d3715 to 33567a6 Compare August 22, 2024 01:00
@turadg
Copy link
Member Author

turadg commented Aug 22, 2024

Cloudflare build failure looks due to 9a487fc, not yarn. If other jobs pass then I'll rerun omitting that commit. cc @Jovonni @amessbee the agoric-sdk submodule is at Agoric/agoric-sdk@03b6420 which is 4.5 months old.

@github-actions GitHub Actions
Copy link

Cloudflare deployment logs are available here

@turadg turadg marked this pull request as ready for review August 22, 2024 14:25
@turadg turadg requested review from dckc and toliaqat and removed request for 0xpatrickdev August 22, 2024 14:26
@turadg turadg enabled auto-merge August 22, 2024 15:56
kriskowal
kriskowal approved these changes Aug 22, 2024
View reviewed changes
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What’s the plan for the copypasta from Zoe test infra? Do you have a separate change to export those in a future version?

@turadg turadg merged commit 8fa8208 into main Aug 22, 2024
5 checks passed
@turadg turadg deleted the ta/yarn-4 branch August 22, 2024 16:47
@turadg
Copy link
Member Author

turadg commented Aug 22, 2024

Do you have a separate change to export those in a future version?

No plan. Test support code seems very copyable to me. If it provides what the tests need, it works. If the tests' needs change they can go back upstream to see if there's something better to copy.

If we want to abstract that into a supported set of exports, I think it needs a new package: Agoric/agoric-sdk#7392

@turadg turadg mentioned this pull request Aug 22, 2024
Merged
mergify bot added a commit to Agoric/agoric-sdk that referenced this pull request Aug 22, 2024
@mergify
fixup dapp CI config (#9947)
5b61dc2 
_incidental_

## Description
documentation CI is [failing](https://github.com/Agoric/agoric-sdk/actions/runs/10513357847/job/29128771880?pr=9945) since Agoric/documentation#912

This fixes it

### Security Considerations


### Scaling Considerations


### Documentation Considerations


### Testing Considerations


### Upgrade Considerations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kriskowal kriskowal kriskowal approved these changes

@dckc dckc Awaiting requested review from dckc

@toliaqat toliaqat Awaiting requested review from toliaqat

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@turadg @michaelfig @kriskowal @dckc