Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

`./config --debug`: node's .default_configuration should be 'Debug' for debug binary, 'Release' for release binary #4299

Closed
wants to merge 1 commit into from

5 participants

@ackalker

NodeJS from Git, configured with (among other things) ./configure --debug, builds both a Debug and a Release build of node.
make install installs the Release build, but node -e 'console.log(process.config.target_defaults.default_configuration)' still gives "Debug" when using the Release build node binary.

From what I've found, this causes node-gyp to build Debug versions of addons unless '-r' option is specified.
This causes problem when npm install builds these addons as dependencies, since no way to specify '-r' option to node-gyp. Not all entry scripts are designed to deal with path to the Debug build of the addon, so using these modules will fail.

Wouldn't it be much more logical to have process.config.target_defaults.default_configuration be:

  • 'Release' for out/Release/node (and the binary installed with make install) and
  • 'Debug` for out/Debug/node

?

@ackalker

See Issue #2571 for reference.
From looking at this issue, I'd quess the solution would be to set default_configuration on each step in {'Debug', 'Release') to match that particular step in the build process.

@TooTallNate
Owner

From looking at this issue, I'd quess the solution would be to set BUILDTYPE on each step in {'Debug', 'Release') to match that particular step in the build process.

No, that still wouldn't fix it. The build process serializes the config.gypi file into a header file as a C string, and that gets compiled into the binary. So we'd need to change that file in between the release and debug build... I haven't yet looked into how hard that would be to do.

@ackalker

ahh... tools/js2c.py. You're right, should have thought of that.

@bnoordhuis

The fact that target_defaults.default_configuration == 'Debug' is not really wrong because, well, it is the default configuration - just not the one that's used to compile the binary.

I guess a fairly straightforward solution is to pass -DBUILDTYPE="$(BUILDTYPE)" to the compiler and act on that in src/node.cc but the question then becomes: where to put that property?

@ackalker

Here's a proof-of-concept to test. Instead of changing node.cc, it uses a Python macro in src/natives_macros.py (new file) to preprocess src/node.js, substituting the current BUILDTYPE.
Yes, I realize that it's not portable (it depends on env to pass an environment variable to js2c.py), but it's enough for testing. It should probably use a constant instead, but that requires generating a macros.py on the fly (or choosing between a debug_macros.py and release_macros.py using a conditional) and passing that to js2c.py.

Perhaps the whole idea of a src/natives_macros.py is worth keeping around for doing build-time configuration of lib/*.js?Please let me know.

@ackalker
$ out/Debug/node -e 'console.log(process.config.target_defaults.default_configuration)'
Debug
$ out/Release/node -e 'console.log(process.config.target_defaults.default_configuration)'
Release
@ackalker

Of couse one could do the same in C++, without the need for passing environment variables around, but I have no experience with accessing Javascript stuff from there.
Also, isn't process.config supposed to be read-only after being initialized?

@ackalker

Nope, after second thought, not currently possible from C++, because the substitution must be done as soon as possible after startup.processConfig, before node.js evaluates any command line -e / --eval stuff or other scripts.

@ackalker ackalker Match node's internal default_configuration with build type.
When building after ./configure --debug , both out/Debug/node and
out/Release/node have
process.config.target_defaults.default_configuration === 'Debug'
which causes node_gyp to build addons in Debug mode.
When `npm install` (which normally uses the Release build of node)
builds such addons as dependencies, some modules will fail to load
because their main script is not designed to use the Debug version
of the addon.
Fix this by setting default_configuration to the build type of the
node binary.
7867f48
@Nodejs-Jenkins

Can one of the admins verify this patch?

@tjfontaine
Owner

We could certainly include this but I question the need to use this information, we could use something like ben suggests. Please open a new PR if you still need this feature.

@tjfontaine tjfontaine closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 24, 2012
  1. @ackalker

    Match node's internal default_configuration with build type.

    ackalker authored
    When building after ./configure --debug , both out/Debug/node and
    out/Release/node have
    process.config.target_defaults.default_configuration === 'Debug'
    which causes node_gyp to build addons in Debug mode.
    When `npm install` (which normally uses the Release build of node)
    builds such addons as dependencies, some modules will fail to load
    because their main script is not designed to use the Debug version
    of the addon.
    Fix this by setting default_configuration to the build type of the
    node binary.
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 0 deletions.
  1. +3 −0  node.gyp
  2. +5 −0 src/natives_macros.py
  3. +1 −0  src/node.js
View
3  node.gyp
@@ -332,6 +332,7 @@
'inputs': [
'<@(library_files)',
'./config.gypi',
+ 'src/natives_macros.py',
],
'outputs': [
'<(SHARED_INTERMEDIATE_DIR)/node_natives.h',
@@ -349,6 +350,8 @@
}]
],
'action': [
+ 'env',
+ 'BUILDTYPE=$(BUILDTYPE)',
'<(python)',
'tools/js2c.py',
'<@(_outputs)',
View
5 src/natives_macros.py
@@ -0,0 +1,5 @@
+# This file is used by tools/js2c.py to preprocess the files in lib/
+
+# Replace BUILDTYPE(x) with the current build type
+# Note: x must be specified, but can be anything.
+python macro BUILDTYPE(x) = repr(os.environ["BUILDTYPE"]);
View
1  src/node.js
@@ -235,6 +235,7 @@
if (value === 'false') return false;
return value;
});
+ process.config.target_defaults.default_configuration = BUILDTYPE('');
};
startup.processMakeCallback = function() {
Something went wrong with that request. Please try again.