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

support absolute path in .bowerrc directory option #2130

Merged
merged 2 commits into from
Jan 5, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/core/Manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var semver = require('../util/semver');
var copy = require('../util/copy');
var createError = require('../util/createError');
var scripts = require('./scripts');
var isPathAbsolute = require('../util/isPathAbsolute');

function Manager(config, logger) {
this._config = config;
Expand Down Expand Up @@ -124,7 +125,7 @@ Manager.prototype.resolve = function () {

Manager.prototype.preinstall = function (json) {
var that = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use path.resolve instead? to support relative config.directory

Copy link
Member Author

Choose a reason for hiding this comment

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

When the config.directory begins with an / it's expected to be absolute. In any other case the path is expected to be relative and joined with config.cwd. Not sure if I understood your comment correctly. How can path.resolve help here?

Copy link
Contributor

Choose a reason for hiding this comment

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

resolve "removes" .. from joined path.

path.join('/foo/bar', '../baz') == '/foo/bar/../baz'
path.resolve('/foo/bar', '../baz') == '/foo'

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

var componentsDir = path.join(this._config.cwd, this._config.directory);
var componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);

// If nothing to install, skip the code bellow
if (mout.lang.isEmpty(that._dissected)) {
Expand All @@ -141,7 +142,7 @@ Manager.prototype.preinstall = function (json) {

Manager.prototype.postinstall = function (json) {
var that = this;
var componentsDir = path.join(this._config.cwd, this._config.directory);
var componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);

// If nothing to install, skip the code bellow
if (mout.lang.isEmpty(that._dissected)) {
Expand Down Expand Up @@ -171,7 +172,7 @@ Manager.prototype.install = function (json) {
return Q.resolve({});
}

componentsDir = path.join(this._config.cwd, this._config.directory);
componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
return Q.nfcall(mkdirp, componentsDir)
.then(function () {
var promises = [];
Expand Down
5 changes: 3 additions & 2 deletions lib/core/Project.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var createError = require('../util/createError');
var readJson = require('../util/readJson');
var validLink = require('../util/validLink');
var scripts = require('./scripts');
var isPathAbsolute = require('../util/isPathAbsolute');

function Project(config, logger) {
this._config = config;
Expand Down Expand Up @@ -607,7 +608,7 @@ Project.prototype._readInstalled = function () {

// Gather all folders that are actual packages by
// looking for the package metadata file
componentsDir = path.join(this._config.cwd, this._config.directory);
componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
return this._installed = Q.nfcall(glob, '*/.bower.json', {
cwd: componentsDir,
dot: true
Expand Down Expand Up @@ -648,7 +649,7 @@ Project.prototype._readLinks = function () {
var that = this;

// Read directory, looking for links
componentsDir = path.join(this._config.cwd, this._config.directory);
componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
return Q.nfcall(fs.readdir, componentsDir)
.then(function (filenames) {
var promises;
Expand Down
5 changes: 5 additions & 0 deletions lib/util/isPathAbsolute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function isAbsolutePath(filePath) {
return filePath.charAt(0) === '/';
}

module.exports = isAbsolutePath;
32 changes: 32 additions & 0 deletions test/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var expect = require('expect.js');
var path = require('path');
var helpers = require('../helpers');
var nock = require('nock');
var rimraf = require('rimraf');
var fs = require('../../lib/util/fs');
var tar = require('tar-fs');
var destroy = require('destroy');
Expand Down Expand Up @@ -154,6 +155,37 @@ describe('bower install', function() {
});
});

it('.bowerrc directory can be an absolute path', function() {
mainPackage.prepare({
foo: 'bar'
});

tempDir.prepare({
'.bowerrc': {
directory: '/tmp/bower-absolute-destination-directory'
},
'bower.json': {
name: 'test',
dependencies: {
package: mainPackage.path
}
}
});

return helpers.run(install).then(function() {
expect(require('fs').readFileSync('/tmp/bower-absolute-destination-directory/package/foo', 'utf8').toString()).to.be('bar');
var deferred = Q.defer();
rimraf('/tmp/bower-absolute-destination-directory', function(err) {
if(err) {
deferred.reject(err);
} else {
deferred.resolve();
}
});
return deferred;
});
});

it('runs preinstall hook', function() {
mainPackage.prepare();

Expand Down
1 change: 1 addition & 0 deletions test/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ describe('util', function () {
require('./removeIgnores');
require('./analytics');
require('./download');
require('./isPathAbsolute');
});
14 changes: 14 additions & 0 deletions test/util/isPathAbsolute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
var expect = require('expect.js');
var isPathAbsolute = require('../../lib/util/isPathAbsolute');

describe('isPathAbsolute', function () {

it('returns true when a path begins with /', function() {
expect(isPathAbsolute('/tmp/foo')).to.be.ok();
});

it('returns false when a path does not begin with /', function() {
expect(isPathAbsolute('./tmp/foo')).to.not.be.ok();
});

});