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 all commits
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
3 changes: 2 additions & 1 deletion lib/commands/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var Q = require('q');
var Project = require('../core/Project');
var createLink = require('../util/createLink');
var defaultConfig = require('../config');
var relativeToBaseDir = require('../util/relativeToBaseDir');

function link(logger, name, localName, config) {
if (name) {
Expand Down Expand Up @@ -49,7 +50,7 @@ function linkTo(logger, name, localName, config) {

localName = localName || name;
src = path.join(config.storage.links, name);
dst = path.join(config.cwd, config.directory, localName);
dst = path.join(relativeToBaseDir(config.cwd)(config.directory), localName);

// Delete destination folder if any
return Q.nfcall(rimraf, dst)
Expand Down
9 changes: 5 additions & 4 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 relativeToBaseDir = require('../util/relativeToBaseDir');

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 = relativeToBaseDir(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 = relativeToBaseDir(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 = relativeToBaseDir(this._config.cwd)(this._config.directory);
return Q.nfcall(mkdirp, componentsDir)
.then(function () {
var promises = [];
Expand Down Expand Up @@ -634,7 +635,7 @@ Manager.prototype._dissect = function () {
}, this);

// Filter only packages that need to be installed
componentsDir = path.resolve(that._config.cwd, that._config.directory);
componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
this._dissected = mout.object.filter(suitables, function (decEndpoint, name) {
var installedMeta = this._installed[name];
var dst;
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 relativeToBaseDir = require('../util/relativeToBaseDir');

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 = relativeToBaseDir(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 = relativeToBaseDir(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 isPathAbsolute(filePath) {
return filePath.charAt(0) === '/';
}

module.exports = isPathAbsolute;
14 changes: 14 additions & 0 deletions lib/util/relativeToBaseDir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
var path = require('path');
var isPathAbsolute = require('./isPathAbsolute');

function relativeToBaseDir(baseDir) {
return function(filePath) {
if(isPathAbsolute(filePath)) {
return path.resolve(filePath);
} else {
return path.resolve(baseDir, filePath);
}
};
}

module.exports = relativeToBaseDir;
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
50 changes: 50 additions & 0 deletions test/commands/link.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var path = require('path');
var expect = require('expect.js');
var helpers = require('../helpers');

Expand Down Expand Up @@ -69,6 +70,55 @@ describe('bower link', function () {
});
});

it('creates inter-link to relative config.directory', function () {
return helpers.run(link, [undefined, undefined,
{
cwd: mainPackage.path,
storage: {
links: linksDir.path
}
}
]).then(function () {
return helpers.run(link, ['package', undefined,
{
cwd: otherPackage.path,
directory: 'valid-extend',
storage: {
links: linksDir.path
}
}
]);
}).then(function() {
expect(otherPackage.read('valid-extend/package/index.js'))
.to.be('Hello World!');
});
});


it('creates inter-link to absolute config.directory', function () {
return helpers.run(link, [undefined, undefined,
{
cwd: mainPackage.path,
storage: {
links: linksDir.path
}
}
]).then(function () {
return helpers.run(link, ['package', undefined,
{
cwd: path.join(otherPackage.path, 'invalid'),
directory: path.join(otherPackage.path, 'valid-override'),
storage: {
links: linksDir.path
}
}
]);
}).then(function() {
expect(otherPackage.read('valid-override/package/index.js'))
.to.be('Hello World!');
});
});

it('creates inter-link with custom local name', function () {
return helpers.run(link, [undefined, undefined,
{
Expand Down
35 changes: 35 additions & 0 deletions test/commands/uninstall.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var path = require('path');
var mkdirp = require('mkdirp');
var expect = require('expect.js');
var fs = require('../../lib/util/fs');

Expand Down Expand Up @@ -53,4 +54,38 @@ describe('bower uninstall', function () {
});
});

it('removes dependency from relative config.directory', function () {
var targetPath = path.resolve(tempDir.path, 'other_directory/underscore');
mkdirp.sync(targetPath);
fs.writeFileSync(path.join(targetPath, '.bower.json'), '{ "name": "underscore" }');

return helpers.run(uninstall, [['underscore'], undefined, {
cwd: tempDir.path,
directory: 'other_directory',
interactive: true
}])
.then(function() {
expect(function() {
fs.statSync(targetPath);
}).to.throwException(/no such file or directory/);
});
});

it('removes dependency from absolute config.directory', function () {
var targetPath = path.resolve(tempDir.path, 'other_directory/underscore');
mkdirp.sync(targetPath);
fs.writeFileSync(path.join(targetPath, '.bower.json'), '{ "name": "underscore" }');

return helpers.run(uninstall, [['underscore'], undefined, {
cwd: tempDir.path,
directory: path.resolve(tempDir.path, 'other_directory'),
interactive: true
}])
.then(function() {
expect(function() {
fs.statSync(targetPath);
}).to.throwException(/no such file or directory/);
});
});

});
57 changes: 57 additions & 0 deletions test/util/createLink.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
var path = require('path');
var Q = require('q');
var fs = require('fs');
var expect = require('expect.js');
var helpers = require('../helpers');
var createLink = require('../../lib/util/createLink');

describe('createLink', function () {

var srcDir = new helpers.TempDir({
someFile: 'Hello World',
someDirectory: {
otherFile: 'Hello World'
}
});

var dstDir = new helpers.TempDir();

beforeEach(function() {
srcDir.prepare();
dstDir.prepare();
});

it('creates a symlink to a file', function() {

var src = path.join(srcDir.path, 'someFile'),
dst = path.join(dstDir.path, 'someFile');

return createLink(src, dst)
.then(function() {
return Q.nfcall(fs.readlink, dst)
.then(function(linkString) {
expect(linkString).to.be.equal(src);
});
});
});

it('throws an error when destination already exists', function() {

var src = path.join(srcDir.path, 'someFile'),
dst = path.join(dstDir.path);

var deferred = Q.defer();

createLink(src, dst)
.catch(function(err) {
expect(err.code).to.be.equal('EEXIST');
deferred.resolve();
})
.then(function() {
deferred.reject();
});

return deferred.promise;
});

});
3 changes: 3 additions & 0 deletions test/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ describe('util', function () {
require('./removeIgnores');
require('./analytics');
require('./download');
require('./isPathAbsolute');
require('./relativeToBaseDir');
require('./createLink');
});
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();
});

});
18 changes: 18 additions & 0 deletions test/util/relativeToBaseDir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
var path = require('path');
var expect = require('expect.js');
var relativeToBaseDir = require('../../lib/util/relativeToBaseDir');

describe('relativeToBaseDir', function () {

var joinOrReturnAbsolutePath = relativeToBaseDir('/tmp');

it('returns a partial function that joins paths of the partials first arguments', function() {
expect(joinOrReturnAbsolutePath('foo')).to.be.equal(path.resolve('/tmp/foo'));
expect(joinOrReturnAbsolutePath('./foo')).to.be.equal(path.resolve('/tmp/foo'));
});

it('returns a partial function that returns it\'s first argument when it begins with /', function() {
expect(joinOrReturnAbsolutePath('/foo')).to.be.equal(path.resolve('/foo'));
expect(joinOrReturnAbsolutePath('/foo/bar')).to.be.equal(path.resolve('/foo/bar'));
});
});