Skip to content

Loading…

Implement npm install foo --save equivalent #118

Open
wants to merge 6 commits into from

5 participants

@wombleton

Update package.json's jam.dependencies or jam.devDependencies if --save
or --save-dev passed

@wombleton wombleton Implement npm install foo --save equivalent
Update package.json's jam.dependencies or jam.devDependencies if --save
or --save-dev passed
02f6d1a
@caolan caolan commented on an outdated diff
lib/commands/install.js
@@ -406,7 +406,34 @@ exports.installRepo = function (name, range, opt, callback) {
if (err) {
return callback(err);
}
- exports.cpDir(name, v, from_cache, cdir, opt, callback);
+ if (opt.save || opt.save_dev) {
+ exports.cpDir(name, v, from_cache, cdir, opt, function() {
+ fs.readFile('package.json', function(err, data) {
+ var deps = opt.save ? 'dependencies' : 'devDependencies';
@caolan Owner
caolan added a note

It's also possible to use jam.dependencies in package.json... devDependencies is currently unused by jam. I'd recommend we only use jam.dependencies to avoid conflicting with NPM's 'dependencies' property when saving.

...and I've just noticed you are namespacing it under 'jam' later on ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@caolan caolan commented on an outdated diff
lib/commands/install.js
@@ -406,7 +406,34 @@ exports.installRepo = function (name, range, opt, callback) {
if (err) {
return callback(err);
}
- exports.cpDir(name, v, from_cache, cdir, opt, callback);
+ if (opt.save || opt.save_dev) {
+ exports.cpDir(name, v, from_cache, cdir, opt, function() {
+ fs.readFile('package.json', function(err, data) {
@caolan Owner
caolan added a note

Does this path work when in a sub-directory of the project? Currently commands will walk up the directory hierarchy to find the project directory containing package.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@caolan caolan commented on an outdated diff
lib/commands/install.js
@@ -69,7 +69,9 @@ exports.run = function (settings, args) {
var a = argParse(args, {
'repository': {match: ['-r', '--repository'], value: true},
'target_dir': {match: ['-d', '--package-dir'], value: true},
- 'baseurl': {match: ['-b', '--baseurl'], value: true}
+ 'baseurl': {match: ['-b', '--baseurl'], value: true},
+ 'save': {match: ['-S', '--save'], value: false},
+ 'save_dev': {match: ['-D', '--save-dev'], value: false}
@caolan Owner
caolan added a note

No need for this option, since Jam doesn't use devDependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@caolan
Owner

Seems like a nice feature to have. Can you please take a look at my comments on the diff and add an integration test? Thanks :)

@wombleton

I'll get on that tomorrow, cheers.

@caolan caolan commented on an outdated diff
lib/commands/install.js
@@ -406,7 +407,34 @@ exports.installRepo = function (name, range, opt, callback) {
if (err) {
return callback(err);
}
- exports.cpDir(name, v, from_cache, cdir, opt, callback);
+ if (opt.save) {
+ exports.cpDir(name, v, from_cache, cdir, opt, function() {
@caolan Owner
caolan added a note

no error handling for the cpDir callback?

Turns out I had no reason at all for leaving that out. Fix'd!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thebigredgeek

Any update on this? I would like to see this soon.

@ryanramage ryanramage commented on an outdated diff
lib/commands/install.js
((13 lines not shown))
+ package = path.resolve(opt.proj_dir, 'package.json');
+
+ fs.readFile(path.resolve(package), function(err, data) {
+ try {
+ data = JSON.parse(data.toString('utf8'));
+ } catch(e) {
+ err = e;
+ }
+ if (err) {
+ return callback();
+ }
+ if (!data.jam) {
+ data.jam = {};
+ }
+ data.jam.dependencies = data.jam.dependencies || {};
+ data.jam.dependencies[name] = v;
@ryanramage Collaborator

This assumes the dependences are stored in the jam property. storing dependencies on the root of the package.json is acceptable. So you should test which way the current package is setup (if any)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thebigredgeek

Any update on this @wombleton ?

@thebigredgeek

Bump...

@suisho

I want this too.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 17, 2012
  1. @wombleton

    Implement npm install foo --save equivalent

    wombleton committed
    Update package.json's jam.dependencies or jam.devDependencies if --save
    or --save-dev passed
  2. @wombleton
  3. @wombleton

    Add test to scripts property

    wombleton committed
  4. @wombleton
Commits on Apr 23, 2013
  1. @wombleton

    Handle error for cpDir

    wombleton committed
Commits on Jul 12, 2013
  1. @wombleton
Showing with 162 additions and 4 deletions.
  1. +43 −4 lib/commands/install.js
  2. +3 −0 package.json
  3. +116 −0 test/integration/test-install-save.js
View
47 lib/commands/install.js
@@ -69,7 +69,8 @@ exports.run = function (settings, args) {
var a = argParse(args, {
'repository': {match: ['-r', '--repository'], value: true},
'target_dir': {match: ['-d', '--package-dir'], value: true},
- 'baseurl': {match: ['-b', '--baseurl'], value: true}
+ 'baseurl': {match: ['-b', '--baseurl'], value: true},
+ 'save': {match: ['-S', '--save'], value: false}
});
var opt = a.options;
@@ -117,6 +118,8 @@ exports.run = function (settings, args) {
exports.extendOptions = function (proj_dir, settings, cfg, opt) {
+ opt.proj_dir = proj_dir;
+
if (!opt.target_dir) {
if (cfg.jam && cfg.jam.packageDir) {
opt.target_dir = path.resolve(proj_dir, cfg.jam.packageDir);
@@ -159,7 +162,6 @@ exports.reinstallPackages = function (cfg, opt, callback) {
if (err) {
return logger.error(err);
}
- // TODO: write package.json if --save option provided
project.updateRequireConfig(opt.target_dir, opt.baseurl,
function (err) {
if (err) {
@@ -187,7 +189,6 @@ exports.installPackages = function (cfg, names, opt, callback) {
if (err) {
return callback(err);
}
- // TODO: write package.json if --save option provided
project.updateRequireConfig(opt.target_dir, opt.baseurl, callback);
});
};
@@ -406,7 +407,45 @@ exports.installRepo = function (name, range, opt, callback) {
if (err) {
return callback(err);
}
- exports.cpDir(name, v, from_cache, cdir, opt, callback);
+ if (opt.save) {
+ exports.cpDir(name, v, from_cache, cdir, opt, function(err) {
+ var package;
+
+ if (err) {
+ return callback(err);
+ }
+
+ package = path.resolve(opt.proj_dir, 'package.json');
+
+ fs.readFile(path.resolve(package), function(err, data) {
+ try {
+ data = JSON.parse(data.toString('utf8'));
+ } catch(e) {
+ err = e;
+ }
+ if (err) {
+ return callback();
+ }
+ if (!data.dependencies && !data.jam) {
+ data.jam = {};
+ }
+ // if jam namespace exists
+ if (data.jam) {
+ data.jam.dependencies = data.jam.dependencies || {};
+ data.jam.dependencies[name] = v;
+ } else {
+ data.dependencies[name] = v;
+ }
+
+ data = JSON.stringify(data, null, 2) + '\n';
+ fs.writeFile(package, data, function(err) {
+ callback(err);
+ });
+ });
+ });
+ } else {
+ exports.cpDir(name, v, from_cache, cdir, opt, callback);
+ }
}
);
};
View
3 package.json
@@ -34,5 +34,8 @@
"bugs": {"url": "http://github.com/caolan/jam/issues"},
"bin": {
"jam": "./bin/jam.js"
+ },
+ "scripts": {
+ "test": "test/all.sh"
}
}
View
116 test/integration/test-install-save.js
@@ -0,0 +1,116 @@
+/**
+ * Test description
+ * ================
+ *
+ * Tests that jam install --save adds to package.json
+ *
+ * Starting with project with *ranged* deps in package.json
+ * - jam publish package-one @ 0.0.1
+ * - jam publish package-two @ 0.0.1
+ * - jam install package-one --save, test package.json updated
+ */
+
+
+var couchdb = require('../../lib/couchdb'),
+ logger = require('../../lib/logger'),
+ env = require('../../lib/env'),
+ utils = require('../utils'),
+ async = require('async'),
+ http = require('http'),
+ path = require('path'),
+ ncp = require('ncp').ncp,
+ fs = require('fs'),
+ _ = require('underscore');
+
+
+var pathExists = fs.exists || path.exists;
+
+
+logger.clean_exit = true;
+
+// CouchDB database URL to use for testing
+var TESTDB = process.env['JAM_TEST_DB'],
+ BIN = path.resolve(__dirname, '../../bin/jam.js'),
+ ENV = {JAM_TEST: 'true', JAM_TEST_DB: TESTDB};
+
+if (!TESTDB) {
+ throw 'JAM_TEST_DB environment variable not set';
+}
+
+// remove trailing-slash from TESTDB URL
+TESTDB = TESTDB.replace(/\/$/, '');
+
+
+exports.setUp = function (callback) {
+ // change to integration test directory before running test
+ this._cwd = process.cwd();
+ process.chdir(__dirname);
+
+ // recreate any existing test db
+ couchdb(TESTDB).deleteDB(function (err) {
+ if (err && err.error !== 'not_found') {
+ return callback(err);
+ }
+ // create test db
+ couchdb(TESTDB).createDB(callback);
+ });
+};
+
+exports.tearDown = function (callback) {
+ // change back to original working directory after running test
+ process.chdir(this._cwd);
+ // delete test db
+ couchdb(TESTDB).deleteDB(callback);
+};
+
+
+exports['project with ranged dependencies in package.json'] = {
+
+ setUp: function (callback) {
+ this.project_dir = path.resolve(env.temp, 'jamtest-' + Math.random());
+ // set current project to empty directory
+ ncp('./fixtures/project-rangedeps', this.project_dir, callback);
+ },
+
+ /*
+ tearDown: function (callback) {
+ var that = this;
+ // clear current project
+ //utils.myrimraf(that.project_dir, callback);
+ },
+ */
+
+ 'publish, install, ls, remove': function (test) {
+ test.expect(1);
+ var that = this;
+ process.chdir(that.project_dir);
+
+ async.series([
+ async.apply(
+ utils.runJam,
+ ['publish', path.resolve(__dirname, 'fixtures', 'package-one')],
+ {env: ENV}
+ ),
+ async.apply(
+ utils.runJam,
+ ['publish', path.resolve(__dirname, 'fixtures', 'package-two')],
+ {env: ENV}
+ ),
+ async.apply(utils.runJam, ['install', 'package-one', '--save'], {env: ENV}),
+ function (cb) {
+ fs.readFile(path.resolve(that.project_dir, 'package.json'), function(err, data) {
+ data = JSON.parse(data);
+
+ test.same(data.jam.dependencies, {
+ "package-one": "0.0.1",
+ "package-two": "<=0.0.2"
+ });
+ cb();
+ });
+ }
+ ],
+ test.done);
+ }
+
+};
+
Something went wrong with that request. Please try again.