diff --git a/e2e/packages/WORKSPACE b/e2e/packages/WORKSPACE index c1f9d641b2..eff4e97037 100644 --- a/e2e/packages/WORKSPACE +++ b/e2e/packages/WORKSPACE @@ -17,16 +17,16 @@ node_repositories() npm_install( name = "e2e_packages_npm_install", + args = ["--production"], data = ["//:postinstall.js"], package_json = "//:npm1/package.json", package_lock_json = "//:npm1/package-lock.json", - # Just here as a smoke test for this attribute - prod_only = True, symlink_node_modules = False, ) npm_install( name = "e2e_packages_npm_install_duplicate_for_determinism_testing", + args = ["--production"], data = ["//:postinstall.js"], package_json = "//:npm2/package.json", package_lock_json = "//:npm2/package-lock.json", @@ -35,6 +35,7 @@ npm_install( yarn_install( name = "e2e_packages_yarn_install", + args = ["--prod"], data = ["//:postinstall.js"], package_json = "//:yarn1/package.json", symlink_node_modules = False, @@ -43,6 +44,7 @@ yarn_install( yarn_install( name = "e2e_packages_yarn_install_duplicate_for_determinism_testing", + args = ["--prod"], data = ["//:postinstall.js"], package_json = "//:yarn2/package.json", symlink_node_modules = False, diff --git a/e2e/packages/npm1/package.json b/e2e/packages/npm1/package.json index 5bff01fd1d..396c0aeb47 100644 --- a/e2e/packages/npm1/package.json +++ b/e2e/packages/npm1/package.json @@ -4,6 +4,9 @@ "jsesc": "~1.2.0", "jasmine": "2.8.0" }, + "devDependencies": { + "tmp": "0.1.0" + }, "scripts": { "postinstall": "node ./postinstall.js" } diff --git a/e2e/packages/npm2/package.json b/e2e/packages/npm2/package.json index 5bff01fd1d..396c0aeb47 100644 --- a/e2e/packages/npm2/package.json +++ b/e2e/packages/npm2/package.json @@ -4,6 +4,9 @@ "jsesc": "~1.2.0", "jasmine": "2.8.0" }, + "devDependencies": { + "tmp": "0.1.0" + }, "scripts": { "postinstall": "node ./postinstall.js" } diff --git a/e2e/packages/npm_determinism.spec.js b/e2e/packages/npm_determinism.spec.js index e7504730d6..a6e08dfdc5 100644 --- a/e2e/packages/npm_determinism.spec.js +++ b/e2e/packages/npm_determinism.spec.js @@ -8,6 +8,13 @@ const packageJsonPath2 = packageJsonPath.replace( '/e2e_packages_npm_install_duplicate_for_determinism_testing/', '/e2e_packages_npm_install/'); const packageJson2 = JSON.parse(fs.readFileSync(packageJsonPath2)); +try { + require.resolve('tmp'); + console.error( + 'expected tmp to not be installed by npm as --production was passed via args in npm_install'); + process.exitCode = 1; +} catch (_) { +} if (packageJsonPath === packageJsonPath2) { console.error('expected different json paths'); diff --git a/e2e/packages/yarn1/package.json b/e2e/packages/yarn1/package.json index 5bff01fd1d..396c0aeb47 100644 --- a/e2e/packages/yarn1/package.json +++ b/e2e/packages/yarn1/package.json @@ -4,6 +4,9 @@ "jsesc": "~1.2.0", "jasmine": "2.8.0" }, + "devDependencies": { + "tmp": "0.1.0" + }, "scripts": { "postinstall": "node ./postinstall.js" } diff --git a/e2e/packages/yarn2/package.json b/e2e/packages/yarn2/package.json index 5bff01fd1d..396c0aeb47 100644 --- a/e2e/packages/yarn2/package.json +++ b/e2e/packages/yarn2/package.json @@ -4,6 +4,9 @@ "jsesc": "~1.2.0", "jasmine": "2.8.0" }, + "devDependencies": { + "tmp": "0.1.0" + }, "scripts": { "postinstall": "node ./postinstall.js" } diff --git a/e2e/packages/yarn_determinism.spec.js b/e2e/packages/yarn_determinism.spec.js index c2b6814dab..281a753d6d 100644 --- a/e2e/packages/yarn_determinism.spec.js +++ b/e2e/packages/yarn_determinism.spec.js @@ -8,6 +8,14 @@ const packageJsonPath2 = packageJsonPath.replace( '/e2e_packages_yarn_install_duplicate_for_determinism_testing/', '/e2e_packages_yarn_install/'); const packageJson2 = JSON.parse(fs.readFileSync(packageJsonPath2)); +try { + require.resolve('tmp'); + console.error( + 'expected tmp to not be installed by yarn as --prod was passed via args in yarn_install'); + process.exitCode = 1; +} catch (_) { +} + if (packageJsonPath === packageJsonPath2) { console.error('expected different json paths'); process.exitCode = 1; diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index 6ea88ac9af..a06d3f8d9a 100644 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -26,6 +26,10 @@ load("//internal/common:os_name.bzl", "is_windows_os") load("//internal/node:node_labels.bzl", "get_node_label", "get_npm_label", "get_yarn_label") COMMON_ATTRIBUTES = dict(dict(), **{ + "timeout": attr.int( + default = 3600, + doc = """Maximum duration of the package manager execution in seconds.""", + ), "always_hide_bazel_files": attr.bool( doc = """Always hide Bazel build files such as `BUILD` and BUILD.bazel` by prefixing them with `_`. @@ -101,10 +105,6 @@ fine grained npm dependencies. mandatory = True, allow_single_file = True, ), - "prod_only": attr.bool( - default = False, - doc = "Don't install devDependencies", - ), "quiet": attr.bool( default = True, doc = "If stdout and stderr should be printed to the terminal.", @@ -207,10 +207,7 @@ 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"] - - if repository_ctx.attr.prod_only: - npm_args.append("--production") + npm_args = ["install"] + 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 @@ -294,10 +291,11 @@ cd "{root}" && "{npm}" {npm_args} npm_install = repository_rule( attrs = dict(COMMON_ATTRIBUTES, **{ - "timeout": attr.int( - default = 3600, - doc = """Maximum duration of the command "npm install" in seconds - (default is 3600 seconds).""", + "args": attr.string_list( + doc = """Arguments passed to npm install. + +See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of supported arguments.""", + default = [], ), "package_lock_json": attr.label( mandatory = True, @@ -345,15 +343,8 @@ def _yarn_install_impl(repository_ctx): repository_ctx.path(yarn), "--cwd", root, - "--network-timeout", - str(repository_ctx.attr.network_timeout * 1000), # in ms ] - if repository_ctx.attr.frozen_lockfile: - args.append("--frozen-lockfile") - - if repository_ctx.attr.prod_only: - args.append("--prod") if not repository_ctx.attr.use_global_yarn_cache: args.extend(["--cache-folder", repository_ctx.path("_yarn_cache")]) else: @@ -365,6 +356,8 @@ def _yarn_install_impl(repository_ctx): # artifacts somewhere, so we rely on yarn to be correct. args.extend(["--mutex", "network"]) + args.extend(repository_ctx.attr.args) + repository_ctx.report_progress("Running yarn install on %s" % repository_ctx.attr.package_json) result = repository_ctx.execute( args, @@ -381,23 +374,11 @@ def _yarn_install_impl(repository_ctx): yarn_install = repository_rule( attrs = dict(COMMON_ATTRIBUTES, **{ - "timeout": attr.int( - default = 3600, - doc = """Maximum duration of the command "yarn install" in seconds - (default is 3600 seconds).""", - ), - "frozen_lockfile": attr.bool( - default = False, - doc = """Passes the --frozen-lockfile flag to prevent updating yarn.lock. + "args": attr.string_list( + doc = """Arguments passed to yarn install. -Note that enabling this option will require that you run yarn outside of Bazel -when making changes to package.json. -""", - ), - "network_timeout": attr.int( - default = 300, - doc = """Maximum duration of a network request made by yarn in seconds - (default is 300 seconds).""", +See yarn CLI docs https://yarnpkg.com/en/docs/cli/install for complete list of supported arguments.""", + default = [], ), "use_global_yarn_cache": attr.bool( default = True, @@ -406,8 +387,15 @@ when making changes to package.json. The cache lets you avoid downloading packages multiple times. However, it can introduce non-hermeticity, and the yarn cache can have bugs. + Disabling this attribute causes every run of yarn to have a unique cache_directory. + +If True, this rule will pass `--mutex network` to yarn to ensure that +the global cache can be shared by parallelized yarn_install rules. + +If False, this rule will pass `--cache-folder /path/to/external/repository/__yarn_cache` +to yarn so that the local cache is contained within the external repository. """, ), "yarn_lock": attr.label(