perf(zip)!: don't access the filesystem from wasm (#5178)

* perf(zip)!: don't access the filesystem from wasm * test: fix `fstat` test * Update 0cd7b07a.yml * Updates the PnP file Co-authored-by: Maël Nison <nison.mael@gmail.com>
yarnpkg · Jan 18, 2023 · 1431217 · 1431217
1 parent c5deaff
commit 1431217
Show file tree

Hide file tree

Showing 12 changed files with 613 additions and 11,260 deletions.
diff --git a/.pnp.cjs b/.pnp.cjs
diff --git a/.yarn/versions/0cd7b07a.yml b/.yarn/versions/0cd7b07a.yml
@@ -0,0 +1,38 @@
+releases:
+  "@yarnpkg/cli": major
+  "@yarnpkg/core": patch
+  "@yarnpkg/libzip": major
+  "@yarnpkg/pnp": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@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"
+  - vscode-zipfs
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/fslib"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,9 @@ Yarn now accepts sponsorships! Please give a look at our [OpenCollective](https:
 - The `pnpDataPath` option has been removed to adhere to our new [PnP specification](https://yarnpkg.com/advanced/pnp-spec). For consistency, all PnP files will now be hardcoded to a single value so that third-party tools can implement the PnP specification without relying on the Yarn configuration.
 - The `ZipFS` and `ZipOpenFS` classes have been moved from `@yarnpkg/fslib` to `@yarnpkg/libzip`. They no longer need or accept the `libzip` parameter.
 - Yarn now assumes that the `fs.lutimes` bindings are always available (which is true for all supported Node versions).
+- Some libzip bindings are no longer needed for `ZipFS` and have been removed:
+  - `open`
+  - `ZIP_CREATE` & `ZIP_TRUNCATE`
 
 ### **API Changes**
 

diff --git a/packages/yarnpkg-core/sources/worker-zip/index.js b/packages/yarnpkg-core/sources/worker-zip/index.js
diff --git a/packages/yarnpkg-fslib/tests/patchedFs.test.ts b/packages/yarnpkg-fslib/tests/patchedFs.test.ts
@@ -175,7 +175,7 @@ describe(`patchedFs`, () => {
 
       expect(statUtils.areStatsEqual(stat, fdStat)).toEqual(true);
     } finally {
-      patchedFs.closeSync(fd);
+      patchedFs.closeSync(zipFd);
     }
   });
 

