From 7064332d6b41d4dcbd45dd1c1bd779c63a808c8b Mon Sep 17 00:00:00 2001 From: Jeremy Volkman Date: Fri, 14 Oct 2022 21:00:05 -0700 Subject: [PATCH] Switch to using Args --- pycross/private/lock_file.bzl | 80 ++++++-------------------- pycross/private/pdm_lock_model.bzl | 20 +++---- pycross/private/poetry_lock_model.bzl | 14 ++--- pycross/private/target_environment.bzl | 62 +++++++------------- pycross/private/wheel_build.bzl | 75 ++++++++++++------------ pycross/private/wheel_library.bzl | 18 ++---- 6 files changed, 96 insertions(+), 173 deletions(-) diff --git a/pycross/private/lock_file.bzl b/pycross/private/lock_file.bzl index a9625870..b1450eb1 100644 --- a/pycross/private/lock_file.bzl +++ b/pycross/private/lock_file.bzl @@ -14,91 +14,49 @@ def _pycross_lock_file_impl(ctx): else: repo_prefix = ctx.attr.name.lower().replace("-", "_") - args = [ - "--lock-model-file", - ctx.file.lock_model_file.path, - "--repo-prefix", - repo_prefix, - "--package-prefix", - ctx.attr.package_prefix, - "--build-prefix", - ctx.attr.build_prefix, - "--environment-prefix", - ctx.attr.environment_prefix, - "--output", - out.path, - ] + args = ctx.actions.args() + args.add("--lock-model-file", ctx.file.lock_model_file) + args.add("--repo-prefix", repo_prefix) + args.add("--package-prefix", ctx.attr.package_prefix) + args.add("--build-prefix", ctx.attr.build_prefix) + args.add("--environment-prefix", ctx.attr.environment_prefix) + args.add("--output", out) for t in ctx.attr.target_environments: - args.extend([ - "--target-environment", - t[PycrossTargetEnvironmentInfo].file.path, - fully_qualified_label(t.label), - ]) + args.add_all("--target-environment", [t[PycrossTargetEnvironmentInfo].file.path, fully_qualified_label(t.label)]) for local_wheel in ctx.files.local_wheels: if not local_wheel.owner: fail("Could not determine owning lable for local wheel: %s" % local_wheel) - args.extend([ - "--local-wheel", - local_wheel.basename, - fully_qualified_label(local_wheel.owner), - ]) + args.add_all("--local-wheel", [local_wheel.basename, fully_qualified_label(local_wheel.owner)]) for remote_wheel_url, sha256 in ctx.attr.remote_wheels.items(): - args.extend([ - "--remote-wheel", - remote_wheel_url, - sha256, - ]) + args.add_all("--remote-wheel", [remote_wheel_url, sha256]) if ctx.attr.package_prefix != None: - args.extend([ - "--package-prefix", - ctx.attr.package_prefix, - ]) + args.add("--package-prefix", ctx.attr.package_prefix) if ctx.attr.build_prefix != None: - args.extend([ - "--build-prefix", - ctx.attr.build_prefix, - ]) + args.add("--build-prefix", ctx.attr.build_prefix) if ctx.attr.environment_prefix != None: - args.extend([ - "--environment-prefix", - ctx.attr.environment_prefix, - ]) + args.add("--environment-prefix", ctx.attr.environment_prefix) if ctx.attr.default_alias_single_version: - args.append("--default-alias-single-version") + args.add("--default-alias-single-version") for k, t in ctx.attr.build_target_overrides.items(): - args.extend([ - "--build-target-override", - k, - t, - ]) + args.add_all("--build-target-override", [k, t]) for k in ctx.attr.always_build_packages: - args.extend([ - "--always-build-package", - k - ]) + args.add("--always-build-package", k) for k, d in ctx.attr.package_build_dependencies.items(): for dep in d: - args.extend([ - "--build-dependency", - k, - dep, - ]) + args.add_all("--build-dependency", [k, dep]) if ctx.attr.pypi_index: - args.extend([ - "--pypi-index", - ctx.attr.pypi_index, - ]) + args.add("--pypi-index", ctx.attr.pypi_index) ctx.actions.run( inputs = ( @@ -107,7 +65,7 @@ def _pycross_lock_file_impl(ctx): ), outputs = [out], executable = ctx.executable._tool, - arguments = args, + arguments = [args], ) return [ diff --git a/pycross/private/pdm_lock_model.bzl b/pycross/private/pdm_lock_model.bzl index c3b512ce..a81050d8 100644 --- a/pycross/private/pdm_lock_model.bzl +++ b/pycross/private/pdm_lock_model.bzl @@ -3,23 +3,19 @@ def _pycross_pdm_lock_model_impl(ctx): out = ctx.actions.declare_file(ctx.attr.name + ".json") - args = [ - "--project-file", - ctx.file.project_file.path, - "--lock-file", - ctx.file.lock_file.path, - "--output", - out.path, - ] + args = ctx.actions.args() + args.add("--project-file", ctx.file.project_file) + args.add("--lock-file", ctx.file.lock_file) + args.add("--output", out) if ctx.attr.default: - args.append("--default") + args.add("--default") if ctx.attr.dev: - args.append("--dev") + args.add("--dev") for group in ctx.attr.groups: - args.extend(["--group", group]) + args.add("--group", group) ctx.actions.run( inputs = ( @@ -28,7 +24,7 @@ def _pycross_pdm_lock_model_impl(ctx): ), outputs = [out], executable = ctx.executable._tool, - arguments = args, + arguments = [args], ) return [ diff --git a/pycross/private/poetry_lock_model.bzl b/pycross/private/poetry_lock_model.bzl index 6265298e..c97fafa3 100644 --- a/pycross/private/poetry_lock_model.bzl +++ b/pycross/private/poetry_lock_model.bzl @@ -3,14 +3,10 @@ def _pycross_poetry_lock_model_impl(ctx): out = ctx.actions.declare_file(ctx.attr.name + ".json") - args = [ - "--poetry-project-file", - ctx.file.poetry_project_file.path, - "--poetry-lock-file", - ctx.file.poetry_lock_file.path, - "--output", - out.path, - ] + args = ctx.actions.args() + args.add("--poetry-project-file", ctx.file.poetry_project_file) + args.add("--poetry-lock-file", ctx.file.poetry_lock_file) + args.add("--output", out) ctx.actions.run( inputs = ( @@ -19,7 +15,7 @@ def _pycross_poetry_lock_model_impl(ctx): ), outputs = [out], executable = ctx.executable._tool, - arguments = args, + arguments = [args], ) return [ diff --git a/pycross/private/target_environment.bzl b/pycross/private/target_environment.bzl index 9756b8ea..f2f9582c 100644 --- a/pycross/private/target_environment.bzl +++ b/pycross/private/target_environment.bzl @@ -9,39 +9,28 @@ def fully_qualified_label(label): def _target_python_impl(ctx): f = ctx.actions.declare_file(ctx.attr.name + ".json") - args = [ - "--name", - ctx.attr.name, - "--output", - f.path, - "--implementation", - ctx.attr.implementation, - "--version", - ctx.attr.version, - ] + args = ctx.actions.args() + args.add("--name", ctx.attr.name) + args.add("--output", f) + args.add("--implementation", ctx.attr.implementation) + args.add("--version", ctx.attr.version) for abi in ctx.attr.abis: - args.extend(["--abi", abi]) + args.add("--abi", abi) for platform in ctx.attr.platforms: - args.extend(["--platform", platform]) + args.add("--platform", platform) for constraint in ctx.attr.python_compatible_with: - args.extend([ - "--python-compatible-with", - fully_qualified_label(constraint.label), - ]) + args.add("--python-compatible-with", fully_qualified_label(constraint.label)) for key, val in ctx.attr.envornment_markers.items(): - args.extend([ - "--environment-marker", - "%s=%s" % (key, val), - ]) + args.add("--environment-marker", "%s=%s" % (key, val)) ctx.actions.run( outputs = [f], executable = ctx.executable._tool, - arguments = args, + arguments = [args], ) return [ @@ -104,39 +93,28 @@ pycross_target_environment = rule( def _macos_target_python_impl(ctx): f = ctx.actions.declare_file(ctx.attr.name + ".json") - args = [ - "--name", - ctx.attr.name, - "--output", - f.path, - "--implementation", - ctx.attr.implementation, - "--version", - ctx.attr.version, - ] + args = ctx.actions.args() + args.add("--name", ctx.attr.name) + args.add("--output", f) + args.add("--implementation", ctx.attr.implementation) + args.add("--version", ctx.attr.version) for abi in ctx.attr.abis: - args.extend(["--abi", abi]) + args.add("--abi", abi) for platform in ctx.attr.platforms: - args.extend(["--platform", platform]) + args.add("--platform", platform) for constraint in ctx.attr.python_compatible_with: - args.extend([ - "--python-compatible-with", - fully_qualified_label(constraint.label), - ]) + args.add("--python-compatible-with", fully_qualified_label(constraint.label)) for key, val in ctx.attr.envornment_markers.items(): - args.extend([ - "--environment-marker", - "%s=%s" % (key, val), - ]) + args.add("--environment-marker", "%s=%s" % (key, val)) ctx.actions.run( outputs = [f], executable = ctx.executable._tool, - arguments = args, + arguments = [args], ) return [ diff --git a/pycross/private/wheel_build.bzl b/pycross/private/wheel_build.bzl index 0de8a500..d4b15677 100644 --- a/pycross/private/wheel_build.bzl +++ b/pycross/private/wheel_build.bzl @@ -45,7 +45,16 @@ def _is_sibling_repository_layout_enabled(ctx): test = Label("@not_" + ctx.workspace_name) # the not_ prefix means it can't be our local workspace. return test.workspace_root.startswith("..") -def _resolve_import_path(ctx, import_name): +def _resolve_import_path_fn(ctx): + # Call the inner function with simple values so the closure it returns doesn't hold onto a large + # amount of state. + return _resolve_import_path_fn_inner( + ctx.workspace_name, + ctx.bin_dir.path, + _is_sibling_repository_layout_enabled(ctx), + ) + +def _resolve_import_path_fn_inner(workspace_name, bin_dir, sibling_layout): # The PyInfo import names assume a runfiles-type structure. E.g.: # mytool.runfiles/ # main_repo/ @@ -86,27 +95,30 @@ def _resolve_import_path(ctx, import_name): # ... # - # Split the import name into its repo and path. - import_repo, import_path = import_name.split("/", 1) - # ctx.bin_dir returns something like bazel-out/k8-fastbuild/bin in legacy mode, or # bazel-out/my_external_repo/k8-fastbuild/bin in sibling layout when the target is within an external repo. # We really just want the first part and the last two parts. The repo name that's added with sibling mode isn't # useful for our case. - bin_dir_parts = ctx.bin_dir.path.split("/") + bin_dir_parts = bin_dir.split("/") output_dir = bin_dir_parts[0] bin_dir = paths.join(*bin_dir_parts[-2:]) - # Packages within the workspace are always the same regardless of sibling layout. - if import_repo == ctx.workspace_name: - return paths.join(output_dir, bin_dir, import_path) + def fn(import_name): + # Split the import name into its repo and path. + import_repo, import_path = import_name.split("/", 1) + + # Packages within the workspace are always the same regardless of sibling layout. + if import_repo == workspace_name: + return paths.join(output_dir, bin_dir, import_path) - # Otherwise, if sibling layout is enabled... - if _is_sibling_repository_layout_enabled(ctx): - return paths.join(output_dir, import_repo, bin_dir, import_path) + # Otherwise, if sibling layout is enabled... + if sibling_layout: + return paths.join(output_dir, import_repo, bin_dir, import_path) - # And lastly, just use the traditional layout. - return paths.join(output_dir, bin_dir, "external", import_repo, import_path) + # And lastly, just use the traditional layout. + return paths.join(output_dir, bin_dir, "external", import_repo, import_path) + + return fn def _pycross_wheel_build_impl(ctx): cc_sysconfig_data = ctx.actions.declare_file(paths.join(ctx.attr.name, "cc_sysconfig.json")) @@ -135,41 +147,29 @@ def _pycross_wheel_build_impl(ctx): if not executable: executable = py_toolchain.interpreter.path - args = [ - "--sdist", - ctx.file.sdist.path, - "--sysconfig-vars", - cc_sysconfig_data.path, - "--wheel-file", - out_wheel.path, - "--wheel-name-file", - out_name.path, - "--exec-python-executable", - executable, - ] + args = ctx.actions.args() + args.add("--sdist", ctx.file.sdist) + args.add("--sysconfig-vars", cc_sysconfig_data) + args.add("--wheel-file", out_wheel) + args.add("--wheel-name-file", out_name) + args.add("--exec-python-executable", executable) if ctx.attr.target_environment: - args.extend([ - "--target-environment-file", - ctx.attr.target_environment[PycrossTargetEnvironmentInfo].file.path, - ]) + target_environment_file = ctx.attr.target_environment[PycrossTargetEnvironmentInfo].file + args.add("--target-environment-file", target_environment_file) imports = depset( transitive = [d[PyInfo].imports for d in ctx.attr.deps], ) - for import_name in imports.to_list(): - args.extend([ - "--path", - _resolve_import_path(ctx, import_name), - ]) + args.add_all(imports, before_each="--path", map_each=_resolve_import_path_fn(ctx), allow_closure=True) ctx.actions.write(cc_sysconfig_data, json.encode(sysconfig_vars)) deps = [ ctx.file.sdist, cc_sysconfig_data, - ] + ctx.files.deps + ] transitive_sources = [dep[PyInfo].transitive_sources for dep in ctx.attr.deps if PyInfo in dep] @@ -186,12 +186,13 @@ def _pycross_wheel_build_impl(ctx): env.update(ctx.configuration.default_shell_env) ctx.actions.run( - inputs = depset(deps, transitive = toolchain_deps + transitive_sources), + inputs = deps, outputs = [out_wheel, out_name], + tools = depset(transitive = toolchain_deps + transitive_sources), executable = ctx.executable._tool, use_default_shell_env = False, env = env, - arguments = args, + arguments = [args], mnemonic = "WheelBuild", progress_message = "Building %s" % ctx.file.sdist.basename, ) diff --git a/pycross/private/wheel_library.bzl b/pycross/private/wheel_library.bzl index 68924c23..b829092f 100644 --- a/pycross/private/wheel_library.bzl +++ b/pycross/private/wheel_library.bzl @@ -15,29 +15,23 @@ def _pycross_wheel_library_impl(ctx): wheel_file = ctx.file.wheel name_file = None - args = [ - "--wheel", - wheel_file.path, - "--directory", - out.path, - ] + args = ctx.actions.args() + args.add("--wheel", wheel_file) + args.add("--directory", out.path) inputs = [wheel_file] if name_file: inputs.append(name_file) - args.extend([ - "--wheel-name-file", - name_file.path, - ]) + args.add("--wheel-name-file", name_file) if ctx.attr.enable_implicit_namespace_pkgs: - args.append("--enable-implicit-namespace-pkgs") + args.add("--enable-implicit-namespace-pkgs") ctx.actions.run( inputs = inputs, outputs = [out], executable = ctx.executable._tool, - arguments = args, + arguments = [args], # Set environment variables to make generated .pyc files reproducible. env = { "SOURCE_DATE_EPOCH": "315532800",