redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.15k stars 980 forks source link

Github action fails with yarn 3 since bumping to v0.49.1 #4822

Closed noire-munich closed 11 months ago

noire-munich commented 2 years ago

We've bumped to v0.49.1 today in an attempt to finally get to yarn 3 - super excited.

Locally it runs very well and caused little troubles to upgrade.

The problem occurs when using a github action.

The workflow, truncated:

on:
  push:
    branches:
      - preprod

name: Deploy preprod

env:
  repo_name: ***
  node_version: 16.6.1

jobs:
  deploy:
    name: Deploy
    runs-on: ubuntu-latest
    environment: preproduction

    steps:
      - name: Configure AWS credentials
        uses: aws-actions/configure-aws-credentials@v1
        with:
          aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
          aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
          aws-region: ${{ secrets.AWS_REGION }}

      - name: Login to Amazon ECR
        id: login-ecr
        uses: aws-actions/amazon-ecr-login@v1

      - name: Checkout
        uses: actions/checkout@v2

      - name: Create environment files
        working-directory: aws
        run: |
          yarn

          # Generate ebextensions env file
          node ./generate_preprod_env.js ebextensions > ./.ebextensions/02-envvars.config

          # Generate dotenv file
          node ./generate_preprod_env.js dotenv > ../.env

// the rest is irrelevant

This part has been working well until now, and debugging proves to be rather difficult.

The action fails on step Create environment files and unfolding the details only show a diff of a file which should not have been edited outside of our local files. There is no explicit error message:

image

$ yarn rw info
  System:
    OS: Linux 5.13 Ubuntu 21.10 21.10 (Impish Indri)
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.6.1 - /tmp/xfs-a76dd022/node
    Yarn: 3.2.0 - /tmp/xfs-a76dd022/yarn
  Browsers:
    Chrome: 99.0.4844.51
    Firefox: 98.0
  npmPackages:
    @redwoodjs/core: ^0.49.1 => 0.49.1 
    @redwoodjs/eslint-config: ^0.49.1 => 0.49.1 
    @redwoodjs/record: ^0.47.0 => 0.47.1 
thedavidprice commented 2 years ago

@noire-munich Yarn 3 includes greatly improved resolution than Yarn 1, including more tools to verify and keep yarn.lock robust.

Do take a look at the new (and removed) options for yarn install in v3. For CI, you'll see we always run yarn install --immutable. Our Smoke Test workflow is a great example to follow in general: https://github.com/redwoodjs/redwood/blob/main/.github/workflows/smoke-test.yaml

You should also get in the habit of running yarn dedup locally.

To me, this reads like you have a busted yarn.lock. I'd try deleting and rebuilding. And/or start with yarn dedupe locally. Definitely use yarn install --immutable in CI.

Keep me posted!

thedavidprice commented 2 years ago
  npmPackages:
    @redwoodjs/core: ^0.49.1 => 0.49.1 
    @redwoodjs/eslint-config: ^0.49.1 => 0.49.1 
    @redwoodjs/record: ^0.47.0 => 0.47.1 

Why is Record at a different version? Are all packages at the correct version?

virtuoushub commented 2 years ago

Looking at the screenshot, it seems as if the lockfile is using the yarn v1 format.

Agree with @thedavidprice , updating CI to use yarn install --immutable explicitly is a good suggestion (it is supposed to default to true on CI, but by setting we remove all doubt). Hopefully that gives more feedback about what is going on.

FWIW in some of my personal projects where I use Yarn 3, I have noticed that dependabot will clobber my yarn v3 formatted lockfile and replace it with a v1 format. This results in the exact same Post-resolution validation errors.

noire-munich commented 2 years ago

Why is Record at a different version? Are all packages at the correct version?

@thedavidprice hum that's a mistake indeed - it's a preproduction branch and I had a branch for RW version bump and one which introduced RW records, both were not at the same version indeed.

Nevertheless, I recreated the preproduction branch from main and only merged the version bump branch. I then:

name: Deploy preprod

env: repo_name: *** node_version: 16.6.1

jobs: deploy: name: Deploy runs-on: ubuntu-latest environment: preproduction

