Waits for the manifests to be written before running the second loops (…

…#5871)

**What's the problem this PR addresses?**

I forgot to add a `Promise.all`, so the `package.json` files could end
up corrupted when the constraints engine had to perform multiple
iterations of the loop.

Fixes #5853

**How did you fix it?**

Adds `Promise.all` before running subsequent iterations.

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

.yarn/versions/2f9e11d3.yml

@@ -0,0 +1,23 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-constraints": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"

packages/acceptance-tests/pkg-tests-core/sources/utils/makeTemporaryEnv.ts

@@ -15,7 +15,7 @@ const mte = generatePkgDriver({
   async runDriver(
     path,
     [command, ...args],
-    {cwd, projectFolder, registryUrl, env, ...config},
+    {cwd, execArgv = [], projectFolder, registryUrl, env, ...config},
   ) {
     const rcEnv: Record<string, any> = {};
     for (const [key, value] of Object.entries(config))
@@ -32,7 +32,7 @@ const mte = generatePkgDriver({
       ? require.resolve(`${__dirname}/../../../../../scripts/run-yarn.js`)
       : require.resolve(`${__dirname}/../../../../yarnpkg-cli/bundles/yarn.js`);
 
-    const res = await execFile(process.execPath, [yarnBinary, ...cwdArgs, command, ...args], {
+    const res = await execFile(process.execPath, [...execArgv, yarnBinary, ...cwdArgs, command, ...args], {
       cwd: cwd || path,
       env: {
         [`HOME`]: nativeHomePath,

packages/acceptance-tests/pkg-tests-specs/sources/commands/constraints/fix.test.js

@@ -1,6 +1,6 @@
-import {xfs}          from '@yarnpkg/fslib';
+import {Filename, ppath, xfs} from '@yarnpkg/fslib';
 
-import {environments} from './environments';
+import {environments}         from './environments';
 
 describe(`Commands`, () => {
   const manifest = {
@@ -18,6 +18,32 @@ describe(`Commands`, () => {
   };
 
   describe(`constraints --fix`, () => {
+    it(`shouldn't crash due to an unending fix loop`, makeTemporaryEnv({
+      foo: ``,
+    }, async ({path, run, source}) => {
+      await run(`install`);
+
+      await xfs.writeFilePromise(ppath.join(path, `yarn.config.cjs`), `
+        exports.constraints = ({Yarn}) => {
+          for (const w of Yarn.workspaces()) {
+            w.set(['foo'], \`\${w.manifest.foo}x\`);
+          }
+        };
+      `);
+
+      await run(`constraints`, `--fix`, {
+        execArgv: [
+          `--require`, require.resolve(`@yarnpkg/monorepo/.pnp.cjs`),
+          `--require`, require.resolve(`@yarnpkg/monorepo/scripts/setup-ts-execution`),
+          `--require`, require.resolve(`@yarnpkg/monorepo/scripts/detect-unsafe-writes.ts`),
+        ],
+      });
+
+      await expect(xfs.readJsonPromise(ppath.join(path, Filename.manifest))).resolves.toMatchObject({
+        foo: `xxxxxxxxxx`,
+      });
+    }));
+
     test(`test apply fix to dependencies`, makeTemporaryEnv(manifest, async ({path, run, source}) => {
       await xfs.writeFilePromise(`${path}/constraints.pro`, `
         gen_enforced_dependency('.', 'is-number', '2.0.0', dependencies).

packages/plugin-constraints/sources/commands/constraints.ts

@@ -83,6 +83,8 @@ export default class ConstraintsCheckCommand extends BaseCommand {
         updates.push(workspace.persistManifest());
       }
 
+      await Promise.all(updates);
+
       if (changedWorkspaces.size > 0 && t > 1)
         continue;
 

scripts/detect-unsafe-writes.ts

@@ -0,0 +1,30 @@
+import {NodeFS, PortablePath, PosixFS, patchFs} from '@yarnpkg/fslib';
+import fs                                       from 'fs';
+
+class SafeCheckFS extends NodeFS {
+  private lockRegistry = new Set<PortablePath>();
+
+  async writeFilePromise(p: PortablePath, ...args: Array<any>) {
+    return await this.lock(p, super.writeFilePromise, args);
+  }
+
+  private async lock(p: PortablePath, fn: (...args: Array<any>) => Promise<any>, args: Array<any>) {
+    if (this.lockRegistry.has(p))
+      throw new Error(`Unsafe write detected: ${p}`);
+
+    this.lockRegistry.add(p);
+    try {
+      return await fn.call(this, p, ...args);
+    } finally {
+      this.lockRegistry.delete(p);
+    }
+  }
+}
+
+// We must copy the fs into a local, because otherwise
+// 1. we would make the NodeFS instance use the function that we patched (infinite loop)
+// 2. Object.create(fs) isn't enough, since it won't prevent the proto from being modified
+const localFs: typeof fs = {...fs};
+const nodeFs = new SafeCheckFS(localFs);
+
+patchFs(fs, new PosixFS(nodeFs));