Skip to content

Commit

Permalink
Updated paths, added meteorite alias, addressed the HOME env with HOM…
Browse files Browse the repository at this point in the history
…EPATH, and added in the wich package for fining git, curl and meteor.

This seems to work reasonably well in cmd.exe and in cygwin.

Still working on test issues though.
  • Loading branch information
seanmwalker committed Jan 20, 2014
1 parent 6d09b3d commit bd7c019
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 16 deletions.
4 changes: 2 additions & 2 deletions lib/atmosphere.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ var writeUserTokenFile = function(user) {
var cfg = {
user: user
};
var filePath = path.join(process.env.HOME, '.mrt.cfg');
var filePath = path.join(process.env.HOME || process.env.HOMEPATH, '.mrt.cfg');

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

HOMEPATH does not contain system drive letter. Like this: \Users\yeputons. You should concatenate it with HOMEDRIVE, which may contain something like D:

This comment has been minimized.

Copy link
@seanmwalker

seanmwalker Jan 20, 2014

Author

Are you on a domain controller / corporate network? I'm not in any domain and my string starts with a drive letter. One thing I'd worried a little about is that these values can be set by network policy, so there may be more investigation to be sure. I'll add something in for this, but do you think we need to inspect the value to see if there is a X:\ (or /X/ in cygwin)?

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

Yes, I'm in corporate network. That's interesting. Do you have HOMEDRIVE variable?

This comment has been minimized.

Copy link
@seanmwalker

seanmwalker Jan 21, 2014

Author

Yep, I do.

I just fired up a few more windows machines here at the home office, and the others all show the same values you show. So I suspect my dev machine had it's value was changed while I was working with it. I rebooted and it's back to the same pattern as well.

It appears that Cygwin uses this:

Set up USER's home directory

if [ -z "$HOME" -o ! -d "$HOME" ]; then
HOME="$HOMEDRIVE$HOMEPATH"
if [ -z "$HOME" -o ! -d "$HOME" ]; then
HOME="$USERPROFILE"
fi
fi

But there may be an additional item to consider: If the files are located on a UNC drive then it should use HOMESHARE instead of HOMEDRIVE.

I've put together a function that will use HOME or concat (HOMEDRIVE || HOMESHARE) + HOMEPATH to build the path when HOME does not exist. Since there are two files that need this, I've added a new file for them both to reference. I'll do some additional testing but should have that in today.

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 21, 2014

Contributor

@seanmwalker By the way, do we really need user home directory? Meteor uses LOCALAPPDATA for all settings and it's happy. I found Wikipedia article on that topic, this Stackoverflow answer (which links to a Microsoft paper, read page 10). They claim that while we use HOME/{.AppName} in Unix, Windows analogue is APPDATA\{DeveloperName\AppName. I think that we can use simply APPDATA\meteorite (without a dot) to be consistent with what Windows offers.

This comment has been minimized.

Copy link
@seanmwalker

seanmwalker Jan 21, 2014

Author

@yeputons Yeah I was wondering about where this really wants to be. On *nix it's the home directory where all that .meteor etc goes. But appdata is where .meteor is usually at on my systems. I've put something in just a bit ago which has a function that builds the 'home' path for us, so it's easier to change than in 3 places and 2 files now. I can play around with that a bit more unless you want to take a look at it.


fs.writeFileSync(filePath, JSON.stringify(cfg));
fs.chmodSync(filePath, '0600');
};