steps:
  - name: Configure AWS credentials
    uses: aws-actions/configure-aws-credentials@v1
    with:
      aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
      aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
      aws-region: ${{ secrets.AWS_REGION }}

  - name: Login to Amazon ECR
    id: login-ecr
    uses: aws-actions/amazon-ecr-login@v1

  - name: Checkout
    uses: actions/checkout@v3

  - name: Setup node
    uses: actions/setup-node@v3
    with:
      node-version: 16.6.1

  - name: Get yarn cache directory path
    id: yarn-cache-dir-path
    run: echo "::set-output name=dir::$(yarn config get cacheFolder)"

  - name: Cache yarn
    uses: actions/cache@v2
    id: yarn-cache
    with:
      path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
      key: yarn-1234-${{ hashFiles('yarn.lock') }} #change yarn-{randomString} to bust the cache
      restore-keys: |
        yarn-1234

  - name: Create environment files
    working-directory: aws
    run: |
      yarn install --immutable


Which introduces the `yarn install --immutable`.

I still get the same error though.

@virtuoushub is it something you have resolved in those projects?
virtuoushub commented 2 years ago

@noire-munich - yes ( I stopped using dependabot which stopped the lockfile clobber )

If I had to guess what is going on, something in the environment ( maybe actions/setup-node ? ) is still using yarn 1 in CI.

noire-munich commented 2 years ago

I removed dependabot, still same problem with the lockfile - even after I cleaned yarn.lock. Don't know about actions/setup-node, I didn't have it until I took example on the smoke-tests. Also, rebuilt preprod branch several times from main, resulting in no change whatsoever.

virtuoushub commented 2 years ago

@noire-munich is there an example repo I can take a look at?

noire-munich commented 2 years ago

@virtuoushub unfortunately we don't have such a repo and I'm not sure I can manage one in the coming two weeks at least - not quite sure what would need to be included in it. Would you be up for a call, say 30 minutes max, to talk about this issue and steps to reproduce/debug ?

simoncrypta commented 2 years ago

@virtuoushub Can this cause by the thing you said on slack about the engines of package.json, Which should be "yarn": ">=3.2.0" and not "yarn": ">=1.15 <2" ?

virtuoushub commented 2 years ago

@virtuoushub Can this cause by the thing you said on slack about the engines of package.json, Which should be "yarn": ">=3.2.0" and not "yarn": ">=1.15 <2" ?

@simoncrypta , not sure; but this is a great callout! AFAIK yarn@2+ ignores the engines part of a package.json file, however yarn@1 should fail fast.

So, @noire-munich, one quick thing to try is to make sure your package.json's engines section is similar to:

...
  "engines": {
    "node": ">=14.17 <=16.x",
    "yarn": ">=3.2.0"
  },
...

While we are there we can also try adding a packagemanager field to the package.json file. edit npx @redwoodjs/codemods@canary upgrade-yarn does this part for us. I will leave the snippet below for transparency.

...
  "packageManager": "yarn@3.2.0"
...

What this should do is make sure that yarn@3 is used to install dependencies, and that your CI/CD environment is compatible with the yarn@3 version. If it is not, at least it should better indicate where the problem is.

@virtuoushub unfortunately we don't have such a repo and I'm not sure I can manage one in the coming two weeks at least - not quite sure what would need to be included in it. Would you be up for a call, say 30 minutes max, to talk about this issue and steps to reproduce/debug ?

👋🏻 @noire-munich , unfortunately I will not be available for a call. I outlined some things to try above. If those do not work to resolve the issue, and you do end up having a repo for me to take a look at, then I will be happy to help by checking it out.

noire-munich commented 2 years ago

unfortunately I will not be available for a call. I outlined some things to try above. If those do not work to resolve the issue, and you do end up having a repo for me to take a look at, then I will be happy to help by checking it out.

@virtuoushub No problem! Thanks a lot for the help :+1:

Ok so I checked the two points:

I fixed according to your diff @virtuoushub , removed node_modules and yarn.lock, ran yarn dedupe then yarn install and pushed my commit. Nothing has changed, there's still the diff between v1 and v3 yarn.lock versions.

Only thing I eventually noticed is the ending line of that diff:

➤ YN0028: │ The lockfile would have been modified by this install, which is explicitly forbidden.

I do not understand what would make that happen in the github action I've provided above, which got corrected according to @thedavidprice 's smoke-tests.

