Skip to content

Commit

Permalink
fix: handle http:// package.json references with mismatching package …
Browse files Browse the repository at this point in the history 
…name (#424)

Also escape # characters when generating bazel name from pnpm lockfile values
  • Loading branch information
@gregmagolan
gregmagolan authored Aug 31, 2022
1 parent d653358 commit 49ae1d4
Show file tree
Hide file tree
Showing 11 changed files with 790 additions and 691 deletions.
51 changes: 25 additions & 26 deletions npm/private/npm_import.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def npm_imported_package_store(
ref_deps = {ref_deps}
has_lifecycle_build_target = {has_lifecycle_build_target}
store_target_name = "{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)
store_target_name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)
# reference target used to avoid circular deps
_npm_package_store(
Expand Down Expand Up @@ -167,7 +167,7 @@ def npm_link_imported_package_store(
fail(msg)
link_root_name = name[:-len("/{{}}".format(link_alias))]
store_target_name = "{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)
store_target_name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)
# terminal package store target to link
_npm_link_package_store(
Expand Down Expand Up @@ -247,7 +247,7 @@ def npm_link_imported_package(

_BIN_MACRO_TMPL = """
def _{bin_name}_internal(name, link_root_name, **kwargs):
store_target_name = "{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)
store_target_name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)
_directory_path(
name = "%s__entry_point" % name,
directory = "@{link_workspace}//{root_package}:{{}}/dir".format(store_target_name),
Expand All @@ -266,7 +266,7 @@ def _{bin_name}_internal(name, link_root_name, **kwargs):
)
def _{bin_name}_test_internal(name, link_root_name, **kwargs):
store_target_name = "{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)
store_target_name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)
_directory_path(
name = "%s__entry_point" % name,
directory = "@{link_workspace}//{root_package}:{{}}/dir".format(store_target_name),
Expand All @@ -280,7 +280,7 @@ def _{bin_name}_test_internal(name, link_root_name, **kwargs):
)
def _{bin_name}_binary_internal(name, link_root_name, **kwargs):
store_target_name = "{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)
store_target_name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)
_directory_path(
name = "%s__entry_point" % name,
directory = "@{link_workspace}//{root_package}:{{}}/dir".format(store_target_name),
Expand Down Expand Up @@ -412,6 +412,8 @@ def _impl(rctx):
))

if bins:
virtual_store_name = utils.virtual_store_name(rctx.attr.package, rctx.attr.version)

for link_package in rctx.attr.link_packages.keys():
bin_bzl = generated_by_lines + [
"""load("@aspect_bazel_lib//lib:directory_path.bzl", _directory_path = "directory_path")""",
Expand All @@ -429,6 +431,7 @@ def _impl(rctx):
package = rctx.attr.package,
root_package = rctx.attr.root_package,
version = rctx.attr.version,
virtual_store_name = virtual_store_name,
virtual_store_root = utils.virtual_store_root,
),
)
Expand Down Expand Up @@ -502,10 +505,9 @@ def _impl_links(rctx):

for (dep_name, dep_version) in rctx.attr.deps.items():
if dep_version.startswith("link:") or dep_version.startswith("file:"):
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)""".format(
root_package = rctx.attr.root_package,
package = dep_name,
version = "0.0.0",
virtual_store_name = utils.virtual_store_name(dep_name, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
)
else:
Expand All @@ -514,9 +516,8 @@ def _impl_links(rctx):
else:
store_package = dep_name
store_version = dep_version
dep_store_target = """":{virtual_store_root}/{{}}/{package}/{version}/ref".format(link_root_name)""".format(
package = store_package,
version = store_version,
dep_store_target = """":{virtual_store_root}/{{}}/{virtual_store_name}/ref".format(link_root_name)""".format(
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)
ref_deps[dep_store_target] = ref_deps[dep_store_target] + [dep_name] if dep_store_target in ref_deps else [dep_name]
Expand All @@ -529,10 +530,9 @@ def _impl_links(rctx):
for (dep_name, dep_versions) in rctx.attr.transitive_closure.items():
for dep_version in dep_versions:
if dep_version.startswith("link:") or dep_version.startswith("file:"):
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)""".format(
root_package = rctx.attr.root_package,
package = dep_name,
version = "0.0.0",
virtual_store_name = utils.virtual_store_name(dep_name, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
)
lc_deps[dep_store_target] = lc_deps[dep_store_target] + [dep_name] if dep_store_target in lc_deps else [dep_name]
Expand All @@ -543,15 +543,13 @@ def _impl_links(rctx):
else:
store_package = dep_name
store_version = dep_version
dep_store_target_pkg = """":{virtual_store_root}/{{}}/{package}/{version}/pkg".format(link_root_name)""".format(
package = store_package,
version = store_version,
dep_store_target_pkg = """":{virtual_store_root}/{{}}/{virtual_store_name}/pkg".format(link_root_name)""".format(
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)
if dep_name == rctx.attr.package and dep_version == rctx.attr.version:
dep_store_target_pkg_pre_lc_lite = """":{virtual_store_root}/{{}}/{package}/{version}/pkg_pre_lc_lite".format(link_root_name)""".format(
package = store_package,
version = store_version,
dep_store_target_pkg_pre_lc_lite = """":{virtual_store_root}/{{}}/{virtual_store_name}/pkg_pre_lc_lite".format(link_root_name)""".format(
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)

Expand All @@ -565,10 +563,9 @@ def _impl_links(rctx):
else:
for (dep_name, dep_version) in rctx.attr.deps.items():
if dep_version.startswith("link:") or dep_version.startswith("file:"):
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(link_root_name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(link_root_name)""".format(
root_package = rctx.attr.root_package,
package = dep_name,
version = "0.0.0",
virtual_store_name = utils.virtual_store_name(dep_name, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
)
else:
Expand All @@ -577,9 +574,8 @@ def _impl_links(rctx):
else:
store_package = dep_name
store_version = dep_version
dep_store_target = """":{virtual_store_root}/{{}}/{package}/{version}/pkg".format(link_root_name)""".format(
package = store_package,
version = store_version,
dep_store_target = """":{virtual_store_root}/{{}}/{virtual_store_name}/pkg".format(link_root_name)""".format(
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)
lc_deps[dep_store_target] = lc_deps[dep_store_target] + [dep_name] if dep_store_target in lc_deps else [dep_name]
Expand Down Expand Up @@ -628,6 +624,8 @@ def _impl_links(rctx):
maybe_bins = ("""
bins = %s,""" % starlark_codegen_utils.to_dict_attr(rctx.attr.bins, 3)) if len(rctx.attr.bins) > 0 else ""

virtual_store_name = utils.virtual_store_name(rctx.attr.package, rctx.attr.version)

npm_link_package_bzl = [_LINK_JS_PACKAGE_TMPL.format(
deps = starlark_codegen_utils.to_dict_attr(deps, 2, quote_key = False),
link_default = "None" if rctx.attr.link_packages else "True",
Expand All @@ -647,6 +645,7 @@ def _impl_links(rctx):
root_package = rctx.attr.root_package,
transitive_closure_pattern = str(transitive_closure_pattern),
version = rctx.attr.version,
virtual_store_name = virtual_store_name,
virtual_store_root = utils.virtual_store_root,
maybe_bins = maybe_bins,
)]
Expand Down
37 changes: 16 additions & 21 deletions npm/private/npm_translate_lock.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ _FP_STORE_TMPL = \
"""
if is_root:
_npm_package_store(
name = "{virtual_store_root}/{{}}/{package}/0.0.0".format(name),
name = "{virtual_store_root}/{{}}/{virtual_store_name}".format(name),
src = "{npm_package_target}",
package = "{package}",
version = "0.0.0",
Expand All @@ -75,7 +75,7 @@ _FP_DIRECT_TMPL = \
# terminal target for direct dependencies
_npm_link_package_store(
name = "{{}}/{name}".format(name),
src = "//{root_package}:{virtual_store_root}/{{}}/{package}/0.0.0".format(name),
src = "//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name),
visibility = ["//visibility:public"],
tags = ["manual"],
use_declare_symlink = select({{
Expand Down Expand Up @@ -548,25 +548,22 @@ load("@aspect_rules_js//js:defs.bzl", _js_library = "js_library")"""]
transitive_deps = {}
for raw_package, raw_version in deps.items():
if raw_version.startswith("link:") or raw_version.startswith("file:"):
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = raw_package,
version = "0.0.0",
virtual_store_name = utils.virtual_store_name(raw_package, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
)
elif raw_version.startswith("/"):
store_package, store_version = utils.parse_pnpm_name(raw_version[1:])
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = store_package,
version = store_version,
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)
else:
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = raw_package,
version = raw_version,
virtual_store_name = utils.virtual_store_name(raw_package, raw_version),
virtual_store_root = utils.virtual_store_root,
)
if dep_store_target not in transitive_deps:
Expand Down Expand Up @@ -610,25 +607,22 @@ load("@aspect_rules_js//js:defs.bzl", _js_library = "js_library")"""]
raw_deps = importers.get(dep_importer).get("dependencies")
for raw_package, raw_version in raw_deps.items():
if raw_version.startswith("link:") or raw_version.startswith("file:"):
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = raw_package,
version = "0.0.0",
virtual_store_name = utils.virtual_store_name(raw_package, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
)
elif raw_version.startswith("/"):
store_package, store_version = utils.parse_pnpm_name(raw_version[1:])
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = store_package,
version = store_version,
virtual_store_name = utils.virtual_store_name(store_package, store_version),
virtual_store_root = utils.virtual_store_root,
)
else:
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{package}/{version}".format(name)""".format(
dep_store_target = """"//{root_package}:{virtual_store_root}/{{}}/{virtual_store_name}".format(name)""".format(
root_package = root_package,
package = raw_package,
version = raw_version,
virtual_store_name = utils.virtual_store_name(raw_package, raw_version),
virtual_store_root = utils.virtual_store_root,
)
if dep_store_target not in transitive_deps:
Expand Down Expand Up @@ -832,16 +826,17 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
deps = starlark_codegen_utils.to_dict_attr(fp_deps, 3, quote_key = False),
npm_package_target = fp_target,
package = fp_package,
virtual_store_name = utils.virtual_store_name(fp_package, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
))

defs_bzl_body.append(_FP_DIRECT_TMPL.format(
bazel_name = fp_bazel_name,
link_packages = fp_link_packages.keys(),
name = fp_package,
package = fp_package,
package_directory_output_group = utils.package_directory_output_group,
root_package = root_package,
virtual_store_name = utils.virtual_store_name(fp_package, "0.0.0"),
virtual_store_root = utils.virtual_store_root,
))

Expand Down
3 changes: 3 additions & 0 deletions npm/private/test/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ filegroup(
":node_modules/webpack-bundle-analyzer",
":node_modules/semver-first-satisfied",
":node_modules/@figma/nodegit",
":node_modules/typescript",
":node_modules/inline-fixtures",
":node_modules/json-stable-stringify",
# intentionally don't include node_modules/unused
],
)
Expand Down
Loading

0 comments on commit 49ae1d4

Please sign in to comment.