Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for private GitHub shortcuts deps #971

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions __tests__/package-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import * as fs from '../src/util/fs.js';
jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000;

const path = require('path');
const isCI = require('is-ci');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice find!


function addTest(pattern, registry = 'npm') {
// TODO renable these test.concurrent
Expand All @@ -35,6 +36,7 @@ function addTest(pattern, registry = 'npm') {
});
}

// Public deps
addTest('https://github.com/npm-ml/re'); // git url with no .git
addTest('https://bitbucket.org/hgarcia/node-bitbucket-api.git'); // hosted git url
addTest('https://github.com/PolymerElements/font-roboto/archive/2fd5c7bd715a24fb5b250298a140a3ba1b71fe46.tar.gz'); // tarball
Expand All @@ -51,3 +53,12 @@ addTest('react-native'); // npm
addTest('ember-cli'); // npm
addTest('npm:gulp'); // npm
addTest('@polymer/iron-icon'); // npm scoped package

// Private deps

// Only the yarn CI tools have access to this private deps. If you want to test this locally,
// remove the if condition and change the urls to match your private deps.
if (isCI) {
addTest('yarnpkg/private-dep#c6cf811'); // private github shortcut
addTest('github:yarnpkg/private-dep#c6cf811'); // private github shortcut, with provider
}
15 changes: 13 additions & 2 deletions __tests__/resolvers/exotics/bitbucket-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,25 @@ test('getGitHTTPUrl should return the correct git bitbucket url', () => {
expect(BitBucketResolver.getGitHTTPUrl(fragment)).toBe(expected);
});

test('getGitHTTPUrl should return the correct git bitbucket SSH url', () => {
test('getGitSSH should return the correct git bitbucket SSH url', () => {
const fragment: ExplodedFragment = {
user: 'foo',
repo: 'bar',
hash: '',
};

const expected = '[email protected]:' + fragment.user + '/' + fragment.repo + '.git';
const expected = `[email protected]:${fragment.user}/${fragment.repo}.git`;
expect(BitBucketResolver.getGitSSH(fragment)).toBe(expected);
});

test('getGitSSHUrl should return the correct git bitbucket SSH url', () => {
const fragment: ExplodedFragment = {
user: 'foo',
repo: 'bar',
hash: '',
};

const expected = `git+ssh://[email protected]/${fragment.user}/${fragment.repo}.git`;
expect(BitBucketResolver.getGitSSHUrl(fragment)).toBe(expected);
});

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"ini": "^1.3.4",
"invariant": "^2.2.0",
"is-builtin-module": "^1.0.0",
"is-ci": "^1.0.9",
"leven": "^2.0.0",
"loud-rejection": "^1.2.0",
"minimatch": "^3.0.3",
Expand Down
4 changes: 4 additions & 0 deletions src/resolvers/exotics/bitbucket-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export default class BitbucketResolver extends HostedGitResolver {
}

static getGitSSHUrl(parts: ExplodedFragment): string {
return `git+ssh://[email protected]/${ parts.user }/${ parts.repo }.git`;
}

static getGitSSH(parts: ExplodedFragment): string {
return `[email protected]:${parts.user}/${parts.repo}.git`;
}

Expand Down
5 changes: 5 additions & 0 deletions src/resolvers/exotics/github-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export default class GitHubResolver extends HostedGitResolver {
}

static getGitSSHUrl(parts: ExplodedFragment): string {
return `git+ssh://[email protected]/${ parts.user }/${ parts.repo }.git` +
`${parts.hash ? '#' + decodeURIComponent(parts.hash) : ''}`;
}

static getGitSSH(parts: ExplodedFragment): string {
return `git@${this.hostname}:${parts.user}/${parts.repo}.git` +
`${parts.hash ? '#' + decodeURIComponent(parts.hash) : ''}`;
}
Expand Down
4 changes: 4 additions & 0 deletions src/resolvers/exotics/gitlab-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export default class GitLabResolver extends HostedGitResolver {
}

static getGitSSHUrl(parts: ExplodedFragment): string {
return `git+ssh://[email protected]/${ parts.user }/${ parts.repo }.git`;
}

static getGitSSH(parts: ExplodedFragment): string {
return `[email protected]:${parts.user}/${parts.repo}.git`;
}

Expand Down
22 changes: 14 additions & 8 deletions src/resolvers/exotics/hosted-git-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ export default class HostedGitResolver extends ExoticResolver {
throw new Error('Not implemented');
}

static getGitSSH(exploded: ExplodedFragment): string {
exploded;
throw new Error('Not implemented');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is actually the purpose of this? Why would you add call to this function and just throw an error in it?

Copy link
Contributor Author

@ramasilveyra ramasilveyra Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what it does on all the methods of HostedGitResolver (src/resolvers/exotics/hosted-git-resolver.js) https://github.com/ramasilveyra/yarn/blob/efd9801987ef395a6a5ed30b3bc5e0cef94326b5/src/resolvers/exotics/hosted-git-resolver.js#L58-L84
I think that is for dev assurance for when you extend the class HostedGitResolver.

}

static getHTTPFileUrl(exploded: ExplodedFragment, filename: string, commit: string) {
exploded;
filename;
Expand Down Expand Up @@ -170,14 +175,15 @@ export default class HostedGitResolver extends ExoticResolver {
}

async resolve(): Promise<Manifest> {
const httpUrl = this.constructor.getGitHTTPUrl(this.exploded);
const sshUrl = this.constructor.getGitSSHUrl(this.exploded);
const gitHTTPUrl = this.constructor.getGitHTTPUrl(this.exploded);
const gitSSHUrl = this.constructor.getGitSSHUrl(this.exploded);
const gitSSH = this.constructor.getGitSSH(this.exploded);

// If we can access the files over HTTP then we should as it's MUCH faster than git
// archive and tarball unarchiving. The HTTP API is only available for public repos
// though.
if (await this.hasHTTPCapability(httpUrl)) {
return await this.resolveOverHTTP(httpUrl);
if (await this.hasHTTPCapability(gitHTTPUrl)) {
return await this.resolveOverHTTP(gitHTTPUrl);
}

// If the url is accessible over git archive then we should immediately delegate to
Expand All @@ -186,13 +192,13 @@ export default class HostedGitResolver extends ExoticResolver {
// NOTE: Here we use a different url than when we delegate to the git resolver later on.
// This is because `git archive` requires access over ssh and github only allows that
// if you have write permissions
if (await Git.hasArchiveCapability(sshUrl)) {
const archiveClient = new Git(this.config, sshUrl, this.hash);
if (await Git.hasArchiveCapability(gitSSH)) {
const archiveClient = new Git(this.config, gitSSH, this.hash);
const commit = await archiveClient.initRemote();
return await this.fork(GitResolver, true, `${sshUrl}#${commit}`);
return await this.fork(GitResolver, true, `${gitSSH}#${commit}`);
}

// fallback to the plain git resolver
return await this.fork(GitResolver, true, sshUrl);
return await this.fork(GitResolver, true, gitSSHUrl);
}
}
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2671,7 +2671,7 @@ is-builtin-module@^1.0.0:
dependencies:
builtin-modules "^1.0.0"

is-ci@^1.0.9:
is-ci, is-ci@^1.0.9:
version "1.0.9"
resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-1.0.9.tgz#de2c5ffe49ab3237fda38c47c8a3bbfd55bbcca7"

Expand Down