Last thing to know that should be brought to @simoncrypta 's attention: I upgraded to yarn 3 by using npx @redwoodjs/codemods@canary upgrade-yarn, which did not fail, but it also didn't fix the engine section of package.json. Not sure what to make of it but I thought it was to be brought to attention, i'd have expected the code mod to have changed that as well.

noire-munich commented 2 years ago

Ok, someone on the internets actually had something similar...

Here it is: https://github.com/yarnpkg/berry/issues/2948

So I additionally followed his steps, I cloned the repo in a new directory, checked out the proper branch, ran yarn... didn't change the yarn.lock.

virtuoushub commented 2 years ago

i'd have expected the code mod to have changed that as well.

I agree. I brought this up with the core team. Will follow back up.

So I additionally followed his steps, I cloned the repo in a new directory, checked out the proper branch, ran yarn... didn't change the yarn.lock.

I don't quite follow. Does that mean by following the steps, you were able to resolve your issue?

thedavidprice commented 2 years ago

Your yarn.lock needs updating and/or is out if sync. Run yarn dedupe locally. Commit those changes.

Do you have a yarn.lock checked in? And updated?

I know I could fix this for you fast. Sorry I don't have time right now :(

noire-munich commented 2 years ago

I know I could fix this for you fast. Sorry I don't have time right now :(

No problem! I know why x). Also it's not critical to us, it would be a clear improvement for sure but the real motivation is to get v1 on time for launch week - just to be proud of being current. ( Also very curious about graphql-yoga )

Here's my latest attempt, I'm using history to trace my latest changes so if there's anything I did wrong it would be obvious.

# from main, which sports a v0.47.

10902  git branch -D preprod
10903  git checkout -b preprod
10904  rm yarn.lock
10905  gst
10906  git commit -m"Remove yarn.lock."
10907  git add . && git commit -m"Remove yarn.lock."
10908  gst
10909  yarn rw upgrade
10910  gst
10911  yarn dedupe
10912  vim package.json
10913  yarn 
10914  git clean -fxd -e .env\nyarn install
10915  yarn install
10916  yarn --help
10917  yarn version latest
10918  npx @redwoodjs/codemods@canary upgrade-yarn
10919  yarn install
10920  gst
10921  git add . && git commit -m"Bump."
10922  yarn dedupe
10923  gst
10924 git push

What does not appear in this is that I had to manually change package.json to include the modifications suggested by @virtuoushub, so it looks like this in the end:

{
  "name": "SportOffice",
  "version": "0.0.1",
  "private": true,
  "workspaces": {
    "packages": [
      "api",
      "web",
      "packages/*"
    ]
  },
  "devDependencies": {
    "@redwoodjs/core": "^0.50.0",
    "eslint-plugin-prettier": "^3.4.0"
  },
  "eslintConfig": {
    "extends": "@redwoodjs/eslint-config",
    "root": true
  },
  "engines": {
    "node": ">=14.17 <=16.x",
    "yarn": ">=3.2.0"
  },
  "dependencies": {
    "@redwoodjs/eslint-config": "^0.50.0",
    "@redwoodjs/record": "^0.50.0",
    "dotenv": "^10.0.0",
    "is_js": "^0.9.0",
    "slug": "^5.1.0"
  },
  "scripts": {
    "testAPI": "yarn rw test api",
    "testWEB": "yarn rw test web",
    "preCI": "rm -rf node_modules",
    "CI": "yarn preCI && yarn install --frozen-lockfile && yarn testAPI"
  },
  "prisma": {
    "seed": "yarn rw exec seed"
  },
  "packageManager": "yarn@3.2.0"
}

so, yep, @virtuoushub , issue is still not solved on my side :-.

virtuoushub commented 2 years ago

What does not appear in this is that I had to manually change package.json to include the modifications suggested by @virtuoushub, so it looks like this in the end:

One thing to note here, AFAIK yarn@3 does not care about the engines prop.

I suggested those edits to package.json to see if we can get the CI/CD to fail faster/louder; so we can see for sure that it is using yarn@1 and not yarn@3 which is my hypothesis.

noire-munich commented 2 years ago

After a debugging session with @virtuoushub , we've hit another roadblock. Seems like we're missing a dependency:

