From 887c496c976be99fd2fc9bfd176b559ebda9203a Mon Sep 17 00:00:00 2001 From: Lukas Holzer Date: Wed, 2 Dec 2020 23:23:42 +0100 Subject: [PATCH] feat(builtin): use npm ci as default behaviour for installing node_modules To be more hermetic with the install of the dependencies use npm ci to install the exact version from the package-lock.json file. To update a dependency use the vendored npm binary with `bazel run @nodejs//:npm install `. Fixes #159 --- WORKSPACE | 1 + e2e/packages/WORKSPACE | 2 ++ e2e/symlinked_node_modules_npm/WORKSPACE | 1 + internal/bazel_integration_test/test_runner.js | 5 +++++ internal/npm_install/npm_install.bzl | 11 ++++++++++- 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/WORKSPACE b/WORKSPACE index 8dd222bd2b..c771e31c4e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -214,6 +214,7 @@ npm_install( ".json", ".proto", ], + npm_command = "install", package_json = "//:tools/fine_grained_deps_npm/package.json", package_lock_json = "//:tools/fine_grained_deps_npm/package-lock.json", symlink_node_modules = False, diff --git a/e2e/packages/WORKSPACE b/e2e/packages/WORKSPACE index a14e377bec..cea6b66ca0 100644 --- a/e2e/packages/WORKSPACE +++ b/e2e/packages/WORKSPACE @@ -19,6 +19,7 @@ npm_install( name = "e2e_packages_npm_install", args = ["--production"], data = ["//:postinstall.js"], + npm_command = "install", package_json = "//:npm1/package.json", package_lock_json = "//:npm1/package-lock.json", symlink_node_modules = False, @@ -28,6 +29,7 @@ npm_install( name = "e2e_packages_npm_install_duplicate_for_determinism_testing", args = ["--production"], data = ["//:postinstall.js"], + npm_command = "install", package_json = "//:npm2/package.json", package_lock_json = "//:npm2/package-lock.json", symlink_node_modules = False, diff --git a/e2e/symlinked_node_modules_npm/WORKSPACE b/e2e/symlinked_node_modules_npm/WORKSPACE index f7f3439b77..a6f0dbb158 100644 --- a/e2e/symlinked_node_modules_npm/WORKSPACE +++ b/e2e/symlinked_node_modules_npm/WORKSPACE @@ -28,6 +28,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "npm_install") npm_install( name = "npm", + npm_command = "install", package_json = "//:package.json", package_lock_json = "//:package-lock.json", quiet = False, diff --git a/internal/bazel_integration_test/test_runner.js b/internal/bazel_integration_test/test_runner.js index 07942101e9..00824481b7 100644 --- a/internal/bazel_integration_test/test_runner.js +++ b/internal/bazel_integration_test/test_runner.js @@ -226,6 +226,11 @@ if (config.bazelrcAppend) { workspaceContents = workspaceContents.replace(/(yarn_lock[\s\S]+?,)/gm, 'frozen_lockfile = False,\n $1') + // We have to use npm install in favour of npm ci as the package-lock.json would not match the + // replaced version + workspaceContents = workspaceContents.replace( + /(package_lock_json[\s\S]+?,)/gm, 'npm_command = "install",\n $1') + if (!workspaceContents.includes(archiveFile)) { console.error( `bazel_integration_test: WORKSPACE replacement for repository ${repositoryKey} failed!`) diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index 6e48d79f8a..b2c702c406 100644 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -206,7 +206,11 @@ def _npm_install_impl(repository_ctx): is_windows_host = is_windows_os(repository_ctx) node = repository_ctx.path(get_node_label(repository_ctx)) npm = get_npm_label(repository_ctx) - npm_args = ["install"] + repository_ctx.attr.args + + # Set the base command (install or ci) + npm_args = [repository_ctx.attr.npm_command] + + npm_args.extend(repository_ctx.attr.args) # If symlink_node_modules is true then run the package manager # in the package.json folder; otherwise, run it in the root of @@ -303,6 +307,11 @@ npm_install = repository_rule( See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of supported arguments.""", default = [], ), + "npm_command": attr.string( + default = "ci", + doc = "The npm command to run, to install dependencies.", + values = ["ci", "install"], + ), "package_lock_json": attr.label( mandatory = True, allow_single_file = True,