immuni-app / immuni-app-android

Official repository for the Android version of the immuni application
GNU Affero General Public License v3.0
858 stars 145 forks source link

Yarn v3 #377

Closed tiziodcaio closed 2 years ago

tiziodcaio commented 2 years ago

I warn about you MUST enable corepack

Have done after #370

Description

This PR tackles:

Checklist

Feature added

ImmuniDGR commented 2 years ago

🚔 Safety Check 🚔

🔰 Result 🔰

⚠️ Some configuration files don't match the development branch. If you did not perform these changes, please rebase on the development branch.

🛠 Diagnostic information 🛠

valerio-castelli commented 2 years ago

This change has broken the CI checks due to the inability to overwrite an already installed version of corepack @tiziodcaio. See this failed CI job for reference. This is a little odd, though, since CircleCI's documentation about the cimg/android:2021.10.2-node Docker image states that it only includes Node 14.x. This is likely due to yarn already being installed in the image.

I'll update the CI to run npm install -g corepack with --force, but I'd ask you to double check that explicitly installing corepack is actually necessary at all.

tiziodcaio commented 2 years ago

I explain what corepack does Yarn from v>1 decided to don't update the npm yarn, but it downloads directly into the .yarn folder a huge js file. In previous PRs someone has (correctly) not trusted my PR with this huge js file (which in fact was only downloaded using the command yarn set version berry [codename for yarn v2/3]). So after that I began to suggest the use of corepack, a package developed from the nodejs foundation that helps to override problems with the node pakage managers. Pratically corepack downloads itself the yarn command from the repository, so I don't have to add the js yarn script to the PR. If you have seen the package.json has got an add that tells to corepack wich package manager to use. To update or downgrade yarn simply change that version.

valerio-castelli commented 2 years ago

Thanks for the explanation, @tiziodcaio! I was just pointing out that using that syntax is causing the CI pipeline to fail pretty badly, due to npm refusing to overwrite the default yarn installation 🙂

#!/bin/bash -eo pipefail
sudo npm i -g corepack; sudo corepack enable
[..................] / rollbackFailedOptional: verb npm-session 0a874f57ff2f500[[K..................] / rollbackFailedOptional: verb npm-session 0a874f57ff2f500[[K..................] / rollbackFailedOptional: verb npm-session 0a874f57ff2f500[[K..................] - rollbackFailedOptional: verb npm-session 0a874f57ff2f500[[K       ...........] | extract:corepack: verb lock using /root/.npm/_locks/stag/usr/local/bin/corepack -> /usr/local/lib/node_modules/corepack/dist/corepack.js
/usr/local/bin/pnpm -> /usr/local/lib/node_modules/corepack/dist/pnpm.js
/usr/local/bin/pnpx -> /usr/local/lib/node_modules/corepack/dist/pnpx.js
npm ERR! code EEXIST
npm ERR! syscall symlink
npm ERR! path ../lib/node_modules/corepack/dist/yarn.js
npm ERR! dest /usr/local/bin/yarn
npm ERR! errno -17
npm ERR! EEXIST: file already exists, symlink '../lib/node_modules/corepack/dist/yarn.js' -> '/usr/local/bin/yarn'
npm ERR! File exists: /usr/local/bin/yarn
npm ERR! Remove the existing file and try again, or run npm
npm ERR! with --force to overwrite files recklessly.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2022-01-06T16_08_18_122Z-debug.log

Exited with code exit status 239

I'm waiting the next execution of the periodic CI job (which should happen in about one hour) to verify whether using --force is enough to restore the pipeline.

tiziodcaio commented 2 years ago

Maybe we can try removing the yarn default package and then adding corepack

valerio-castelli commented 2 years ago

In principle yes, we could, but it seems a bit more complex than necessary. Anyway, I've just verified that using the --force flag allows the CI to execute danger successfully, so I'd consider the problem resolved! 👍

tiziodcaio commented 2 years ago

Super!

astagi commented 2 years ago

Thank you @valerio-castelli @tiziodcaio