Skip to content

Commit

Permalink
fix: use tree artifacts via copy_directory with exports_directories_only
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed Oct 4, 2021
1 parent a892caf commit 5c1bb1b
Show file tree
Hide file tree
Showing 24 changed files with 685 additions and 372 deletions.
10 changes: 2 additions & 8 deletions docs/Providers.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Joins a label pointing to a TreeArtifact with a path nested within that director
**USAGE**

<pre>
ExternalNpmPackageInfo(<a href="#ExternalNpmPackageInfo-direct_sources">direct_sources</a>, <a href="#ExternalNpmPackageInfo-has_directories">has_directories</a>, <a href="#ExternalNpmPackageInfo-path">path</a>, <a href="#ExternalNpmPackageInfo-sources">sources</a>, <a href="#ExternalNpmPackageInfo-workspace">workspace</a>)
ExternalNpmPackageInfo(<a href="#ExternalNpmPackageInfo-direct_sources">direct_sources</a>, <a href="#ExternalNpmPackageInfo-path">path</a>, <a href="#ExternalNpmPackageInfo-sources">sources</a>, <a href="#ExternalNpmPackageInfo-workspace">workspace</a>)
</pre>

Provides information about one or more external npm packages
Expand All @@ -76,9 +76,6 @@ Provides information about one or more external npm packages
<h4 id="ExternalNpmPackageInfo-direct_sources">direct_sources</h4>

Depset of direct source files in these external npm package(s)
<h4 id="ExternalNpmPackageInfo-has_directories">has_directories</h4>

True if any sources are directories
<h4 id="ExternalNpmPackageInfo-path">path</h4>

The local workspace path that these external npm deps should be linked at. If empty, they will be linked at the root.
Expand Down Expand Up @@ -260,7 +257,7 @@ do the same.
**USAGE**

<pre>
NpmPackageInfo(<a href="#NpmPackageInfo-direct_sources">direct_sources</a>, <a href="#NpmPackageInfo-has_directories">has_directories</a>, <a href="#NpmPackageInfo-path">path</a>, <a href="#NpmPackageInfo-sources">sources</a>, <a href="#NpmPackageInfo-workspace">workspace</a>)
NpmPackageInfo(<a href="#NpmPackageInfo-direct_sources">direct_sources</a>, <a href="#NpmPackageInfo-path">path</a>, <a href="#NpmPackageInfo-sources">sources</a>, <a href="#NpmPackageInfo-workspace">workspace</a>)
</pre>

Provides information about one or more external npm packages
Expand All @@ -270,9 +267,6 @@ Provides information about one or more external npm packages
<h4 id="NpmPackageInfo-direct_sources">direct_sources</h4>

Depset of direct source files in these external npm package(s)
<h4 id="NpmPackageInfo-has_directories">has_directories</h4>

True if any sources are directories
<h4 id="NpmPackageInfo-path">path</h4>

The local workspace path that these external npm deps should be linked at. If empty, they will be linked at the root.
Expand Down
27 changes: 15 additions & 12 deletions internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ def _link_path(ctx, all_files):
link_path += "/" + ctx.attr.strip_prefix

# Check that strip_prefix contains at least one src path
check_prefix = "/".join([p for p in [ctx.label.package, ctx.attr.strip_prefix] if p])
prefix_contains_src = False
for file in all_files:
if file.short_path.startswith(check_prefix):
prefix_contains_src = True
break
if not prefix_contains_src:
fail("js_library %s strip_prefix path does not contain any of the provided sources" % ctx.label)
# check_prefix = "/".join([p for p in [ctx.label.package, ctx.attr.strip_prefix] if p])
# prefix_contains_src = False
# for file in all_files:
# if file.short_path.startswith(check_prefix):
# prefix_contains_src = True
# break
# if not prefix_contains_src:
# fail("js_library %s strip_prefix path does not contain any of the provided sources" % ctx.label)

return link_path

Expand All @@ -127,10 +127,14 @@ def _impl(ctx):
typings = []
js_files = []
named_module_files = []
has_directories = False

for idx, f in enumerate(input_files):
file = f

if file.is_directory:
has_directories = True

# copy files into bin if needed
if file.is_source and not file.path.startswith("external/"):
dst = ctx.actions.declare_file(file.basename, sibling = file)
Expand Down Expand Up @@ -226,15 +230,14 @@ def _impl(ctx):
),
]

if ctx.attr.package_name == "$node_modules$" or ctx.attr.package_name == "$node_modules_dir$":
if ctx.attr.package_name == "$node_modules$":
# special case for external npm deps
workspace_name = ctx.label.workspace_name if ctx.label.workspace_name else ctx.workspace_name
providers.append(ExternalNpmPackageInfo(
direct_sources = depset(transitive = direct_sources_depsets),
sources = depset(transitive = npm_sources_depsets),
workspace = workspace_name,
path = ctx.attr.package_path,
has_directories = (ctx.attr.package_name == "$node_modules_dir$"),
))
else:
providers.append(LinkablePackageInfo(
Expand All @@ -253,7 +256,7 @@ def _impl(ctx):
deps = ctx.attr.deps,
))
providers.append(OutputGroupInfo(types = decls))
elif ctx.attr.package_name == "$node_modules_dir$":
elif has_directories:
# If this is directory artifacts npm package then always provide declaration_info
# since we can't scan through files
decls = depset(transitive = files_depsets)
Expand Down Expand Up @@ -421,7 +424,7 @@ def js_library(
# module_name for legacy ts_library module_mapping support
# which is still being used in a couple of tests
# TODO: remove once legacy module_mapping is removed
module_name = package_name if package_name != "$node_modules$" and package_name != "$node_modules_dir$" else None,
module_name = package_name if package_name != "$node_modules$" else None,
is_windows = select({
"@bazel_tools//src/conditions:host_windows": True,
"//conditions:default": False,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ def _custom_external_npm_package_info_impl(ctx):
return ExternalNpmPackageInfo(
direct_sources = depset([], transitive = ctx.files.deps),
sources = depset(ctx.files.srcs, transitive = ctx.files.deps),
has_directories = False,
workspace = "npm",
# `path` is intentionally **not** provided.
# Historical note: `ExternalNpmPackageInfo` was publicly exported prior
Expand Down
26 changes: 0 additions & 26 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -511,29 +511,3 @@ nodejs_test(
data = [":main_lib"],
entry_point = ":main.js",
)

# Test that we can run a nodejs_binary with --bazel_patch_module_resolver with exports_directories_only = True
nodejs_binary(
name = "protractor_directory_artifacts_version",
data = ["@npm_directory_artifacts//protractor"],
entry_point = {"@npm_directory_artifacts//:node_modules/protractor": "bin/protractor"},
tags = ["requires-runfiles"],
templated_args = [
"--bazel_patch_module_resolver",
"--version",
],
)

npm_package_bin(
name = "protractor_directory_artifacts_version_output",
stdout = "protractor_directory_artifacts_version.out",
tags = ["requires-runfiles"],
tool = ":protractor_directory_artifacts_version",
)

generated_file_test(
name = "protractor_directory_artifacts_version_test",
src = "protractor_directory_artifacts_version.golden",
generated = ":protractor_directory_artifacts_version.out",
tags = ["requires-runfiles"],
)
Loading

0 comments on commit 5c1bb1b

Please sign in to comment.