Skip to content

Commit

Permalink
fix: don't double copy 3rd party packages when linking with no lifecy…
Browse files Browse the repository at this point in the history
…cle hook (#98)
  • Loading branch information
gregmagolan authored May 11, 2022
1 parent 95868fc commit 5b6bd3d
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
10 changes: 7 additions & 3 deletions js/private/js_package.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,13 @@ def _impl(ctx):

if ctx.attr.src:
if ctx.file.src.is_source:
# copy the source directory to a TreeArtifact
directory = ctx.actions.declare_directory(ctx.attr.name)
copy_directory_action(ctx, ctx.file.src, directory, is_windows = is_windows)
if getattr(ctx.attr, "provide_source_directory", False):
# pass the source directory through; for rules_js internal use only
directory = ctx.file.src
else:
# copy the source directory to a TreeArtifact
directory = ctx.actions.declare_directory(ctx.attr.name)
copy_directory_action(ctx, ctx.file.src, directory, is_windows = is_windows)
elif ctx.file.src.is_directory:
# pass-through since src is already a TreeArtifact
directory = ctx.file.src
Expand Down
18 changes: 18 additions & 0 deletions js/private/js_package_internal.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
"js_package_internal rule"

load("@bazel_skylib//lib:dicts.bzl", "dicts")
load(":js_package.bzl", _js_package_lib = "js_package_lib")

_INTERNAL_ATTRS = dicts.add(_js_package_lib.attrs, {
"provide_source_directory": attr.bool(
doc = """If true, source directories are provided and not copied to the output tree.
For internal rules_js use only.""",
),
})

js_package_internal = rule(
implementation = _js_package_lib.implementation,
attrs = _INTERNAL_ATTRS,
provides = _js_package_lib.provides,
)
19 changes: 15 additions & 4 deletions js/private/npm_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ def link_js_package(fail_if_no_link = False):
_run_js_binary(
name = "{lifecycle_target_name}",
srcs = [
"{js_package_target}",
"{js_package_target_lc}",
":{namespace}{bazel_name}__pkg_lc"
],
# run_js_binary runs in the output dir; must add "../../../" because paths are relative to the exec root
args = [
"{package}",
"../../../$(execpath {js_package_target})",
"../../../$(execpath {js_package_target_lc})",
"../../../$(@D)",
],
copy_srcs_to_bin = False,
Expand Down Expand Up @@ -185,6 +185,15 @@ def {bin_name}_binary(name, **kwargs):
"""

_JS_PACKAGE_TMPL = """
_js_package(
name = "jsp_source_directory",
src = ":{extract_to_dirname}",
provide_source_directory = True,
package = "{package}",
version = "{version}",
visibility = ["//visibility:public"],
)
_js_package(
name = "jsp",
src = ":{extract_to_dirname}",
Expand Down Expand Up @@ -282,7 +291,7 @@ def _impl(rctx):
patch(rctx, patch_args = rctx.attr.patch_args, patch_directory = _EXTRACT_TO_DIRNAME)

build_file = generated_by_lines + [
"""load("@aspect_rules_js//js:defs.bzl", _js_package = "js_package")""",
"""load("@aspect_rules_js//js/private:js_package_internal.bzl", _js_package = "js_package_internal")""",
]

build_file.append(_JS_PACKAGE_TMPL.format(
Expand Down Expand Up @@ -352,7 +361,8 @@ def _impl_links(rctx):
# strip _links post-fix to get the repository name of the npm sources
npm_import_sources_repo_name = rctx.name[:-len(pnpm_utils.links_postfix)]

js_package_target = "@{}//:jsp".format(npm_import_sources_repo_name)
js_package_target = "@{}//:jsp_source_directory".format(npm_import_sources_repo_name)
js_package_target_lc = "@{}//:jsp".format(npm_import_sources_repo_name)

link_js_package_bzl = [_LINK_JS_PACKAGE_TMPL.format(
alias = pnpm_utils.bazel_name(rctx.attr.package),
Expand All @@ -361,6 +371,7 @@ def _impl_links(rctx):
dir_postfix = pnpm_utils.dir_postfix,
extract_to_dirname = _EXTRACT_TO_DIRNAME,
js_package_target = js_package_target,
js_package_target_lc = js_package_target_lc,
lc_deps = starlark_codegen_utils.to_list_attr(lc_deps, 1),
lifecycle_build_target = str(rctx.attr.lifecycle_build_target),
lifecycle_target_name = lifecycle_target_name,
Expand Down

0 comments on commit 5b6bd3d

Please sign in to comment.