var readUserTokenFile = function() {
var filePath = path.join(process.env.HOME, '.mrt.cfg');
var filePath = path.join(process.env.HOME || process.env.HOMEPATH, '.mrt.cfg');

var fileContents = fs.readFileSync(filePath);
return JSON.parse(fileContents);
Expand Down
21 changes: 19 additions & 2 deletions lib/meteor.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var fstream = require('fstream');
var wrench = require('wrench');
var spawn = require('child_process').spawn;
var exec = require('child_process').exec;
var which = require('which');


// A 'Meteor' refers to a single commit (branch, tag) of a version of the core meteor
Expand Down Expand Up @@ -75,7 +76,8 @@ Meteor.prototype.install = function(buildDevBundle, fn) {
} else {
// meteor --help installs the dev bundle before doing anything else
console.log('Downloading Meteor development bundle');
var meteor = spawn('./meteor', ['--help'], {cwd: self.source.path});
//var meteor = spawn('./meteor', ['--help'], {cwd: self.source.path});
var meteor = spawn(self._executable(), ['--help'], {cwd: self.source.path});

// just output the status bar
meteor.stderr.pipe(process.stderr);
Expand All @@ -95,6 +97,8 @@ Meteor.prototype.ensurePrepared = function(fn) {
}
};

/*
// Saving the original function so we can revert quickly once spawn-cmd is in place.
Meteor.prototype._executable = function() {
var self = this;
Expand All @@ -104,6 +108,19 @@ Meteor.prototype._executable = function() {
return path.join(self.source.packagePath(), 'meteor');
}
}
*/

// Temporary code to be ripped out once spawn-cmd is in place.
Meteor.prototype._executable = function() {
var self = this;
var executableName = path.basename(which.sync('meteor'));

if (self.defaultMeteor) {
return executableName;
} else {
return path.join(self.source.packagePath(), executableName);
}
}

// run a command using just this checked-out version of meteor
Meteor.prototype.execute = function(args, package_dir, fn) {
Expand Down Expand Up @@ -140,7 +157,7 @@ Meteor.prototype.hasPackage = function(pkgName, fn) {

if (!self.packageNames) {
// we have to call meteor list because there's no obvious place to look
exec('meteor list', function(error, list) {
exec(self._executable() + ' list', function(error, list) {
var lines = list.split('\n');

self.packageNames = _.map(lines, function(l) {
Expand Down
7 changes: 4 additions & 3 deletions lib/meteorite.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,10 @@ Meteorite.prototype['link-package'] = function(fn) {

// pointing to the wrong spot
if (old)
fs.unlinkSync(packagePath);
fs.unlinkSync(path.resolve(packagePath));

fs.symlinkSync(packageDir, packagePath);
// Windows uses junctions to solve for symlinks. They need to be full paths too.
fs.symlinkSync(path.resolve(packageDir), path.resolve(packagePath), 'junction');
}

/////// Package level meteorite commands
Expand Down Expand Up @@ -229,7 +230,7 @@ _.each([
// Class methods

Meteorite.root = function() {
var homeDir = process.env.HOME;
var homeDir = process.env.HOME || process.env.HOMEPATH;
return path.join(homeDir, '.meteorite');
};

Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
, "dependencies":
{ "ddp": ">=0.3.1"
, "underscore": "1.3.3"
, "wrench": "1.3.9"
, "wrench": ">=1.5.5"
, "fstream": ">=0.1.18"
, "optimist": ">=0.3.4"
, "prompt": "0.2.11"
, "colors": "0.6.0-1"
, "async": "0.2.9"
, "rolling_timeout_exec": ">=0.0.1"
, "which": ">=1.0.5"
}
, "devDependencies":
{ "mocha": ">=1.2.2"
Expand All @@ -28,5 +29,5 @@
{ "type" : "git"
, "url" : "https://github.com/oortcloud/meteorite.git"
}
, "bin" : { "mrt" : "./bin/mrt.js" }
, "bin" : { "mrt" : "./bin/mrt.js", "meteorite" : "./bin/mrt.js" }
}
10 changes: 7 additions & 3 deletions spec/acceptance/test_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var fs = require('fs');
var wrench = require('wrench');
var _ = require('underscore');
var ddp = require('ddp');
var which = require('which');

var utils = require('../lib/utils.js');
var atmosphere = require('../lib/atmosphere.js');
Expand All @@ -15,8 +16,11 @@ var appDir = path.join(appHome, 'app');

before(function(done){
// ensure our "cached" CURL is in the path
process.env._METEORITE_REAL_GIT = which.sync('git');
process.env.PATH = [path.resolve(path.join('spec', 'support', 'bin')), process.env.PATH].join(':');

process.env._METEORITE_REAL_CURL = which.sync('curl');

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

I think that curl is unused, see #238 and #240 . At least, it passes all tests if I delete /spec/support/bin/curl.

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

By the way, it's very strange to store this after you've changed PATH. All changes to PATH are applied immediately.

This comment has been minimized.

Copy link
@seanmwalker

seanmwalker Jan 20, 2014

Author

I wasn't clear on if curl really was not needed based on Tom's comments, but as long as it's still there I thought it may as well represent the correct location. Assuming we keep curl we can move the line above the path update. But this wasn't intended to affect the path variable, but rather just store the curl location for later use as a separate variable. Did I miss something in the observation?

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

@seanmwalker It doesn't change PATH, the line above does. Yes, just move this line to the top a bit.

process.env._METEORITE_REAL_METEOR = path.basename(which.sync('meteor'));

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

What is this line for? Quick hack until we use spawn-cmd?

This comment has been minimized.

Copy link
@seanmwalker

seanmwalker Jan 20, 2014

Author

It's a quick change to allow it to work now until the pending change to use spawn-cmd. Aren't we still going down that route? If not this may work for our needs, I just did not want to imply a change in direction from our previous discussions.

This comment has been minimized.

Copy link
@yeputons

yeputons Jan 20, 2014

Contributor

I just wanted to confirm that I understood you right.


// make sure Meteor doesn't try to install into our soon to be clean home dir
process.env.METEOR_WAREHOUSE_DIR = path.join(process.env.HOME, '.meteor');

Expand All @@ -27,7 +31,7 @@ before(function(done){
console.log("Preparing..")
// ensure we have the latest dev bundle cached
console.log(" Ensuring we have the dev bundle for system meteor");
utils.downloadDevBundle('meteor', function() {
utils.downloadDevBundle(process.env._METEORITE_REAL_METEOR, function() {
// ensure we have dev bundles for all our meteor forks
// XXX:
// for meteor in meteors/
Expand Down Expand Up @@ -62,4 +66,4 @@ beforeEach(function() {
}
}
fs.mkdirSync(appHome);
});
});
5 changes: 3 additions & 2 deletions spec/support/bin/curl
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ var wrench = require('wrench');
var args = process.argv;
var fileNameParts = args[args.length - 1].split('/');
var fileName = fileNameParts[fileNameParts.length - 1];
var REAL_CURL = process.env._METEORITE_REAL_CURL;

console.error('CURL called for ', fileName);

if (! fileName.match(/^dev_bundle_/)) {

// if it's not the dev bundle we are looking for, bail out to standard curl
console.error('skipping...')
spawn('/usr/bin/curl', process.argv.slice(2));
spawn(REAL_CURL, process.argv.slice(2));

} else {
// it's a dev bundle, so cache it
Expand All @@ -35,7 +36,7 @@ if (! fileName.match(/^dev_bundle_/)) {

var url = process.argv[process.argv.length - 1];

var curlCache = spawn('/usr/bin/curl', ['-O', url], { cwd: cachePath }, function() {});
var curlCache = spawn(REAL_CURL, ['-O', url], { cwd: cachePath }, function() {});

curlCache.on('exit', outputFile);

Expand Down
10 changes: 8 additions & 2 deletions spec/support/bin/git
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ var copyRepo = function(from, to) {
spawn('cp', ['-r', from, to]);
};

var REAL_GIT = process.env._METEORITE_REAL_GIT;
if (!REAL_GIT) {
console.error('No _METEORITE_REAL_GIT specified');
process.exit(1);
}

if (command === 'clone') {
var cachePath = path.join(__dirname, '..', 'cache');

Expand All @@ -31,7 +37,7 @@ if (command === 'clone') {
if (!fs.existsSync(repoCachePath)) {
args.push(repoUrl);
args.push(repoCachePath);
var git = spawn('/usr/local/bin/git', args);
var git = spawn(REAL_GIT, args);

git.on('exit', function() {
copyRepo(repoCachePath, destPath);
Expand All @@ -44,7 +50,7 @@ if (command === 'clone') {

} else if (command !== 'pull') { // we will skip pulls, expensive and redudant.

var git = spawn('/usr/local/bin/git', args);
var git = spawn(REAL_GIT, args);

git.stderr.pipe(process.stderr);
git.stdout.pipe(process.stdout);
Expand Down

0 comments on commit bd7c019

Please sign in to comment.