diff --git a/packages/yarnpkg-libzip/artifacts/build.sh b/packages/yarnpkg-libzip/artifacts/build.sh
@@ -128,7 +128,7 @@ build() {
     -s SUPPORT_BIG_ENDIAN=1 \
     -s TEXTDECODER=2 \
     -s ENVIRONMENT=node \
-    -s NODERAWFS=1 \
+    -s FILESYSTEM=0 \
     -s SINGLE_FILE=1 \
     -s MODULARIZE=1 \
     -s STRICT=1 \

diff --git a/packages/yarnpkg-libzip/artifacts/exported.json b/packages/yarnpkg-libzip/artifacts/exported.json
@@ -22,7 +22,6 @@
     "_zip_get_num_entries",
     "_zip_name_locate",
     "_zip_open_from_source",
-    "_zip_open",
     "_zip_set_file_compression",
     "_zip_stat",
     "_zip_stat_index",

diff --git a/packages/yarnpkg-libzip/sources/ZipFS.ts b/packages/yarnpkg-libzip/sources/ZipFS.ts
@@ -5,7 +5,7 @@ import {NodeFS}
 import {opendir}                                                                                                                                     from '@yarnpkg/fslib';
 import {watchFile, unwatchFile, unwatchAllFiles}                                                                                                     from '@yarnpkg/fslib';
 import {errors, statUtils}                                                                                                                           from '@yarnpkg/fslib';
-import {FSPath, PortablePath, npath, ppath, Filename}                                                                                                from '@yarnpkg/fslib';
+import {FSPath, PortablePath, ppath, Filename}                                                                                                       from '@yarnpkg/fslib';
 import {Libzip}                                                                                                                                      from '@yarnpkg/libzip';
 import {ReadStream, WriteStream, constants}                                                                                                          from 'fs';
 import {PassThrough}                                                                                                                                 from 'stream';
@@ -76,7 +76,7 @@ export class ZipFS extends BasePortableFakeFS {
 
   private readonly stats: Stats;
   private readonly zip: number;
-  private readonly lzSource: number | null = null;
+  private readonly lzSource: number;
   private readonly level: ZipCompression;
 
   private readonly listings: Map<PortablePath, Set<Filename>> = new Map();
@@ -149,26 +149,24 @@ export class ZipFS extends BasePortableFakeFS {
     try {
       let flags = 0;
 
-      if (typeof source === `string` && pathOptions.create)
-        flags |= this.libzip.ZIP_CREATE | this.libzip.ZIP_TRUNCATE;
-
       if (opts.readOnly) {
         flags |= this.libzip.ZIP_RDONLY;
         this.readOnly = true;
       }
 
-      if (typeof source === `string`) {
-        this.zip = this.libzip.open(npath.fromPortablePath(source), flags, errPtr);
-      } else {
-        const lzSource = this.allocateUnattachedSource(source);
+      if (typeof source === `string`)
+        source = pathOptions.create
+          ? makeEmptyArchive()
+          : this.baseFs!.readFileSync(source);
 
-        try {
-          this.zip = this.libzip.openFromSource(lzSource, flags, errPtr);
-          this.lzSource = lzSource;
-        } catch (error) {
-          this.libzip.source.free(lzSource);
-          throw error;
-        }
+      const lzSource = this.allocateUnattachedSource(source);
+
+      try {
+        this.zip = this.libzip.openFromSource(lzSource, flags, errPtr);
+        this.lzSource = lzSource;
+      } catch (error) {
+        this.libzip.source.free(lzSource);
+        throw error;
       }
 
       if (this.zip === 0) {
@@ -241,12 +239,16 @@ export class ZipFS extends BasePortableFakeFS {
     return this.path;
   }
 
+  private prepareClose() {
+    if (!this.ready)
+      throw errors.EBUSY(`archive closed, close`);
+
+    unwatchAllFiles(this);
+  }
+
   getBufferAndClose(): Buffer {
     this.prepareClose();
 
-    if (!this.lzSource)
-      throw new Error(`ZipFS was not created from a Buffer`);
-
     // zip_source_open on an unlink-after-write empty archive fails with "Entry has been deleted"
     if (this.entries.size === 0) {
       this.discardAndClose();
@@ -304,19 +306,18 @@ export class ZipFS extends BasePortableFakeFS {
     }
   }
 
-  private prepareClose() {
-    if (!this.ready)
-      throw errors.EBUSY(`archive closed, close`);
+  discardAndClose() {
+    this.prepareClose();
 
-    unwatchAllFiles(this);
+    this.libzip.discard(this.zip);
+
+    this.ready = false;
   }
 
   saveAndClose() {
     if (!this.path || !this.baseFs)
       throw new Error(`ZipFS cannot be saved and must be discarded when loaded from a buffer`);
 
-    this.prepareClose();
-
     if (this.readOnly) {
       this.discardAndClose();
       return;
@@ -326,27 +327,7 @@ export class ZipFS extends BasePortableFakeFS {
       ? undefined
       : this.stats.mode;
 
-    // zip_close doesn't persist empty archives
-    if (this.entries.size === 0) {
-      this.discardAndClose();
-      this.baseFs.writeFileSync(this.path, makeEmptyArchive(), {mode: newMode});
-    } else {
-      const rc = this.libzip.close(this.zip);
-      if (rc === -1)
-        throw this.makeLibzipError(this.libzip.getError(this.zip));
-
-      if (typeof newMode !== `undefined`) {
-        this.baseFs.chmodSync(this.path, newMode);
-      }
-    }
-
-    this.ready = false;
-  }
-
-  discardAndClose() {
-    this.prepareClose();
-
-    this.libzip.discard(this.zip);
+    this.baseFs.writeFileSync(this.path, this.getBufferAndClose(), {mode: newMode});
 
     this.ready = false;
   }

diff --git a/packages/yarnpkg-libzip/sources/libzipAsync.js b/packages/yarnpkg-libzip/sources/libzipAsync.js
diff --git a/packages/yarnpkg-libzip/sources/libzipSync.js b/packages/yarnpkg-libzip/sources/libzipSync.js
diff --git a/packages/yarnpkg-libzip/sources/makeInterface.ts b/packages/yarnpkg-libzip/sources/makeInterface.ts
@@ -58,9 +58,7 @@ export const makeInterface = (emZip: LibzipEmscriptenModule) => ({
   SEEK_END: 2,
 
   ZIP_CHECKCONS: 4,
-  ZIP_CREATE: 1,
   ZIP_EXCL: 2,
-  ZIP_TRUNCATE: 8,
   ZIP_RDONLY: 16,
 
   ZIP_FL_OVERWRITE: 8192,
@@ -101,7 +99,6 @@ export const makeInterface = (emZip: LibzipEmscriptenModule) => ({
 
   getValue: emZip.getValue,
 
-  open: emZip.cwrap(`zip_open`, `number`, [`string`, `number`, `number`]),
   openFromSource: emZip.cwrap(`zip_open_from_source`, `number`, [`number`, `number`, `number`]),
   close: emZip.cwrap(`zip_close`, `number`, [`number`]),
   discard: emZip.cwrap(`zip_discard`, null, [`number`]),

diff --git a/packages/yarnpkg-pnp/sources/hook.js b/packages/yarnpkg-pnp/sources/hook.js