Rules_nodejs: $COMPILATION_MODE is always set to `opt` inside node processes

[joeljeske]

🐞 bug report

Affected Rule

The issue is caused by the rule: nodejs_binary when using @npm//*/bin:*

Is this a regression?

Not that I am aware of

Description

When running any @npm//* bin, the environment var COMPILATION_MODE is always set to opt. I am not sure how this is occurring, since bazel does output to the proper locations, and rebuilds everything when changing the mode. Perhaps since these targets are in another repository bazel changes the mode? Not sure, just guessing.

🔬 Minimal Reproduction

Rollup is a good example of this issue. When running any rollup target with a custom config file, inside your "rollup.config.js" always process.env.COMPILATION_MODE === "opt". This makes it more difficult for me to change the plugin configuration based on the mode.

rollup_bundle(
        name = "rollup",
        entry_points = "index.js",
        config_file = "rollup.config.js",
)

//rollup.config.js
console.log(process.env.COMPILATION_MODE)
export default {
  plugins: []
}

## 🔥 Exception or Error

## 🌍 Your Environment **Operating System:**

  
    macOS 10.14.6
  

Output of bazel version:

  
Build label: 1.2.0
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Wed Nov 20 15:08:12 2019 (1574262492)
Build timestamp: 1574262492
Build timestamp as int: 1574262492
  

Rules version (SHA):

  
8dc1466f8563f3aa4ac7ab7aa3c96651eb7764108219f40b2d1c918e1a81c601
  

Anything else relevant?

[alexeagle]

I reproduced it.

The environment seen by the node process gets set in our loader.sh generated shell script.

Building with -s is always a good first debugging step

```bazel build :bundle -c dbg -s
SUBCOMMAND: # //:bundle [action 'Bundling JavaScript bundle [rollup]', configuration: d5b05cf7c98aa2d5a062d9eac38e5af09fe4c2a9dd3b99299121f6449b1f2338]
(cd /usr/local/google/home/alexeagle/.cache/bazel/_bazel_alexeagle/a4a61376da347577de4fb55dd52f8554/execroot/kotlin_example && \
exec env - \
bazel-out/host/bin/external/npm/rollup/bin/rollup.sh 'bundle=bazel-out/k8-dbg/bin/_bootstrap.js' --output.dir bazel-out/k8-dbg/bin/bundle --format cjs --config bazel-out/k8-dbg/bin/_bundle.rollup_config.js --preserveSymlinks --sourcemap inline '--bazel_node_modules_manifest=bazel-out/k8-dbg/bin/_bundle.module_mappings.json' --nobazel_patch_module_resolver)

So the shell script printed here is where the value comes:

$ grep COMPILATION_MODE bazel-out/host/bin/external/npm/rollup/bin/rollup.sh
export COMPILATION_MODE="opt"
```

This gets set up when we generated the shell wrapper for the rollup target. This is because the -c flag affects the build outputs for the target architecture, but not the tools built to run on the host architecture (you have to think of it from a C++ cross-compilation point of view, because that's how the compilation_mode feature was originally modelled)

This line is where we determine that:
https://github.com/bazelbuild/rules_nodejs/blob/0.42.3/packages/rollup/src/rollup_bundle.bzl#L147

If you flip this to cfg = "target" instead, you'll run the rollup binary in the target architecture which will print COMPILATION_MODE = dbg from inside the node process instead.

I don't understand this distinction well enough to say whether we should make that change. I will try to figure it out (I think rollup always runs on the build machine so cfg="host" should be correct) but you can certainly patch it locally to get this behavior.

Note that the terser_minified rule reads the COMPILATION_MODE variable from starlark code before invoking node, so it has a different way to propagate this bit into the config file. See https://github.com/bazelbuild/rules_nodejs/blob/master/docs/Terser.md#config_file
We could follow that model for rollup as another way to propagate this value, but I don't like how you have to read docs to know about this feature.

[alexeagle]

Discussed with @tjgq on my team. cfg=host is indeed the right setting for executing a tool, it should be compiled for the local machine where actions are executed, and this is set to opt regardless of the debug setting for the host platform. So the right solution here is to dynamically pass this value from the surrounding environment where the tool is executed rather than to template the value into the binary at the time it's built.

[joeljeske]

Thank you for the response and explaining this.

I’m a bit confused on your suggestion. How might I pass the value dynamically? I am trying to read the env var. or perhaps are you mentioning a change that might need to occur in your codebase?

What would be the recommended way to use the compilation mode inside the configuration file?

[alexeagle]

Sorry, too rushed.
Passing the value dynamically is the right fix in our code, but I don't know immediately where that belongs.
Your workaround for now is to create a patch file for the @bazel/rollup package that adds cfg="target" at that spot I pointed to in rollup_bundle. see https://github.com/bazelbuild/rules_nodejs/blob/master/docs/index.md#patching-the-npm-packages

[alexeagle]

FYI @ashi009 who added the COMPILATION_MODE feature

[gregmagolan]

I sorted it out in https://github.com/bazelbuild/rules_nodejs/pull/1521 which resolves this issue :)