diff --git a/internal/common/preserve_legacy_rlocation.bzl b/internal/common/preserve_legacy_rlocation.bzl new file mode 100644 index 0000000000..b9f9beba11 --- /dev/null +++ b/internal/common/preserve_legacy_rlocation.bzl @@ -0,0 +1,36 @@ +# Copyright 2017 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Helper functions to preserve legacy `$(rlocation ` usage +""" + +def preserve_legacy_rlocation(input): + """Converts legacy `$(rlocation ` to `$(rlocation ` which is preserved when expanding make + variables with ctx.expand_make_variables. + + Args: + input: String to be modified + + Returns: + The modified string + """ + result = "" + length = len(input) + for i in range(length): + if input[i:].startswith("$(rlocation "): + if i == 0 or input[i - 1] != "$": + # insert an additional "$" + result += "$" + result += input[i] + return result diff --git a/internal/common/test/BUILD.bazel b/internal/common/test/BUILD.bazel index 71312fa0ce..1ef00fba1c 100644 --- a/internal/common/test/BUILD.bazel +++ b/internal/common/test/BUILD.bazel @@ -77,3 +77,7 @@ sh_test( load(":expand_into_runfiles_test.bzl", "expand_into_runfiles_test_suite") expand_into_runfiles_test_suite() + +load(":preserve_legacy_rlocation_test.bzl", "preserve_legacy_rlocation_test_suite") + +preserve_legacy_rlocation_test_suite() diff --git a/internal/common/test/preserve_legacy_rlocation_test.bzl b/internal/common/test/preserve_legacy_rlocation_test.bzl new file mode 100644 index 0000000000..6d40d6c0b5 --- /dev/null +++ b/internal/common/test/preserve_legacy_rlocation_test.bzl @@ -0,0 +1,31 @@ +"Unit tests for //internal/common:preserve_legacy_rlocation.bzl" + +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//internal/common:preserve_legacy_rlocation.bzl", "preserve_legacy_rlocation") + +def _impl(ctx): + env = unittest.begin(ctx) + + conversions = { + "$$(rlocation foobar)": "$$(rlocation foobar)", + "$(rlocation foobar)": "$$(rlocation foobar)", + "$(rlocation! foobar)": "$(rlocation! foobar)", + } + + for key in conversions: + asserts.equals(env, "%s" % conversions[key], preserve_legacy_rlocation("%s" % key)) + asserts.equals(env, " %s " % conversions[key], preserve_legacy_rlocation(" %s " % key)) + asserts.equals(env, "%s%s" % (conversions[key], conversions[key]), preserve_legacy_rlocation("%s%s" % (key, key))) + asserts.equals(env, "%s %s" % (conversions[key], conversions[key]), preserve_legacy_rlocation("%s %s" % (key, key))) + asserts.equals(env, " %s %s " % (conversions[key], conversions[key]), preserve_legacy_rlocation(" %s %s " % (key, key))) + asserts.equals(env, "a%sb%sc" % (conversions[key], conversions[key]), preserve_legacy_rlocation("a%sb%sc" % (key, key))) + + return unittest.end(env) + +preserve_legacy_rlocation_test = unittest.make( + impl = _impl, + attrs = {}, +) + +def preserve_legacy_rlocation_test_suite(): + unittest.suite("preserve_legacy_rlocation_tests", preserve_legacy_rlocation_test) diff --git a/internal/node/node.bzl b/internal/node/node.bzl index c1c5843499..b8cefce868 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -24,6 +24,7 @@ load("//:providers.bzl", "JSNamedModuleInfo", "NodeRuntimeDepsInfo", "NpmPackage load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles") load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect") load("//internal/common:path_utils.bzl", "strip_external") +load("//internal/common:preserve_legacy_rlocation.bzl", "preserve_legacy_rlocation") load("//internal/common:windows_utils.bzl", "create_windows_native_launcher_script", "is_windows") load("//internal/linker:link_node_modules.bzl", "module_mappings_aspect", "write_node_modules_manifest") load("//internal/node:node_repositories.bzl", "BUILT_IN_NODE_PLATFORMS") @@ -211,9 +212,13 @@ def _nodejs_binary_impl(ctx): is_builtin = ctx.attr._node.label.workspace_name in ["nodejs_%s" % p for p in BUILT_IN_NODE_PLATFORMS] + # First replace any instances of "$(rlocation " with "$$(rlocation " to preserve + # legacy uses of "$(rlocation" + expanded_args = [preserve_legacy_rlocation(a) for a in ctx.attr.templated_args] + # First expand predefined source/output path variables: # $(execpath), $(rootpath) & legacy $(location) - expanded_args = [expand_location_into_runfiles(ctx, a, ctx.attr.data) for a in ctx.attr.templated_args] + expanded_args = [expand_location_into_runfiles(ctx, a, ctx.attr.data) for a in expanded_args] # Next expand predefined variables & custom variables expanded_args = [ctx.expand_make_variables("templated_args", e, {}) for e in expanded_args] @@ -510,7 +515,10 @@ of being expanded. `$(rlocation)` is then evaluated by the bash node launcher sc the `rlocation` function in the runfiles.bash helper. For example, the templated arg `$$(rlocation $(rootpath //:some_file))` is expanded by Bazel to `$(rlocation ./some_file)` which is then converted in bash to the absolute path of `//:some_file` in runfiles by the runfiles.bash helper -before being passed as an argument to the program +before being passed as an argument to the program. + +NB: nodejs_binary and nodejs_test will preserve the legacy behavior of `$(rlocation)` so users don't +need to update to `$$(rlocation)`. This may be changed in the future. 2. Subject to predefined variables & custom variable substitutions. diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel index 484f3139e3..76eb196c12 100644 --- a/internal/node/test/BUILD.bazel +++ b/internal/node/test/BUILD.bazel @@ -111,7 +111,9 @@ nodejs_test( "@npm//tmp", ], entry_point = ":bootstrap.spec.js", - templated_args = ["--node_options=--require=$$(rlocation $(location :bootstrap.js))"], + # Ensure that legacy "$(rlocation //:foobar)" usage is preserved so that users + # don't have to upgrade to "$$(rlocation //:foobar)" + templated_args = ["--node_options=--require=$(rlocation $(location :bootstrap.js))"], ) nodejs_test(