feat: automatic ZipFS memory management

.yarn/versions/efa6f145.yml

@@ -0,0 +1,39 @@
+releases:
+  "@yarnpkg/cli": major
+  "@yarnpkg/core": major
+  "@yarnpkg/fslib": major
+  "@yarnpkg/plugin-essentials": major
+  "@yarnpkg/plugin-exec": major
+  "@yarnpkg/plugin-file": major
+  "@yarnpkg/plugin-git": major
+  "@yarnpkg/plugin-github": major
+  "@yarnpkg/plugin-http": major
+  "@yarnpkg/plugin-link": major
+  "@yarnpkg/plugin-npm": major
+  "@yarnpkg/plugin-patch": major
+  "@yarnpkg/plugin-pnp": major
+  "@yarnpkg/plugin-pnpm": major
+  "@yarnpkg/pnp": major
+
+declined:
+  - "@yarnpkg/esbuild-plugin-pnp"
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - vscode-zipfs
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"
+  - "@yarnpkg/shell"

CHANGELOG.md

@@ -50,6 +50,20 @@ The following changes only affect people writing Yarn plugins:
 
 - `versionUtils.{fetchBase,fetchRoot,fetchChangedFiles}` have been moved from `@yarnpkg/plugin-version` to `@yarnpkg/plugin-git`. Use `gitUtils.{fetchBase,fetchRoot,fetchChangedFiles}` instead.
 
+- `Cache.prototype.fetchPackageFromCache` now returns an object instead of a tuple, allowing us to change the return type without it being order-dependent.
+
+- `FetchResult` memory management is now entirely automatic, therefore:
+
+  - The underlying `ZipFS` memory is now automatically discarded when the `ZipFS` gets garbage-collected. There's no need to manually call `discardAndClose` anymore.
+
+  - The `releaseFs` method of `FetchResult` has been removed.
+    - If you called this method on a `fetchResult`, you can safely remove the call as the garbage-collector will release the memory associated with the FS once it goes out of scope.
+    - If you implemented this method on a `fetchResult`, you should use [`FinalizationRegistry`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry) to instead request a callback when the FS gets garbage-collected.
+
+  - The return type of `Cache.prototype.fetchPackageFromCache` no longer includes `releaseFs`.
+
+  - `api.holdFetchResult`, the entire `api` argument of `Installer['installPackage']`, and `InstallPackageExtraApi` have been removed.
+
 ### Installs
 
 - The `pnpm` linker avoids creating symlinks that lead to loops on the file system, by moving them higher up in the directory structure.

packages/plugin-essentials/sources/commands/info.ts

