Skip to content

Commit

Permalink
Remove logic for writing package.json files for binary npm packages.
Browse files Browse the repository at this point in the history
We're going to be using programs/server/setup.sh to run `npm rebuild` in
these packages, which is a much cleaner solution for meteor#6537.
  • Loading branch information
Ben Newman committed Apr 7, 2016
1 parent 91244f3 commit 2dd511f
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 107 deletions.
1 change: 0 additions & 1 deletion tools/cli/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,6 @@ ${Console.command("meteor build ../output")}`,
serverArch: bundleArch,
buildMode: options.debug ? 'development' : 'production',
},
providePackageJSONForUnavailableBinaryDeps: !!process.env.METEOR_BINARY_DEP_WORKAROUND,
});
if (bundleResult.errors) {
Console.error("Errors prevented bundling:");
Expand Down
86 changes: 7 additions & 79 deletions tools/isobuild/bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ export class NodeModulesDirectory {
preferredBundlePath,
local = false,
npmDiscards = null,
writePackageJSON = false,
}) {
// Name of the package this node_modules directory belongs to, or null
// if it belongs to an application.
Expand All @@ -250,9 +249,6 @@ export class NodeModulesDirectory {

// Optionally, files to discard.
this.npmDiscards = npmDiscards;

// Write a package.json file instead of copying the full directory.
this.writePackageJSON = writePackageJSON;
}

copy() {
Expand Down Expand Up @@ -315,7 +311,6 @@ export class NodeModulesDirectory {
toJSON() {
return {
packageName: this.packageName,
writePackageJSON: this.writePackageJSON,
local: this.local,
};
}
Expand Down Expand Up @@ -597,8 +592,6 @@ class Target {
buildMode,
// directory on disk where to store the cache for things like linker
bundlerCacheDir,
// whether to substitute a package.json for unavailable binary deps
providePackageJSONForUnavailableBinaryDeps,
// ... see subclasses for additional options
}) {
this.packageMap = packageMap;
Expand Down Expand Up @@ -647,9 +640,6 @@ class Target {
this.buildMode = buildMode || 'production';

this.bundlerCacheDir = bundlerCacheDir;

this.providePackageJSONForUnavailableBinaryDeps
= providePackageJSONForUnavailableBinaryDeps;
}

// Top-level entry point for building a target. Generally to build a
Expand Down Expand Up @@ -755,9 +745,7 @@ class Target {
if (p.testOnly && this.buildMode !== 'test') {
return;
}
const unibuild = p.getUnibuildAtArch(this.arch, {
allowWrongPlatform: this.providePackageJSONForUnavailableBinaryDeps
});
const unibuild = p.getUnibuildAtArch(this.arch);
unibuild && rootUnibuilds.push(unibuild);
});

Expand Down Expand Up @@ -794,7 +782,6 @@ class Target {
skipDebugOnly: this.buildMode === 'production',
skipProdOnly: this.buildMode !== 'production',
skipTestOnly: this.buildMode !== 'test',
allowWrongPlatform: this.providePackageJSONForUnavailableBinaryDeps,
}, addToGetsUsed);
}.bind(this);

Expand Down Expand Up @@ -864,7 +851,6 @@ class Target {
skipDebugOnly: this.buildMode === 'production',
skipProdOnly: this.buildMode !== 'production',
skipTestOnly: this.buildMode !== 'test',
allowWrongPlatform: this.providePackageJSONForUnavailableBinaryDeps,
}, processUnibuild);
this.unibuilds.push(unibuild);
delete needed[unibuild.id];
Expand Down Expand Up @@ -1003,35 +989,6 @@ class Target {
}

_.each(unibuild.nodeModulesDirectories, nmd => {
if (!archinfo.matches(this.arch, unibuild.arch)) {
// The unibuild we're trying to include doesn't work for the
// bundle target (eg, os.osx.x86_64 instead of os.linux.x86_64)!
// Hopefully this is because we specially enabled the feature
// that leads to this.
if (!this.providePackageJSONForUnavailableBinaryDeps) {
throw Error("mismatched arch without special feature enabled "
+ unibuild.pkg.name + " / " + this.arch + " / " +
unibuild.arch);
}
if (!files.exists(
files.pathJoin(nmd.sourcePath, '.package.json'))) {
buildmessage.error(
"Can't cross-compile package " +
unibuild.pkg.name + ": missing .package.json");
return;
}
if (!files.exists(
files.pathJoin(nmd.sourcePath, '.npm-shrinkwrap.json'))) {
buildmessage.error(
"Can't cross-compile package " +
unibuild.pkg.name + ": missing .npm-shrinkwrap.json");
return;
}

nmd = nmd.copy();
nmd.writePackageJSON = true;
}

this.nodeModulesDirectories[nmd.sourcePath] = nmd;
f.nodeModulesDirectories[nmd.sourcePath] = nmd;
});
Expand Down Expand Up @@ -1213,14 +1170,11 @@ class Target {
// including anything that was specific to Linux, the return value
// would be 'os'.
mostCompatibleArch() {
let arches = _.pluck(this.unibuilds, 'arch');
if (this.providePackageJSONForUnavailableBinaryDeps) {
// Filter out incompatible arches.
arches = arches.filter(
(arch) => archinfo.matches(this.arch, arch)
);
}
return archinfo.leastSpecificDescription(arches);
return archinfo.leastSpecificDescription(
_.pluck(this.unibuilds, 'arch').filter(
arch => archinfo.matches(this.arch, arch)
)
);
}
}

Expand Down Expand Up @@ -1468,8 +1422,6 @@ class JsImage {

// Architecture required by this image
this.arch = null;

this.providePackageJSONForUnavailableBinaryDeps = false;
}

// Load the image into the current process. It gets its own unique
Expand Down Expand Up @@ -1880,29 +1832,7 @@ class JsImage {
rebuildDirs[parentDir] = parentDir;
}

if (nmd.writePackageJSON) {
// Make sure there's an empty node_modules directory at the right place
// in the tree (so that npm install puts modules there instead of
// elsewhere).
builder.reserve(
nmd.preferredBundlePath, {directory: true});
// We check that these source files exist in _emitResources when
// writePackageJSON is initially set.
builder.write(
files.pathJoin(files.pathDirname(nmd.preferredBundlePath),
'package.json'),
{ file: files.pathJoin(nmd.sourcePath, '.package.json') }
);
builder.write(
files.pathJoin(files.pathDirname(nmd.preferredBundlePath),
'npm-shrinkwrap.json'),
{ file: files.pathJoin(nmd.sourcePath, '.npm-shrinkwrap.json') }
);
// XXX does not support npmDiscards!

setupScriptPieces.push(
'(cd ', nmd.preferredBundlePath, ' && npm install)\n');
} else if (nmd.sourcePath !== nmd.preferredBundlePath) {
if (nmd.sourcePath !== nmd.preferredBundlePath) {
builder.copyDirectory({
from: nmd.sourcePath,
to: nmd.preferredBundlePath,
Expand Down Expand Up @@ -2456,7 +2386,6 @@ exports.bundle = function ({
buildOptions,
previousBuilders,
hasCachedBundle,
providePackageJSONForUnavailableBinaryDeps
}) {
buildOptions = buildOptions || {};

Expand Down Expand Up @@ -2540,7 +2469,6 @@ exports.bundle = function ({
releaseName: releaseName,
appIdentifier: appIdentifier,
buildMode: buildOptions.buildMode,
providePackageJSONForUnavailableBinaryDeps
};
if (clientTargets) {
targetOptions.clientTargets = clientTargets;
Expand Down
3 changes: 0 additions & 3 deletions tools/isobuild/compiler-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,6 @@ export class PackageSourceBatch {
skipDebugOnly: true,
skipProdOnly: true,
skipTestOnly: true,
// We only care about getting exports here, so it's OK if we get the Mac
// version when we're bundling for Linux.
allowWrongPlatform: true,
}, depUnibuild => {
_.each(depUnibuild.declaredExports, function (symbol) {
// Slightly hacky implementation of test-only exports.
Expand Down
6 changes: 1 addition & 5 deletions tools/isobuild/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,6 @@ function runLinters({inputSourceArch, isopackCache, sources,
skipDebugOnly: true,
skipProdOnly: true,
skipTestOnly: true,
// We only care about getting exports here, so it's OK if we get the Mac
// version when we're bundling for Linux.
allowWrongPlatform: true,
}, (unibuild) => {
if (unibuild.pkg.name === inputSourceArch.pkg.name) {
return;
Expand Down Expand Up @@ -866,7 +863,6 @@ compiler.eachUsedUnibuild = function (
var dependencies = options.dependencies;
var arch = options.arch;
var isopackCache = options.isopackCache;
const allowWrongPlatform = options.allowWrongPlatform;

var acceptableWeakPackages = options.acceptableWeakPackages || {};

Expand Down Expand Up @@ -906,7 +902,7 @@ compiler.eachUsedUnibuild = function (
continue;
}

var unibuild = usedPackage.getUnibuildAtArch(arch, {allowWrongPlatform});
var unibuild = usedPackage.getUnibuildAtArch(arch);
if (!unibuild) {
// The package exists but there's no unibuild for us. A buildmessage has
// already been issued. Recover by skipping.
Expand Down
15 changes: 6 additions & 9 deletions tools/isobuild/isopack.js
Original file line number Diff line number Diff line change
Expand Up @@ -460,19 +460,16 @@ _.extend(Isopack.prototype, {
// Return the unibuild of the package to use for a given target architecture
// (eg, 'os.linux.x86_64' or 'web'), or throw an exception if that
// packages can't be loaded under these circumstances.
getUnibuildAtArch: Profile("Isopack#getUnibuildAtArch", function (
arch, {allowWrongPlatform} = {}) {
getUnibuildAtArch: Profile("Isopack#getUnibuildAtArch", function (arch) {
var self = this;

let chosenArch = archinfo.mostSpecificMatch(
arch, _.pluck(self.unibuilds, 'arch'));
if (! chosenArch && allowWrongPlatform && arch.match(/^os\./)) {
// Special-case: we're looking for a specific server platform and it's
// not available. (eg, we're deploying from a Mac to Linux and are
// processing a local package with binary npm deps). If we have "allow
// wrong platform" turned on, search again for the host version, which
// might find the Mac version. We'll detect this case later and provide
// package.json instead of Mac binaries.
if (! chosenArch && arch.match(/^os\./)) {
// Special-case: we're looking for a specific server platform and
// it's not available. (eg, we're deploying from a Mac to Linux and
// are processing a local package with binary npm deps). Search
// again for the host version, which might find the Mac version.
chosenArch =
archinfo.mostSpecificMatch(archinfo.host(), _.pluck(self.unibuilds, 'arch'));
}
Expand Down
9 changes: 0 additions & 9 deletions tools/isobuild/meteor-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,6 @@ var updateExistingNpmDirectory = function (packageName, newPackageNpmDir,
}
}

// If the node modules directory exists but doesn't have .package.json and
// .npm-shrinkwrap.json, recreate. This is to ensure that
// providePackageJSONForUnavailableBinaryDeps works.
if (files.exists(nodeModulesDir) &&
(!files.exists(files.pathJoin(nodeModulesDir, '.package.json')) ||
!files.exists(files.pathJoin(nodeModulesDir, '.npm-shrinkwrap.json')))) {
files.rm_recursive(nodeModulesDir);
}

// Make sure node_modules is present (fix for #1761). Prevents npm install
// from installing to an existing node_modules dir higher up in the
// filesystem. node_modules may be absent due to a change in Node version or
Expand Down
1 change: 0 additions & 1 deletion tools/meteor-services/deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ var bundleAndDeploy = function (options) {
projectContext: options.projectContext,
outputPath: bundlePath,
buildOptions: options.buildOptions,
providePackageJSONForUnavailableBinaryDeps: !!process.env.METEOR_BINARY_DEP_WORKAROUND,
});

if (bundleResult.errors) {
Expand Down

0 comments on commit 2dd511f

Please sign in to comment.