Conversation
node-fcntl is now published to npm (scality/node-fcntl#23), so there is no reason to keep pulling it from GitHub. A registry tarball is immutable and carries an integrity hash, where the codeload tarball carried none; nothing else changes, as it is the same code under a scoped name. Only the import path differs, in lib/storage/data/file/utils.js. Yarn 1 resolves it identically -- the addon builds and loads as before. Issue: ARSN-650
Yarn 1 is unmaintained and its installs have become unreliable; this moves Arsenal to Yarn 4.18.0. Yarn is pinned through the packageManager field, with its integrity hash, rather than by committing a release binary: CI enables Corepack explicitly, and anyone without it gets a clear error naming the version to use rather than a silently different Yarn. nodeLinker is node-modules rather than the default PnP, which cannot load this package's native addons from inside a zip. It is also what scality/bench-vault chose. approvedGitRepositories is required, not optional hardening: Yarn defaults to an empty allowlist that blocks every Git fetch, so a package with Git dependencies cannot install without it. Scoped to the Scality org rather than the "**" Yarn writes during migration. npmPreapprovedPackages exempts @scality/* from the one-day quarantine Yarn applies to newly published versions. That guard is aimed at compromised third-party publishes; our own packages come from our CI, so waiting a day to consume them buys nothing. Everything else keeps it. networkConcurrency: 1 replaces Yarn 1's --network-concurrency flag, which Berry rejects on the CLI but still honours as a setting. Git dependencies that still carry Yarn 1 lockfiles are bootstrapped with Yarn Classic, and concurrent Classic installs race on its shared cache. It can be dropped once those dependencies are themselves on Berry. Note that Yarn rewrites .yarnrc.yml during the migration step itself, dropping comments and re-adding permissive security defaults; the file here is the reviewed version, and it stays put on subsequent installs. Issue: ARSN-650
eslint.config.mjs imports @eslint/js and @eslint/eslintrc, neither of which is declared. Both resolve today only because ESLint pulls them in and the node-modules linker hoists them to the top of the tree, so any stricter resolver breaks linting outright. They are pinned to the versions already resolved, so this is a no-op at install time. Issue: ARSN-650
Berry disables install/build scripts by default, where Yarn 1 let all ~1100 packages run them. Four genuinely need to compile: @scality/fcntl and ioctl are node-gyp addons, leveldown ships native code, and mongodb-memory-server downloads a server binary. The key must be the real package name. Keying it on an alias makes Berry silently skip the build (YN0004) -- the install still succeeds and the addon is simply never compiled. This is the clearest security improvement in the migration: install-time code execution goes from every dependency to four named ones. Issue: ARSN-650
Berry runs scripts through its own portable shell, which has no `export` builtin: both scripts failed immediately with "command not found: export". An inline environment prefix is equivalent and also valid in bash, so this works under either package manager. Append to NODE_OPTIONS rather than replacing it, so that a value set by the caller survives; Yarn passes `--require .pnp.cjs` through this variable under the PnP linker, and overwriting it leaves every script unable to resolve its own dependencies. The `:-` default is required because that same shell treats an unset variable as an error rather than as empty, and CI does not set NODE_OPTIONS. Issue: ARSN-650
Setting packageManager flips Yarn's Git-dependency bootstrap from the Yarn Classic branch to the Berry branch. Classic ran `yarn install`, which executes prepare; Berry runs `yarn pack --install-if-needed`, which executes prepack and not prepare. Without prepack, that bootstrap produces a 3-entry, 6969-byte tarball containing only LICENSE, README and package.json -- no build/ at all -- so every Berry consumer would install an empty Arsenal. Verified on a pristine `git archive HEAD` checkout; with prepack the same command produces 701 entries including the full build/ tree. Cloudserver already pins Yarn 4, so it is such a consumer today. Issue: ARSN-650
- corepack enable has to run before actions/setup-node. Its `cache: yarn` probe shells out to Yarn to locate the cache folder, and `yarn cache dir` exits 1 once packageManager pins Yarn 4 (actions/setup-node#1027). - --frozen-lockfile becomes --immutable, and --network-concurrency is dropped: Berry rejects it on the CLI and it now lives in .yarnrc.yml. - Berry removed the global --silent flag, so `yarn --silent X` becomes `yarn X`; Berry is quiet by default. - The tests/workflows job keeps Yarn 1 and therefore drops `cache: yarn`, since setup-node probes from the repo root. It also uses working-directory rather than `yarn --cwd`: Corepack resolves packageManager from the process CWD, so --cwd would pick up the root's Yarn 4 and ignore the nested pin. - Yarn 1 built the package as a side effect of install, via the root prepare script; Berry does not run prepare on install. Some tests spawn a process that requires build/, so it is now built explicitly. - release.spec.ts mocks workflow steps by name, so the new Corepack step needs an entry or the suite really executes it inside the act image. Issue: ARSN-650
This is a separate install root with its own lockfile, so it becomes its own Berry project rather than a workspace: keeping its dependency tree isolated from the root avoids any hoisting interaction, and Berry inherits the root .yarnrc.yml anyway, so only the linker is restated here. @kie/act-js and unrs-resolver need to compile, so they are allowlisted via dependenciesMeta; build scripts are otherwise off by default. CI enables Corepack for this job and uses --immutable in place of --frozen-lockfile. Verified locally on Node 24 against a live container socket: 7/7 suites pass, including the release workflow tests that really execute the new Corepack step through act. Issue: ARSN-650
This directory had a package.json but no lockfile, so Berry treated it
as part of the root project and refused to install ("doesn't seem to be
part of the project declared in ..."). It now carries its own lockfile
and is a separate Berry project, which also means CI stops re-resolving
its dependency ranges on every run.
Since the versions were being pinned for the first time, the 2018-era
ones are refreshed rather than frozen in place:
node-forge ^0.7.1 -> ^1.3.1 (matches the root)
async ~2.6.1 -> ~2.6.4 (matches the root)
mocha 5.2.0 -> ^12.0.2 (and moved to devDependencies)
The code only uses pki.privateKeyFromPem, util.decode64, md.sha256 and
async.waterfall, all unchanged across those majors. All 3 tests pass,
including the decryption path that exercises node-forge.
Issue: ARSN-650
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.6 #2711 +/- ##
===================================================
+ Coverage 74.76% 74.77% +0.01%
===================================================
Files 227 227
Lines 18650 18650
Branches 3864 3864
===================================================
+ Hits 13943 13945 +2
+ Misses 4702 4700 -2
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ConflictThere is a conflict between your branch Please resolve the conflict on the feature branch ( git fetch && \
git checkout origin/improvement/ARSN-650/migrate-to-yarn-4-berry && \
git merge origin/development/8.6Resolve merge conflicts and commit git push origin HEAD:improvement/ARSN-650/migrate-to-yarn-4-berry |
francoisferrand
left a comment
There was a problem hiding this comment.
same as cloudserver PR: please delay for a few days at least, so we take the time to access impact and share/organize rollout:
- there are many dependency bumps / cleanups in progress : so really duplicated work to do this at the same time, and try to fix these problems
- impact on developers is not trivial: enabling corepack is required and global, so it will likely require combined deployment accross all repos. Could be its own project, even with yarn v1, btw → need communication & coordination across repos
- if we switch but don't use the features of yarn (pnp), why stick to yarn? No saying I am against it, but worth evaluating if yarn is still needed (and maybe we still want to keep it anyway, for muscle memory 🤷♂️)
Moves Arsenal from Yarn 1 to Yarn 4.18.0 with the
node-moduleslinker, including the two nested install roots (tests/workflowsandlib/executables/pensieveCreds).Outside the lockfiles the change is small: four CI workflows,
.yarnrc.yml,.gitignore, and the threepackage.jsonfiles. No application code changes.Design decisions
node-modules, not PnP. PnP resolves only what a package declares, and the AWS SDK does not declare what it requires:@aws-sdk/lib-storageimports@smithy/typesfrom its published type definitions while listing it only underdevDependencies, soyarn buildfails outright. It is a recurring shape rather than a one-off — aws-sdk-js-v3#3626 is the same package missing a different dependency, and #5035, #5362, #4972 and smithy-typescript#1492 are the same bug elsewhere in the SDK.ts-jestadds a variant of its own, requiringjest-utilunconditionally while declaring it an optional peer.Both are forceable with
packageExtensions— two entries get Arsenal green on build, all 4433 tests and lint — but that means carrying patches for other people's manifests, with no way to know the list is complete short of exercising every code path. Cloudserver shows where that ends:@scality/cloudserverclient@1.0.12declares 4 dependencies against far more requires, and 28 AWS/Smithy packages added by hand there still had not converged.node-moduleshoists, so none of this arises, and it keeps the change a pure package-manager swap.Corepack and
packageManager, not a committed release binary. The version is pinned by thepackageManagerfield with its integrity hash, so there is no 4 MB blob in the tree and noyarnPath. The cost is that contributors runcorepack enableonce; CI does it explicitly in every job.approvedGitRepositoriesis required, not hardening. Yarn's default is an empty allowlist that blocks every Git fetch, so a package with Git dependencies cannot install at all without it. Scoped to the Scality org rather than the**Yarn writes during migration.networkConcurrency: 1replaces Yarn 1's--network-concurrency, which Berry rejects on the CLI but still honours as a setting. Git dependencies that still carry Yarn 1 lockfiles are bootstrapped by Yarn Classic, and concurrent Classic installs race on its shared cache. Droppable once those dependencies are on Berry.Build scripts stay off by default. Install-time code execution drops from every dependency to four, allowlisted individually in
dependenciesMeta.npmMinimalAgeGateis left at Yarn's 1-day default, with@scality/*preapproved so our own releases are usable immediately.prepackkeeps Git consumers working. Berry does not run the rootpreparescript on install, sobuild/is no longer a side effect ofyarn install;prepackproduces it for consumers installing from Git, and CI builds explicitly. Consumers are otherwise unaffected — Yarn 1 never reads a Git dependency's lockfile, it resolves Arsenal's tree into its own, and Berry consumers do the same.Issue: ARSN-650