Rules_nodejs: make --preserve-symlinks the default

[ppodolsky]

Such things as webpack use __dirname extensively so it can become a problem. The solution is to use --preserve-symlinks flag in nodejs_binary by default but I have no idea if it breaks something else.

[coltonmorris]

+1

[alexeagle]

Can you help me understand what is a problem caused by this?
Also, a snippet of how to use your workaround (is it passed to templated_args?)

[ppodolsky]

@alexeagle Ideologically, it breaks hermeticity of the build. Executing nodejs code suggests that it is placed in your workspace directory, not in the output root. It is the reason why I think --preserve-symlinks will be good as a default option.
Problems arise when js code starts looking for things which must exist in the output root (i.e. collected node_modules) but do not exist in the bazel workspace.
More specific, webpack running from bazel could not find module.rules loaders because it tries to find node_modules directory in the chain of parent directories of webpack.config.js realpath. Without --preserve-symlinks there is no chance that node_modules described in WORKSPACE file will be revealed.

BTW, I have a weak expertise in NodeJS, just trying to integrate it into out bazel repo. Probably, far better solutions exist for the issue.

My workaround is too dirty to be pull-requested - I just patched node_launcher.sh. But I can compose a proper PR in case if we decide it's an appropriate solution.

[alexeagle]

I'm asking for your workaround in case others happen across this issue, so they can apply it as well.

The build should be hermetic, but execution is not. Are you running webpack underneath Bazel (integrated into the toolchain with Skylark) and running bazel build, then seeing this error? Or are you building a program with Bazel and then running it (either directly or with bazel run)? It's hard to be helpful without this context.

[ppodolsky]

Are you running webpack underneath Bazel (integrated into the toolchain with Skylark) and running bazel build, then seeing this error? Or are you building a program with Bazel and then running it (either directly or with bazel run)?

The first case.

I have the following definition for building webpack:

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")


nodejs_binary(
    name = "webpack",
    entry_point = "webpack/bin/webpack.js",
    visibility = ["//visibility:public"],
)

with bzl file like

def _webpack(ctx):
    args = ctx.actions.args()
    args.add(["--display-error-details"])
    args.add(["--config", ctx.file.config.path])

    ctx.actions.run(
        inputs = ctx.files.srcs + [ctx.file.config] + ctx.files.node_modules,
        executable = ctx.executable.webpack,
        outputs = [ctx.outputs.bundle],
        arguments = [args],
        env = {
            "TARGET_PATH": ctx.outputs.bundle.path,
        }
    )
    return struct()

webpack = rule(
    implementation = _webpack,
    attrs = {
        "config": attr.label(allow_files = True, single_file = True),
        "node_modules": attr.label(default = Label("//:node_modules")),
        "srcs": attr.label_list(allow_files = True),
        "webpack": attr.label(
            default = "//bazelisk:webpack",
            executable = True,
            cfg = "host"
        ),
    },
    outputs = {
        "bundle": "www"
    },
)

In details, I have default vue-cli template with pregenerated webpack.conf.js files full of __dirname ops. They resolve into the bazel workplace but not to the output root during building targets

webpack(
    name = 'webpack-files',
    config = 'build/webpack.prod.conf.js',
    srcs = glob([
        'config/**',
        'build/**',
        'src/**',
        'static/**',
        'index.html',
        '.babelrc',
        '.eslintrc.js',
        '.eslintignore',
        '.postcssrc.js',
    ]) + ['//:package.json'],
)

pkg_tar(
    name = 'bin',
    extension = 'tar',
    package_dir = '/usr/lib/...',
    strip_prefix = '.',
    srcs = [
        ':webpack-files',
    ],
    modes = {'init.sh': '0755'},
)

My workaround is

diff --git a/internal/node_launcher.sh b/internal/node_launcher.sh
index dcc66f2..0552ae1 100644
--- a/internal/node_launcher.sh
+++ b/internal/node_launcher.sh
@@ -123,4 +123,4 @@ else
   readonly script="${RUNFILES}/TEMPLATED_script_path"
 fi

-exec "${node}" "${NODE_OPTIONS[@]}" "${script}" "${ARGS[@]}"
+exec "${node}" --preserve-symlinks "${NODE_OPTIONS[@]}" "${script}" "${ARGS[@]}"

In the case above I could be slightly wrong because node_modules is externally managed and exists in the root of workspace. I refused using of bazel-managed node_modules during looking for workaround hence can not provide fully reproducible code for now. But whether node_modules was bazel-managed troubles would happen.

[alexeagle]

I see, you should be able to do

nodejs_binary(
    name = "webpack",
    entry_point = "webpack/bin/webpack.js",
    visibility = ["//visibility:public"],
    templated_args = ["--node_options=--preserve-symlinks"],
)

similarly to how I need to pass the --expose-gc flag when running the typescript compiler
https://github.com/bazelbuild/rules_typescript/blob/eb3244363e1cb265c84e723b347926f28c29aa35/internal/tsc_wrapped/BUILD.bazel#L71-L80

I'll think a bit more about the issue generally before closing. Bazel does run tools against symlinks that point back to the original sources. This is a performance feature, it's faster than copying files around. Generally tools run under bazel need to avoid making assumptions about this, I think your fix is probably the right one for webpack.

[ppodolsky]

I've got it, thank you!

[alexeagle]

Does anyone know a reason why we can't just use --preserve-symlinks for every nodejs_binary execution? That's the simplest resolution.

[ppodolsky]

I think it's the right solution, technically and by spirit. AFAIK, node_modules is all-sufficient directory without any relative links to the outside. Therefore it must be safe to make symlinks to directory and use --preserve-symlinks. Googling around, I have not found any reported problems with copying/symlinking node_modules.

[alexeagle]

Greg is working on rolling out --preserve-symlinks again and dealing with all the breakages

[ppodolsky]

Great news, thank you!