From fb51e5bf934dce4b8c7620b9a457d63cd0c6460b Mon Sep 17 00:00:00 2001 From: Abel Valadez Date: Mon, 3 Jun 2024 15:06:46 -0700 Subject: [PATCH 1/5] Allow copies to be used in sh_binary and python_boostrap_binary --- prelude/decls/shell_rules.bzl | 4 ++++ prelude/python_bootstrap/python_bootstrap.bzl | 11 ++++++++--- prelude/python_bootstrap/tools/BUCK.v2 | 11 ++++++++++- prelude/rules_impl.bzl | 13 +++++++++++-- prelude/sh_binary.bzl | 17 +++++++++++++---- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/prelude/decls/shell_rules.bzl b/prelude/decls/shell_rules.bzl index 6e8b5a7b84ff..5d6dd9df893c 100644 --- a/prelude/decls/shell_rules.bzl +++ b/prelude/decls/shell_rules.bzl @@ -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 = []), diff --git a/prelude/python_bootstrap/python_bootstrap.bzl b/prelude/python_bootstrap/python_bootstrap.bzl index ef628945dc3a..e1053bbd1a8f 100644 --- a/prelude/python_bootstrap/python_bootstrap.bzl +++ b/prelude/python_bootstrap/python_bootstrap.bzl @@ -38,14 +38,19 @@ 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) + copy = True + if copy: + 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 - if ctx.attrs._win_python_wrapper != None: + if ctx.attrs._win_python_copied_wrapper != None and ctx.attrs._win_python_symlinked_wrapper != None: run_args = cmd_args( - ctx.attrs._win_python_wrapper[RunInfo], + ctx.attrs._win_python_copied_wrapper[RunInfo] if copy else ctx.attrs._win_python_symlinked_wrapper[RunInfo], run_tree, interpreter, output, diff --git a/prelude/python_bootstrap/tools/BUCK.v2 b/prelude/python_bootstrap/tools/BUCK.v2 index e3fb697482f5..25102e963efc 100644 --- a/prelude/python_bootstrap/tools/BUCK.v2 +++ b/prelude/python_bootstrap/tools/BUCK.v2 @@ -7,8 +7,17 @@ source_listing() prelude = native prelude.sh_binary( - name = "win_python_wrapper", + name = "win_python_copied_wrapper", main = "win_python_wrapper.bat", + copy_resources = True, + target_compatible_with = ["config//os:windows"], + visibility = ["PUBLIC"], +) + +prelude.sh_binary( + name = "win_python_symlinked_wrapper", + main = "win_python_wrapper.bat", + copy_resources = False, target_compatible_with = ["config//os:windows"], visibility = ["PUBLIC"], ) diff --git a/prelude/rules_impl.bzl b/prelude/rules_impl.bzl index de7173126bd1..3b5f01897759 100644 --- a/prelude/rules_impl.bzl +++ b/prelude/rules_impl.bzl @@ -595,12 +595,21 @@ inlined_extra_attributes = { "main": attrs.source(), "_exec_os_type": buck.exec_os_type_arg(), "_python_bootstrap_toolchain": toolchains_common.python_bootstrap(), - "_win_python_wrapper": attrs.default_only( + "_win_python_copied_wrapper": attrs.default_only( attrs.option( attrs.dep(), default = select({ "DEFAULT": None, - "config//os:windows": "prelude//python_bootstrap/tools:win_python_wrapper", + "config//os:windows": "prelude//python_bootstrap/tools:win_python_copied_wrapper", + }), + ) + ), + "_win_python_symlinked_wrapper": attrs.default_only( + attrs.option( + attrs.dep(), + default = select({ + "DEFAULT": None, + "config//os:windows": "prelude//python_bootstrap/tools:win_python_symlinked_wrapper", }), ), ), diff --git a/prelude/sh_binary.bzl b/prelude/sh_binary.bzl index c54dcd5c07bf..b070cd433b8a 100644 --- a/prelude/sh_binary.bzl +++ b/prelude/sh_binary.bzl @@ -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 @@ -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) @@ -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 ""\') 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 ""\') 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", @@ -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: @@ -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]) From 4de6364ee3779af8cdbd592f81e0aac80fa22a0a Mon Sep 17 00:00:00 2001 From: Abel Valadez Date: Mon, 3 Jun 2024 15:25:59 -0700 Subject: [PATCH 2/5] Add proper argument for copying resources --- prelude/python_bootstrap/python_bootstrap.bzl | 8 ++++---- prelude/python_bootstrap/tools/BUCK.v2 | 11 +---------- prelude/rules_impl.bzl | 14 +++----------- 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/prelude/python_bootstrap/python_bootstrap.bzl b/prelude/python_bootstrap/python_bootstrap.bzl index e1053bbd1a8f..5894ff9697e1 100644 --- a/prelude/python_bootstrap/python_bootstrap.bzl +++ b/prelude/python_bootstrap/python_bootstrap.bzl @@ -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: @@ -38,8 +39,7 @@ def python_bootstrap_binary_impl(ctx: AnalysisContext) -> list[Provider]: run_tree_inputs[src.short_path] = src run_tree_recorded_deps[src.short_path] = dep - copy = True - if copy: + 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) @@ -48,9 +48,9 @@ def python_bootstrap_binary_impl(ctx: AnalysisContext) -> list[Provider]: interpreter = ctx.attrs._python_bootstrap_toolchain[PythonBootstrapToolchainInfo].interpreter - if ctx.attrs._win_python_copied_wrapper != None and ctx.attrs._win_python_symlinked_wrapper != None: + if ctx.attrs._win_python_wrapper != None: run_args = cmd_args( - ctx.attrs._win_python_copied_wrapper[RunInfo] if copy else ctx.attrs._win_python_symlinked_wrapper[RunInfo], + ctx.attrs._win_python_wrapper[RunInfo], run_tree, interpreter, output, diff --git a/prelude/python_bootstrap/tools/BUCK.v2 b/prelude/python_bootstrap/tools/BUCK.v2 index 25102e963efc..e3fb697482f5 100644 --- a/prelude/python_bootstrap/tools/BUCK.v2 +++ b/prelude/python_bootstrap/tools/BUCK.v2 @@ -7,17 +7,8 @@ source_listing() prelude = native prelude.sh_binary( - name = "win_python_copied_wrapper", + name = "win_python_wrapper", main = "win_python_wrapper.bat", - copy_resources = True, - target_compatible_with = ["config//os:windows"], - visibility = ["PUBLIC"], -) - -prelude.sh_binary( - name = "win_python_symlinked_wrapper", - main = "win_python_wrapper.bat", - copy_resources = False, target_compatible_with = ["config//os:windows"], visibility = ["PUBLIC"], ) diff --git a/prelude/rules_impl.bzl b/prelude/rules_impl.bzl index 3b5f01897759..3f6a69b3d2ce 100644 --- a/prelude/rules_impl.bzl +++ b/prelude/rules_impl.bzl @@ -593,23 +593,15 @@ inlined_extra_attributes = { "python_bootstrap_binary": { "deps": attrs.list(attrs.dep(providers = [PythonBootstrapSources]), default = []), "main": attrs.source(), + "copy_deps": attrs.bool(default = False), "_exec_os_type": buck.exec_os_type_arg(), "_python_bootstrap_toolchain": toolchains_common.python_bootstrap(), - "_win_python_copied_wrapper": attrs.default_only( + "_win_python_wrapper": attrs.default_only( attrs.option( attrs.dep(), default = select({ "DEFAULT": None, - "config//os:windows": "prelude//python_bootstrap/tools:win_python_copied_wrapper", - }), - ) - ), - "_win_python_symlinked_wrapper": attrs.default_only( - attrs.option( - attrs.dep(), - default = select({ - "DEFAULT": None, - "config//os:windows": "prelude//python_bootstrap/tools:win_python_symlinked_wrapper", + "config//os:windows": "prelude//python_bootstrap/tools:win_python_wrapper", }), ), ), From 6784922cf4d3faeecbcd2cf73895873f6a506809 Mon Sep 17 00:00:00 2001 From: Abel Valadez Date: Mon, 3 Jun 2024 15:27:48 -0700 Subject: [PATCH 3/5] Export cxx python tools --- prelude/cxx/tools/BUCK.v2 | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/prelude/cxx/tools/BUCK.v2 b/prelude/cxx/tools/BUCK.v2 index 8ca3c303bed6..2748823e9e58 100644 --- a/prelude/cxx/tools/BUCK.v2 +++ b/prelude/cxx/tools/BUCK.v2 @@ -22,15 +22,27 @@ prelude.command_alias( visibility = ["PUBLIC"], ) +prelude.export_file( + name = "make_comp_db.py", + src = "make_comp_db.py", + visibility = ["PUBLIC"], +) + prelude.python_bootstrap_binary( name = "make_comp_db", - main = "make_comp_db.py", + main = ":make_comp_db.py", + visibility = ["PUBLIC"], +) + +prelude.export_file( + name = "dep_file_processor.py", + src = "dep_file_processor.py", visibility = ["PUBLIC"], ) prelude.python_bootstrap_binary( name = "dep_file_processor", - main = "dep_file_processor.py", + main = ":dep_file_processor.py", visibility = ["PUBLIC"], deps = [ ":dep_file_processors", @@ -48,9 +60,15 @@ prelude.python_bootstrap_library( visibility = ["PUBLIC"], ) +prelude.export_file( + name = "linker_wrapper.py", + src = "linker_wrapper.py", + visibility = ["PUBLIC"], +) + prelude.python_bootstrap_binary( name = "linker_wrapper", - main = "linker_wrapper.py", + main = ":linker_wrapper.py", visibility = ["PUBLIC"], ) From 11bb0f5e25c661c9913e26157ebd5573e09e08f8 Mon Sep 17 00:00:00 2001 From: Abel Valadez Date: Mon, 3 Jun 2024 15:33:08 -0700 Subject: [PATCH 4/5] Export win_python_wrapper.bat --- prelude/python_bootstrap/tools/BUCK.v2 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/prelude/python_bootstrap/tools/BUCK.v2 b/prelude/python_bootstrap/tools/BUCK.v2 index e3fb697482f5..b5863cf80682 100644 --- a/prelude/python_bootstrap/tools/BUCK.v2 +++ b/prelude/python_bootstrap/tools/BUCK.v2 @@ -6,9 +6,16 @@ source_listing() prelude = native +prelude.export_file( + name = "win_python_wrapper.bat", + src = "win_python_wrapper.bat", + target_compatible_with = ["config//os:windows"], + visibility = ["PUBLIC"], +) + prelude.sh_binary( name = "win_python_wrapper", - main = "win_python_wrapper.bat", + main = ":win_python_wrapper.bat", target_compatible_with = ["config//os:windows"], visibility = ["PUBLIC"], ) From 3ba89d1a9866859e30218506743ed7de0c7ea4e4 Mon Sep 17 00:00:00 2001 From: Abel Valadez Date: Tue, 4 Jun 2024 08:38:27 -0700 Subject: [PATCH 5/5] Make copy resources default for python_bootstrap --- prelude/cxx/tools/BUCK.v2 | 24 +++--------------------- prelude/python_bootstrap/tools/BUCK.v2 | 10 ++-------- prelude/rules_impl.bzl | 2 +- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/prelude/cxx/tools/BUCK.v2 b/prelude/cxx/tools/BUCK.v2 index 2748823e9e58..8ca3c303bed6 100644 --- a/prelude/cxx/tools/BUCK.v2 +++ b/prelude/cxx/tools/BUCK.v2 @@ -22,27 +22,15 @@ prelude.command_alias( visibility = ["PUBLIC"], ) -prelude.export_file( - name = "make_comp_db.py", - src = "make_comp_db.py", - visibility = ["PUBLIC"], -) - prelude.python_bootstrap_binary( name = "make_comp_db", - main = ":make_comp_db.py", - visibility = ["PUBLIC"], -) - -prelude.export_file( - name = "dep_file_processor.py", - src = "dep_file_processor.py", + main = "make_comp_db.py", visibility = ["PUBLIC"], ) prelude.python_bootstrap_binary( name = "dep_file_processor", - main = ":dep_file_processor.py", + main = "dep_file_processor.py", visibility = ["PUBLIC"], deps = [ ":dep_file_processors", @@ -60,15 +48,9 @@ prelude.python_bootstrap_library( visibility = ["PUBLIC"], ) -prelude.export_file( - name = "linker_wrapper.py", - src = "linker_wrapper.py", - visibility = ["PUBLIC"], -) - prelude.python_bootstrap_binary( name = "linker_wrapper", - main = ":linker_wrapper.py", + main = "linker_wrapper.py", visibility = ["PUBLIC"], ) diff --git a/prelude/python_bootstrap/tools/BUCK.v2 b/prelude/python_bootstrap/tools/BUCK.v2 index b5863cf80682..ad7248eb054f 100644 --- a/prelude/python_bootstrap/tools/BUCK.v2 +++ b/prelude/python_bootstrap/tools/BUCK.v2 @@ -6,16 +6,10 @@ source_listing() prelude = native -prelude.export_file( - name = "win_python_wrapper.bat", - src = "win_python_wrapper.bat", - target_compatible_with = ["config//os:windows"], - visibility = ["PUBLIC"], -) - prelude.sh_binary( name = "win_python_wrapper", - main = ":win_python_wrapper.bat", + main = "win_python_wrapper.bat", + copy_resources = True, target_compatible_with = ["config//os:windows"], visibility = ["PUBLIC"], ) diff --git a/prelude/rules_impl.bzl b/prelude/rules_impl.bzl index 3f6a69b3d2ce..3e4cc1c9bd44 100644 --- a/prelude/rules_impl.bzl +++ b/prelude/rules_impl.bzl @@ -593,7 +593,7 @@ inlined_extra_attributes = { "python_bootstrap_binary": { "deps": attrs.list(attrs.dep(providers = [PythonBootstrapSources]), default = []), "main": attrs.source(), - "copy_deps": attrs.bool(default = False), + "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(