Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix --save not working with urls that get redirected. #418

Closed
wants to merge 4 commits into from

2 participants

@satazor
Owner

Ugly fix for #417.

Also added an unimplemented test (it.skip) for this, so that this bug won't happen in the rewrite.

If anyone is willing to implement that test, please do.

@necolas
Owner

Hey. Please can you squash your commits where appropriate?

@satazor
Owner

@necolas of course, I just want you guys to review so I can use pulley or rebase.

@satazor satazor closed this in c904c81
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 8 deletions.
  1. +9 −7 lib/core/package.js
  2. +1 −1  lib/util/save.js
  3. +2 −0  test/install.js
View
16 lib/core/package.js
@@ -110,6 +110,7 @@ var Package = function (name, endpoint, manager) {
// This is because the tag & paths can get rewritten later
if (this.tag) this.originalTag = this.tag;
if (this.path) this.originalPath = endpoint;
+ if (this.assetUrl) this.originalAssetUrl = this.assetUrl;
// The id is an unique id that describes this package
this.id = crypto.createHash('md5').update(this.name + '%' + this.tag + '%' + this.gitUrl + '%' + this.path + '%' + this.assetUrl).digest('hex');
@@ -308,7 +309,7 @@ Package.prototype.generateAssetJSON = function () {
name: this.name,
main: this.assetType !== '.zip' && this.assetType !== '.tar' ? 'index' + this.assetType : '',
version: '0.0.0',
- repository: { type: 'asset', url: this.assetUrl }
+ repository: { type: 'asset', url: this.originalAssetUrl }
};
};
@@ -444,7 +445,7 @@ Package.prototype.download = function () {
req.get(src, function (res) {
// If assetUrl results in a redirect we update the assetUrl to the redirect to url
if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
- template('action', { name: 'redirect detected', shizzle: this.assetUrl })
+ template('action', { name: 'redirect detected', shizzle: res.headers.location})
.on('data', this.emit.bind(this, 'data'));
this.assetUrl = res.headers.location;
return this.download();
@@ -796,17 +797,17 @@ Package.prototype.readEndpoint = function (replace) {
return { type: 'local', endpoint: this.path };
}
if (this.json.repository.type === 'local') {
- if (replace || !this.path) {
- this.path = path.resolve(this.json.repository.path);
+ if (replace || !this.originalPath) {
+ this.originalPath = this.path = path.resolve(this.json.repository.path);
}
- return { type: 'local', endpoint: this.path };
+ return { type: 'local', endpoint: this.originalPath };
}
if (this.json.repository.type === 'asset') {
if (replace || !this.assetUrl) {
- this.assetUrl = this.json.repository.url;
+ this.originalAssetUrl = this.assetUrl = this.json.repository.url;
this.assetType = path.extname(this.assetUrl);
}
- return { type: 'asset', endpoint: this.assetUrl };
+ return { type: 'asset', endpoint: this.originalAssetUrl };
}
};
@@ -827,6 +828,7 @@ Package.prototype.serialize = function () {
originalTag: this.originalTag,
commit: this.commit,
assetUrl: this.assetUrl,
+ originalAssetUrl: this.originalAssetUrl,
assetType: this.assetType,
lookedUp: this.lookedUp,
json: this.json,
View
2  lib/util/save.js
@@ -85,7 +85,7 @@ function addDependency(json, pkg, dev) {
if (pkg.lookedUp) {
tag = pkg.originalTag || '~' + pkg.version;
} else {
- path = (pkg.shorthand || pkg.gitUrl || pkg.assetUrl || pkg.originalPath || '');
+ path = (pkg.shorthand || pkg.gitUrl || pkg.originalAssetUrl || pkg.originalPath || '');
tag = pkg.originalTag || '~' + pkg.version;
}
View
2  test/install.js
@@ -139,6 +139,8 @@ describe('install', function () {
});
});
+ it.skip('should save an URL to the json even if a redirect occurs');
+
it('Should save devDependencies to the json', function (done) {
fs.mkdirSync(testDir);
process.chdir(testDir);
Something went wrong with that request. Please try again.