Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy resources for python_boostrap rules. #676

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions prelude/decls/shell_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ sh_binary = prelude_rule(
appending one itself if necessary. Setting this to False prevents that behavior and makes the caller
responsible for ensuring an existing appropriate extension.
"""),
"copy_resources": attrs.bool(default = False, doc = """
By default, sh_binary attempts to use symbolic links for the resources. This can be changed so,
that copies are made instead.
"""),
"contacts": attrs.list(attrs.string(), default = []),
"default_host_platform": attrs.option(attrs.configuration_label(), default = None),
"deps": attrs.list(attrs.dep(), default = []),
Expand Down
7 changes: 6 additions & 1 deletion prelude/python_bootstrap/python_bootstrap.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def python_bootstrap_binary_impl(ctx: AnalysisContext) -> list[Provider]:
they can and can't do. In particular, bootstrap binaries can only depend on
bootstrap libraries and can only consist of a single file.
"""
copy_deps = ctx.attrs.copy_deps
run_tree_inputs = {}
run_tree_recorded_deps = {} # For a better error message when files collide
for dep in ctx.attrs.deps:
Expand All @@ -38,7 +39,11 @@ def python_bootstrap_binary_impl(ctx: AnalysisContext) -> list[Provider]:
run_tree_inputs[src.short_path] = src
run_tree_recorded_deps[src.short_path] = dep

run_tree = ctx.actions.symlinked_dir("__%s__" % ctx.attrs.name, run_tree_inputs)
if copy_deps:
run_tree = ctx.actions.copied_dir("__%s__" % ctx.attrs.name, run_tree_inputs)
else:
run_tree = ctx.actions.symlinked_dir("__%s__" % ctx.attrs.name, run_tree_inputs)

output = ctx.actions.copy_file(ctx.attrs.main.short_path, ctx.attrs.main)

interpreter = ctx.attrs._python_bootstrap_toolchain[PythonBootstrapToolchainInfo].interpreter
Expand Down
1 change: 1 addition & 0 deletions prelude/python_bootstrap/tools/BUCK.v2
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ prelude = native
prelude.sh_binary(
name = "win_python_wrapper",
main = "win_python_wrapper.bat",
copy_resources = True,
target_compatible_with = ["config//os:windows"],
visibility = ["PUBLIC"],
)
1 change: 1 addition & 0 deletions prelude/rules_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ inlined_extra_attributes = {
"python_bootstrap_binary": {
"deps": attrs.list(attrs.dep(providers = [PythonBootstrapSources]), default = []),
"main": attrs.source(),
"copy_deps": attrs.bool(default = True),
"_exec_os_type": buck.exec_os_type_arg(),
"_python_bootstrap_toolchain": toolchains_common.python_bootstrap(),
"_win_python_wrapper": attrs.default_only(
Expand Down
17 changes: 13 additions & 4 deletions prelude/sh_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ def _generate_script(
resources: list[Artifact],
append_script_extension: bool,
actions: AnalysisActions,
is_windows: bool) -> (Artifact, Artifact):
is_windows: bool,
copy_resources: bool) -> (Artifact, Artifact):
main_path = main.short_path
if not append_script_extension:
main_link = main_path
Expand All @@ -35,7 +36,12 @@ def _generate_script(
resources = {_derive_link(src): src for src in resources}
resources[main_link] = main

resources_dir = actions.symlinked_dir("resources", resources)
# windows isn't stable with resources passed in as symbolic links for
# remote execution. Allow using copies instead.
if copy_resources:
resources_dir = actions.copied_dir("resources", resources)
else:
resources_dir = actions.symlinked_dir("resources", resources)

script_name = name + (".bat" if is_windows else "")
script = actions.declare_output(script_name)
Expand Down Expand Up @@ -81,8 +87,9 @@ def _generate_script(
"setlocal EnableDelayedExpansion",
# Fully qualified script path.
"set __SRC=%~f0",
# This is essentially a realpath.
'for /f "tokens=2 delims=[]" %%a in (\'dir %__SRC% ^|%SYSTEMROOT%\\System32\\find.exe "<SYMLINK>"\') do set "__SRC=%%a"',
# Symbolic links on windows RE systems aren't stable. so don't try to resolve
# the links when using copies.
'for /f "tokens=2 delims=[]" %%a in (\'dir %__SRC% ^|%SYSTEMROOT%\\System32\\find.exe "<SYMLINK>"\') do set "__SRC=%%a"' if not copy_resources else '',
# Get parent folder.
'for %%a in ("%__SRC%") do set "__SCRIPT_DIR=%%~dpa"',
"set BUCK_SH_BINARY_VERSION_UNSTABLE=2",
Expand All @@ -103,6 +110,7 @@ def _generate_script(
# "deps": attrs.list(attrs.dep(), default = []),
# "main": attrs.source(),
# "resources": attrs.list(attrs.source(), default = []),
# "copy_resources": attrs.bool(default = False),
def sh_binary_impl(ctx):
# TODO: implement deps (not sure what those even do, though)
if len(ctx.attrs.deps) > 0:
Expand All @@ -116,6 +124,7 @@ def sh_binary_impl(ctx):
ctx.attrs.append_script_extension,
ctx.actions,
is_windows,
ctx.attrs.copy_resources,
)

script = script.with_associated_artifacts([resources_dir])
Expand Down