@@ -237,12 +237,7 @@ export default class InfoCommand extends BaseCommand {
 
         const fetchResult = await fetcher.fetch(pkg, fetcherOptions);
 
-        let manifest;
-        try {
-          manifest = await Manifest.find(fetchResult.prefixPath, {baseFs: fetchResult.packageFs});
-        } finally {
-          fetchResult.releaseFs?.();
-        }
+        const manifest = await Manifest.find(fetchResult.prefixPath, {baseFs: fetchResult.packageFs});
 
         registerData(`Manifest`, {
           [`License`]: formatUtils.tuple(formatUtils.Type.NO_HINT, manifest.license),

packages/plugin-exec/sources/ExecFetcher.ts

@@ -47,7 +47,7 @@ export class ExecFetcher implements Fetcher {
   async fetch(locator: Locator, opts: FetchOptions) {
     const expectedChecksum = opts.checksums.get(locator.locatorHash) || null;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator),
       loader: () => this.fetchFromDisk(locator, opts),
@@ -56,7 +56,6 @@ export class ExecFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       localPath: this.getLocalPath(locator, opts),
       checksum,

packages/plugin-exec/sources/ExecResolver.ts

@@ -1,7 +1,7 @@
 import {Resolver, ResolveOptions, MinimalResolveOptions} from '@yarnpkg/core';
 import {Descriptor, Locator, Manifest, Package}          from '@yarnpkg/core';
 import {LinkType}                                        from '@yarnpkg/core';
-import {miscUtils, structUtils, hashUtils}               from '@yarnpkg/core';
+import {structUtils, hashUtils}                          from '@yarnpkg/core';
 
 import {PROTOCOL}                                        from './constants';
 import * as execUtils                                    from './execUtils';
@@ -75,9 +75,7 @@ export class ExecResolver implements Resolver {
 
     const packageFetch = await opts.fetchOptions.fetcher.fetch(locator, opts.fetchOptions);
 
-    const manifest = await miscUtils.releaseAfterUseAsync(async () => {
-      return await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
-    }, packageFetch.releaseFs);
+    const manifest = await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
 
     return {
       ...locator,

packages/plugin-exec/sources/execUtils.ts

@@ -53,10 +53,6 @@ export async function loadGeneratorFile(range: string, protocol: string, opts: F
     ? {packageFs: new CwdFS(PortablePath.root), prefixPath: ppath.relative(PortablePath.root, parentFetch.localPath)}
     : parentFetch;
 
-  // Discard the parent fs unless we really need it to access the files
-  if (parentFetch !== effectiveParentFetch && parentFetch.releaseFs)
-    parentFetch.releaseFs();
-
   const generatorFs = effectiveParentFetch.packageFs;
   const generatorPath = ppath.join(effectiveParentFetch.prefixPath, path);
 

packages/plugin-file/sources/FileFetcher.ts

@@ -29,7 +29,7 @@ export class FileFetcher implements Fetcher {
   async fetch(locator: Locator, opts: FetchOptions) {
     const expectedChecksum = opts.checksums.get(locator.locatorHash) || null;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator, `${structUtils.prettyLocator(opts.project.configuration, locator)} can't be found in the cache and will be fetched from the disk`),
       loader: () => this.fetchFromDisk(locator, opts),
@@ -38,7 +38,6 @@ export class FileFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       localPath: this.getLocalPath(locator, opts),
       checksum,

packages/plugin-file/sources/FileResolver.ts

@@ -1,4 +1,4 @@
-import {miscUtils, structUtils, hashUtils, Package}      from '@yarnpkg/core';
+import {structUtils, hashUtils, Package}                 from '@yarnpkg/core';
 import {LinkType}                                        from '@yarnpkg/core';
 import {Descriptor, Locator, Manifest}                   from '@yarnpkg/core';
 import {Resolver, ResolveOptions, MinimalResolveOptions} from '@yarnpkg/core';
@@ -87,9 +87,7 @@ export class FileResolver implements Resolver {
 
     const packageFetch = await opts.fetchOptions.fetcher.fetch(locator, opts.fetchOptions);
 
-    const manifest = await miscUtils.releaseAfterUseAsync(async () => {
-      return await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
-    }, packageFetch.releaseFs);
+    const manifest = await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
 
     return {
       ...locator,

packages/plugin-file/sources/TarballFileFetcher.ts

@@ -1,6 +1,6 @@
 import {Fetcher, FetchOptions, FetchResult, MinimalFetchOptions} from '@yarnpkg/core';
 import {Locator}                                                 from '@yarnpkg/core';
-import {miscUtils, structUtils, tgzUtils}                        from '@yarnpkg/core';
+import {structUtils, tgzUtils}                                   from '@yarnpkg/core';
 import {PortablePath, ppath, CwdFS}                              from '@yarnpkg/fslib';
 
 import {TARBALL_REGEXP, PROTOCOL}                                from './constants';
@@ -23,7 +23,7 @@ export class TarballFileFetcher implements Fetcher {
   async fetch(locator: Locator, opts: FetchOptions) {
     const expectedChecksum = opts.checksums.get(locator.locatorHash) || null;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator, `${structUtils.prettyLocator(opts.project.configuration, locator)} can't be found in the cache and will be fetched from the disk`),
       loader: () => this.fetchFromDisk(locator, opts),
@@ -32,7 +32,6 @@ export class TarballFileFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       checksum,
     };
@@ -53,20 +52,14 @@ export class TarballFileFetcher implements Fetcher {
       ? {packageFs: new CwdFS(PortablePath.root), prefixPath: ppath.relative(PortablePath.root, parentFetch.localPath)}
       : parentFetch;
 
-    // Discard the parent fs unless we really need it to access the files
-    if (parentFetch !== effectiveParentFetch && parentFetch.releaseFs)
-      parentFetch.releaseFs();
-
     const sourceFs = effectiveParentFetch.packageFs;
     const sourcePath = ppath.join(effectiveParentFetch.prefixPath, path);
     const sourceBuffer = await sourceFs.readFilePromise(sourcePath);
 
-    return await miscUtils.releaseAfterUseAsync(async () => {
-      return await tgzUtils.convertToZip(sourceBuffer, {
-        compressionLevel: opts.project.configuration.get(`compressionLevel`),
-        prefixPath: structUtils.getIdentVendorPath(locator),
-        stripComponents: 1,
-      });
-    }, effectiveParentFetch.releaseFs);
+    return await tgzUtils.convertToZip(sourceBuffer, {
+      compressionLevel: opts.project.configuration.get(`compressionLevel`),
+      prefixPath: structUtils.getIdentVendorPath(locator),
+      stripComponents: 1,
+    });
   }
 }

packages/plugin-file/sources/TarballFileResolver.ts

@@ -1,7 +1,7 @@
 import {Resolver, ResolveOptions, MinimalResolveOptions, Package} from '@yarnpkg/core';
 import {Descriptor, Locator, Manifest}                            from '@yarnpkg/core';
 import {LinkType}                                                 from '@yarnpkg/core';
-import {miscUtils, structUtils}                                   from '@yarnpkg/core';
+import {structUtils}                                              from '@yarnpkg/core';
 import {npath}                                                    from '@yarnpkg/fslib';
 
 import {FILE_REGEXP, TARBALL_REGEXP, PROTOCOL}                    from './constants';
@@ -70,9 +70,7 @@ export class TarballFileResolver implements Resolver {
 
     const packageFetch = await opts.fetchOptions.fetcher.fetch(locator, opts.fetchOptions);
 
-    const manifest = await miscUtils.releaseAfterUseAsync(async () => {
-      return await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
-    }, packageFetch.releaseFs);
+    const manifest = await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
 
     return {
       ...locator,

packages/plugin-file/sources/fileUtils.ts

@@ -1,5 +1,5 @@
-import {structUtils, FetchOptions, Locator, miscUtils, tgzUtils, Ident, FetchResult} from '@yarnpkg/core';
-import {ppath, PortablePath, npath, CwdFS, ZipFS}                                    from '@yarnpkg/fslib';
+import {structUtils, FetchOptions, Locator, tgzUtils, Ident, FetchResult} from '@yarnpkg/core';
+import {ppath, PortablePath, npath, CwdFS, ZipFS}                         from '@yarnpkg/fslib';
 
 export function parseSpec(spec: string) {
   const {params, selector} = structUtils.parseRange(spec);
@@ -52,21 +52,15 @@ export async function makeArchiveFromLocator(locator: Locator, {protocol, fetchO
     ? {packageFs: new CwdFS(PortablePath.root), prefixPath: ppath.relative(PortablePath.root, parentFetch.localPath)}
     : parentFetch;
 
-  // Discard the parent fs unless we really need it to access the files
-  if (parentFetch !== effectiveParentFetch && parentFetch.releaseFs)
-    parentFetch.releaseFs();
-
   const sourceFs = effectiveParentFetch.packageFs;
   const sourcePath = ppath.join(effectiveParentFetch.prefixPath, path);
 
-  return await miscUtils.releaseAfterUseAsync(async () => {
-    return await tgzUtils.makeArchiveFromDirectory(sourcePath, {
-      baseFs: sourceFs,
-      prefixPath: structUtils.getIdentVendorPath(locator),
-      compressionLevel: fetchOptions.project.configuration.get(`compressionLevel`),
-      inMemory,
-    });
-  }, effectiveParentFetch.releaseFs);
+  return await tgzUtils.makeArchiveFromDirectory(sourcePath, {
+    baseFs: sourceFs,
+    prefixPath: structUtils.getIdentVendorPath(locator),
+    compressionLevel: fetchOptions.project.configuration.get(`compressionLevel`),
+    inMemory,
+  });
 }
 
 export async function makeBufferFromLocator(locator: Locator, {protocol, fetchOptions}: {protocol: string, fetchOptions: FetchOptions}) {

packages/plugin-git/sources/GitFetcher.ts

@@ -1,6 +1,6 @@
 import {Fetcher, FetchOptions, MinimalFetchOptions, FetchResult} from '@yarnpkg/core';
 import {Locator}                                                 from '@yarnpkg/core';
-import {miscUtils, scriptUtils, structUtils, tgzUtils}           from '@yarnpkg/core';
+import {scriptUtils, structUtils, tgzUtils}                      from '@yarnpkg/core';
 import {PortablePath, ppath, xfs}                                from '@yarnpkg/fslib';
 
 import * as gitUtils                                             from './gitUtils';
@@ -27,7 +27,7 @@ export class GitFetcher implements Fetcher {
     if (result !== null)
       return result;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator, `${structUtils.prettyLocator(opts.project.configuration, locator)} can't be found in the cache and will be fetched from the remote repository`),
       loader: () => this.cloneFromRemote(normalizedLocator, nextOpts),
@@ -36,7 +36,6 @@ export class GitFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       checksum,
     };
@@ -63,12 +62,10 @@ export class GitFetcher implements Fetcher {
 
     const sourceBuffer = await xfs.readFilePromise(packagePath);
 
-    return await miscUtils.releaseAfterUseAsync(async () => {
-      return await tgzUtils.convertToZip(sourceBuffer, {
-        compressionLevel: opts.project.configuration.get(`compressionLevel`),
-        prefixPath: structUtils.getIdentVendorPath(locator),
-        stripComponents: 1,
-      });
+    return await tgzUtils.convertToZip(sourceBuffer, {
+      compressionLevel: opts.project.configuration.get(`compressionLevel`),
+      prefixPath: structUtils.getIdentVendorPath(locator),
+      stripComponents: 1,
     });
   }
 }

packages/plugin-git/sources/GitResolver.ts

@@ -1,5 +1,5 @@
 import {Resolver, ResolveOptions, MinimalResolveOptions, Package} from '@yarnpkg/core';
-import {miscUtils, structUtils}                                   from '@yarnpkg/core';
+import {structUtils}                                              from '@yarnpkg/core';
 import {LinkType}                                                 from '@yarnpkg/core';
 import {Descriptor, Locator, Manifest}                            from '@yarnpkg/core';
 
@@ -66,9 +66,7 @@ export class GitResolver implements Resolver {
 
     const packageFetch = await opts.fetchOptions.fetcher.fetch(locator, opts.fetchOptions);
 
-    const manifest = await miscUtils.releaseAfterUseAsync(async () => {
-      return await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
-    }, packageFetch.releaseFs);
+    const manifest = await Manifest.find(packageFetch.prefixPath, {baseFs: packageFetch.packageFs});
 
     return {
       ...locator,

packages/plugin-github/sources/GithubFetcher.ts

@@ -21,7 +21,7 @@ export class GithubFetcher implements Fetcher {
   async fetch(locator: Locator, opts: FetchOptions) {
     const expectedChecksum = opts.checksums.get(locator.locatorHash) || null;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator, `${structUtils.prettyLocator(opts.project.configuration, locator)} can't be found in the cache and will be fetched from GitHub`),
       loader: () => this.fetchFromNetwork(locator, opts),
@@ -30,7 +30,6 @@ export class GithubFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       checksum,
     };

packages/plugin-http/sources/TarballHttpFetcher.ts

@@ -22,7 +22,7 @@ export class TarballHttpFetcher implements Fetcher {
   async fetch(locator: Locator, opts: FetchOptions) {
     const expectedChecksum = opts.checksums.get(locator.locatorHash) || null;
 
-    const [packageFs, releaseFs, checksum] = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
+    const {packageFs, checksum} = await opts.cache.fetchPackageFromCache(locator, expectedChecksum, {
       onHit: () => opts.report.reportCacheHit(locator),
       onMiss: () => opts.report.reportCacheMiss(locator, `${structUtils.prettyLocator(opts.project.configuration, locator)} can't be found in the cache and will be fetched from the remote server`),
       loader: () => this.fetchFromNetwork(locator, opts),
@@ -31,7 +31,6 @@ export class TarballHttpFetcher implements Fetcher {
 
     return {
       packageFs,
-      releaseFs,
       prefixPath: structUtils.getIdentVendorPath(locator),
       checksum,
     };