/app/.pnp.cjs:48243
      Error.captureStackTrace(firstError);
            ^

Error: @redwoodjs/cli tried to access @babel/runtime-corejs3, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @babel/runtime-corejs3 (via "@babel/runtime-corejs3/core-js/object/define-property")
Required by: @redwoodjs/cli@npm:1.0.0 (via /app/.yarn/cache/@redwoodjs-cli-npm-1.0.0-953424cca7-6795404bd7.zip/node_modules/@redwoodjs/cli/dist/lib/)

Require stack:
- /app/.yarn/cache/@redwoodjs-cli-npm-1.0.0-953424cca7-6795404bd7.zip/node_modules/@redwoodjs/cli/dist/lib/index.js
- /app/.yarn/cache/@redwoodjs-cli-npm-1.0.0-953424cca7-6795404bd7.zip/node_modules/@redwoodjs/cli/package.json
    at Function.require$$0.Module._resolveFilename (/app/.pnp.cjs:48243:13)
    at Function.require$$0.Module._load (/app/.pnp.cjs:48097:42)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:94:18)
    at Object.<anonymous> (/app/.yarn/cache/@redwoodjs-cli-npm-1.0.0-953424cca7-6795404bd7.zip/node_modules/@redwoodjs/cli/dist/lib/index.js:3:30)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Object.require$$0.Module._extensions..js (/app/.pnp.cjs:48287:33)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.require$$0.Module._load (/app/.pnp.cjs:48127:14)
The command '/bin/sh -c yarn rw build && yarn rw record init' returned a non-zero code: 1

A bit more context though on our progress :

noire-munich commented 2 years ago

Finally \o/

Thanks to @virtuoushub's time and knowledge, we've been able to figure this out.

I've got a Redwood app and at root I have an aws package, configured for yarn v1. In short, there was a conflict while handling both in the github action, because our aws package gets executed in yarn 1 before deployment & then we get the application built in yarn 3. Adding to that, a Redwood JS v1 needs an updated Dockerfile.

So here are the changes we had to make:

# .yarnrc.yml
yarnPath: .yarn/releases/yarn-3.2.0.cjs
nodeLinker: node-modules    # <--- That one matters for our `aws` package.
# Dockerfile
# First we build.
FROM node:16.6.1-alpine as build

WORKDIR /app

COPY package*.json .yarnrc.yml yarn.lock redwood.toml graphql.config.js .env ./ # <--- Adding .yarnrc.yml specific to yarn 3.

COPY .yarn .yarn # <--- Copying yarn 3 cache.

COPY api api

COPY web web

[...]

At some point we've ran into a Docker issue, we needed to add git to the docker image, @virtuoushub I can confirm that this is not necessary.

I'm leaving this opened until I can confirm a successful prod deploy.

@thedavidprice @simoncrypta not sure how such edge case could be used. Maybe some topic in the forums? I've seen DT's similar posts of issues + resolution. Something else?

virtuoushub commented 2 years ago

Quick notes:

The getting yarn@3 squared away in CI/CD learnings are we might want to do a scan for yarn.locks or package.jsons that reside outside of Redwood's standard file structure so we can at least warn framework users that there is probable danger ahead.


Another big take away is nodeLinker was needed to be configured.

nodeLinker: node-modules

this should have happened automatically via npx @redwoodjs/codemods@canary upgrade-yarn; but maybe it was not? or maybe it was not initially checked into version control?

Learnings here might be to better document how drastic of a change yarn@3 is from yarn@1; imho from an end users perspective they should be thought of as separate tools (if anyone remembers AngularJS vs Angular 2+, the distinction between architectural differences in yarn@1 and yarn@2+ is very analogous).

Also maybe have a diagnostic scan that warns if this is not configured?


Also as a learning for the redwood framework as a whole, unless I am mistaken we can not really lean into yarn's pnp architecture until this is resolved on the prisma side: https://github.com/prisma/prisma/issues/1439

thedavidprice commented 2 years ago

Any updates on this? I'd like to take a look and help out otherwise.

Josh-Walker-GM commented 11 months ago

Hey all, I'm going to close this one. Certainly feel free to reopen and we can start discussing again if this is still a problem you are running into.