Draft: Refactor github and tarball fetching and resolution.

.yarn/versions/48fd3c19.yml

@@ -0,0 +1,25 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-git": minor
+  "@yarnpkg/plugin-github": minor
+  "@yarnpkg/plugin-http": minor
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-node-modules"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"

packages/plugin-git/sources/gitUtils.test.js

@@ -1,21 +1,10 @@
 import * as gitUtils from './gitUtils';
 
 const VALID_PATTERNS = [
-  `GitHubOrg/foo-bar.js`,
-  `GitHubOrg/foo2bar.js`,
-  `GitHubOrg/foo-bar.js#hash`,
-  `GitHubOrg/foo-bar.js#commit:hash`,
-  `GitHubOrg/foo-bar.js#commit=hash`,
-  `GitHubOrg/foo-bar.js#commit=hash&workspace=foo`,
-  `GitHubOrg/foo-bar.js#tag=hello`,
-  `GitHubOrg/foo-bar.js#workspace=foo`,
-  `github:GitHubOrg/foo-bar.js`,
-  `github:GitHubOrg/foo-bar.js#hash`,
   `https://github.com/TooTallNate/util-deprecate.git#v1.0.0`,
   `https://github.com/TooTallNate/util-deprecate.git#semver:v1.0.0`,
   `https://github.com/TooTallNate/util-deprecate.git#master`,
   `https://github.com/TooTallNate/util-deprecate.git#b3562c2798507869edb767da869cd7b85487726d`,
-  `https://github.com/TooTallNate/util-deprecate/tarball/b3562c2798507869edb767da869cd7b85487726d`,
   `git://github.com/TooTallNate/util-deprecate.git#v1.0.1`,
   `git+ssh://git@github.com/TooTallNate/util-deprecate.git#v1.0.1`,
   `ssh://git@github.com/TooTallNate/util-deprecate.git#v1.0.1`,

packages/plugin-git/sources/gitUtils.ts

@@ -21,8 +21,14 @@ const gitPatterns = [
 
   /^git@[^#]+\/[^#]+\.git(?:#.*)?$/,
 
+  /**
+   * @deprecated
+   */
   /^(?:github:|https:\/\/github\.com\/)?(?!\.{1,2}\/)([a-zA-Z._0-9-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z._0-9-]+?)(?:\.git)?(?:#.*)?$/,
   // GitHub `/tarball/` URLs
+  /**
+   * @deprecated
+   */
   /^https:\/\/github\.com\/(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)\/tarball\/(.+)?$/,
 ];
 
@@ -132,12 +138,6 @@ export function normalizeRepoUrl(url: string, {git = false}: {git?: boolean} = {
   // disambiguate that this URL points to a Git repository.
   url = url.replace(/^git\+https:/, `https:`);
 
-  // We support this as an alias to GitHub repositories
-  url = url.replace(/^(?:github:|https:\/\/github\.com\/)?(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)(?:\.git)?(#.*)?$/, `https://github.com/$1/$2.git$3`);
-
-  // We support GitHub `/tarball/` URLs
-  url = url.replace(/^https:\/\/github\.com\/(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)\/tarball\/(.+)?$/, `https://github.com/$1/$2.git#$3`);
-
   if (git) {
     // The `git+` prefix doesn't mean anything at all for Git
     url = url.replace(/^git\+([^:]+):/, `$1:`);

packages/plugin-git/sources/index.ts

@@ -10,6 +10,12 @@ export interface Hooks {
     locator: Locator,
     opts: FetchOptions,
   ) => Promise<FetchResult | null>,
+  /**
+   * Hook to be able to exclude patterns from being picked up by this plugin.
+   *
+   * @param addGitHostedRepository call this function to add a pattern to be excluded
+   */
+  addHandledHostedRepository?: (addHandledHostedRepository: (regExp: RegExp) => void) => Promise<void>,
 }
 
 declare module '@yarnpkg/core' {

packages/plugin-github/package.json

@@ -9,7 +9,8 @@
   },
   "peerDependencies": {
     "@yarnpkg/core": "^2.3.1",
-    "@yarnpkg/plugin-git": "^2.2.0"
+    "@yarnpkg/plugin-git": "^2.2.0",
+    "@yarnpkg/plugin-http": "^2.1.1"
   },
   "devDependencies": {
     "@yarnpkg/core": "workspace:^2.3.1",

packages/plugin-github/sources/GithubFetcher.ts

@@ -1,10 +1,6 @@
-import {Fetcher, FetchOptions, MinimalFetchOptions}    from '@yarnpkg/core';
-import {Locator}                                       from '@yarnpkg/core';
-import {httpUtils, scriptUtils, structUtils, tgzUtils} from '@yarnpkg/core';
-import {PortablePath, CwdFS, ppath, xfs}               from '@yarnpkg/fslib';
-import {gitUtils}                                      from '@yarnpkg/plugin-git';
+import {Fetcher, FetchOptions, MinimalFetchOptions, Locator} from '@yarnpkg/core';
 
-import * as githubUtils                                from './githubUtils';
+import * as githubUtils                                      from './githubUtils';
 
 export class GithubFetcher implements Fetcher {
   supports(locator: Locator, opts: MinimalFetchOptions) {
@@ -19,57 +15,6 @@ 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, {
-      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),
-      skipIntegrityCheck: opts.skipIntegrityCheck,
-    });
-
-    return {
-      packageFs,
-      releaseFs,
-      prefixPath: structUtils.getIdentVendorPath(locator),
-      checksum,
-    };
-  }
-
-  async fetchFromNetwork(locator: Locator, opts: FetchOptions) {
-    const sourceBuffer = await httpUtils.get(this.getLocatorUrl(locator, opts), {
-      configuration: opts.project.configuration,
-    });
-
-    return await xfs.mktempPromise(async extractPath => {
-      const extractTarget = new CwdFS(extractPath);
-
-      await tgzUtils.extractArchiveTo(sourceBuffer, extractTarget, {
-        stripComponents: 1,
-      });
-
-      const repoUrlParts = gitUtils.splitRepoUrl(locator.reference);
-      const packagePath = ppath.join(extractPath, `package.tgz` as PortablePath);
-
-      await scriptUtils.prepareExternalProject(extractPath, packagePath, {
-        configuration: opts.project.configuration,
-        report: opts.report,
-        workspace: repoUrlParts.extra.workspace,
-      });
-
-      const packedBuffer = await xfs.readFilePromise(packagePath);
-
-      return await tgzUtils.convertToZip(packedBuffer, {
-        compressionLevel: opts.project.configuration.get(`compressionLevel`),
-        prefixPath: structUtils.getIdentVendorPath(locator),
-        stripComponents: 1,
-      });
-    });
-  }
-
-  private getLocatorUrl(locator: Locator, opts: MinimalFetchOptions) {
-    const {auth, username, reponame, treeish} = githubUtils.parseGithubUrl(locator.reference);
-
-    return `https://${auth ? `${auth}@` : ``}github.com/${username}/${reponame}/archive/${treeish}.tar.gz`;
+    return opts.fetcher.fetch(githubUtils.makeLocator(locator), opts);
   }
 }

packages/plugin-github/sources/GithubResolver.ts

@@ -0,0 +1,41 @@
+import {Resolver, ResolveOptions, MinimalResolveOptions, DescriptorHash, structUtils, Descriptor, Locator, Package} from '@yarnpkg/core';
+
+import * as githubUtils                                                                                             from "./githubUtils";
+
+export class GithubResolver implements Resolver {
+  supportsDescriptor(descriptor: Descriptor, opts: MinimalResolveOptions): boolean {
+    return githubUtils.isGithubUrl(descriptor.range);
+  }
+
+  supportsLocator(locator: Locator, opts: MinimalResolveOptions): boolean {
+    return githubUtils.isGithubUrl(locator.reference);
+  }
+
+  shouldPersistResolution(locator: Locator, opts: MinimalResolveOptions): boolean {
+    return true;
+  }
+
+  bindDescriptor(descriptor: Descriptor, fromLocator: Locator, opts: MinimalResolveOptions): Descriptor {
+    return descriptor;
+  }
+
+  getResolutionDependencies(descriptor: Descriptor, opts: MinimalResolveOptions): Array<Descriptor> {
+    return [githubUtils.makeDescriptor(descriptor)];
+  }
+
+  async getCandidates(descriptor: Descriptor, dependencies: Map<DescriptorHash, Package>, opts: ResolveOptions): Promise<Array<Locator>> {
+    return [githubUtils.makeLocator(structUtils.convertDescriptorToLocator(descriptor))];
+  }
+
+  async getSatisfying(descriptor: Descriptor, references: Array<string>, opts: ResolveOptions): Promise<Array<Locator> | null> {
+    return null;
+  }
+
+  async resolve(locator: Locator, opts: ResolveOptions): Promise<Package> {
+    const sourceLocator = githubUtils.makeLocator(locator);
+
+    const sourcePkg = await opts.resolver.resolve(sourceLocator, opts);
+
+    return {...sourcePkg, ...locator};
+  }
+}

packages/plugin-github/sources/githubUtils.test.js

@@ -1,18 +1,32 @@
-import {gitUtils}       from '@yarnpkg/plugin-git';
-
 import * as githubUtils from './githubUtils';
 
 const VALID_PATTERNS = [
+  // plugin-github only
   `GitHubOrg/foo-bar.js`,
+  `GitHubOrg/foo2bar.js`,
   `GitHubOrg/foo-bar.js#hash`,
+  `GitHubOrg/foo-bar.js#commit:hash`,
+  `GitHubOrg/foo-bar.js#commit=hash`,
+  `GitHubOrg/foo-bar.js#commit=hash&workspace=foo`,
+  `GitHubOrg/foo-bar.js#tag=hello`,
+  `GitHubOrg/foo-bar.js#workspace=foo`,
   `github:GitHubOrg/foo-bar.js`,
   `github:GitHubOrg/foo-bar.js#hash`,
+  `https://github.com/TooTallNate/util-deprecate/tarball/b3562c2798507869edb767da869cd7b85487726d`,
+  // cross with plugin-git
+  `https://github.com/TooTallNate/util-deprecate.git#v1.0.0`,
+  `https://github.com/TooTallNate/util-deprecate.git#semver:v1.0.0`,
+  `https://github.com/TooTallNate/util-deprecate.git#master`,
+  `https://github.com/TooTallNate/util-deprecate.git#b3562c2798507869edb767da869cd7b85487726d`,
+  `git://github.com/TooTallNate/util-deprecate.git#v1.0.1`,
+  `git+https://github.com/TooTallNate/util-deprecate#v1.0.1`,
+  `git+https://github.com/TooTallNate/util-deprecate.git#v1.0.1`,
 ];
 
 describe(`gitUtils`, () => {
   for (const pattern of VALID_PATTERNS) {
     it(`should detect ${pattern} as a valid Git url`, () => {
-      expect(githubUtils.isGithubUrl(gitUtils.normalizeRepoUrl(pattern))).toEqual(true);
+      expect(githubUtils.isGithubUrl(githubUtils.normalizeRepoUrl(pattern))).toEqual(true);
     });
   }
 });

packages/plugin-github/sources/githubUtils.ts

@@ -1,17 +1,40 @@
-import querystring from 'querystring';
+import {structUtils, Locator, Descriptor} from '@yarnpkg/core';
+import {gitUtils}                         from '@yarnpkg/plugin-git';
+
+import querystring                        from 'querystring';
 
 type ParsedGithubUrl = {
   auth?: string;
   username: string;
   reponame: string;
   treeish: string;
+  extra: {
+    [key: string]: string,
+  },
 };
 
+/**
+ * Only match http urls that @yarnpkg/plugin-git may handle
+ */
+const gitPatterns = [
+  /^(?:git|(?:git[+:])?https?):\/\/(?:([^/]+?)@)?github\.com\/(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)(?:\.git)?(?:#(.*))?$/,
+];
+
+/**
+ * All patterns
+ */
 const githubPatterns = [
-  /^https?:\/\/(?:([^/]+?)@)?github.com\/([^/#]+)\/([^/#]+)\/tarball\/([^/#]+)(?:#(.*))?$/,
-  /^https?:\/\/(?:([^/]+?)@)?github.com\/([^/#]+)\/([^/#]+?)(?:\.git)?(?:#(.*))?$/,
+  /^(?:github:|(?:git|(?:git[+:])?https?):\/\/(?:([^/]+?)@)?github\.com\/)?(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)(?:\.git)?(?:#(.*))?$/,
+  /^https?:\/\/(?:([^/]+?@))?github.com\/([^/#]+)\/([^/#]+)\/tarball\/([^/#]+)(?:#(.*))?$/,
+  ...gitPatterns,
 ];
 
+export async function addHandledHostedRepository(addHandledHostedRepository: (regExp: RegExp) => void) {
+  for (const pattern of gitPatterns) {
+    addHandledHostedRepository(pattern);
+  }
+}
+
 /**
  * Determines whether a given url is a valid github git url via regex
  */
@@ -24,6 +47,7 @@ export function isGithubUrl(url: string): boolean {
  * an object of type `ParsedGithubUrl`
  */
 export function parseGithubUrl(urlStr: string): ParsedGithubUrl {
+  urlStr = normalizeRepoUrl(urlStr);
   let match;
   for (const pattern of githubPatterns) {
     match = urlStr.match(pattern);
@@ -37,7 +61,7 @@ export function parseGithubUrl(urlStr: string): ParsedGithubUrl {
 
   let [, auth, username, reponame, treeish = `master`] = match;
 
-  const {commit} = querystring.parse(treeish) as {commit?: string};
+  const {commit, ...extra} = querystring.parse(treeish) as {commit?: string, [key: string]: string | undefined};
 
   treeish =
     // New style:
@@ -49,7 +73,39 @@ export function parseGithubUrl(urlStr: string): ParsedGithubUrl {
     // Shouldn't ever be needed by the GithubFetcher
     || treeish.replace(/[^:]*:/, ``);
 
-  return {auth, username, reponame, treeish};
+  return {auth, username, reponame, treeish, extra: (extra as {[key: string]: string})};
+}
+
+export function normalizeRepoUrl(url: string) {
+  url = gitUtils.normalizeRepoUrl(url);
+
+  // We support this as an alias to GitHub repositories
+  url = url.replace(/^(?:github:|(?:git[+:])?https:\/\/(?:([^/]+?@))?github\.com\/)?(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)(?:\.git)?(#.*)?$/, `https://$1github.com/$2/$3.git$4`);
+
+  // We support GitHub `/tarball/` URLs
+  url = url.replace(/^https:\/\/(?:([^/]+?)@)?github\.com\/(?!\.{1,2}\/)([a-zA-Z0-9._-]+)\/(?!\.{1,2}(?:#|$))([a-zA-Z0-9._-]+?)\/tarball\/(.+)?$/, `https://$1github.com/$2/$3.git#$4`);
+
+  return url;
+}
+
+export function getUrl({auth, username, reponame, treeish, extra}: ParsedGithubUrl) {
+  return `https://${auth ? `${auth}@` : ``}github.com/${username}/${reponame}/archive/${treeish}.tar.gz#source=true${extra.workspace ? `&workspace=${extra.workspace}`: ``}`;
+}
+
+export function getDescriptorUrl(locator: Descriptor) {
+  return getUrl(parseGithubUrl(locator.range));
+}
+
+export function getLocatorUrl(locator: Locator) {
+  return getUrl(parseGithubUrl(locator.reference));
+}
+
+export function makeDescriptor(descriptor: Descriptor) {
+  return structUtils.makeDescriptor(descriptor, getDescriptorUrl(descriptor));
+}
+
+export function makeLocator(locator: Locator) {
+  return structUtils.makeLocator(locator, getLocatorUrl(locator));
 }
 
 export function invalidGithubUrlMessage(url: string): string {