Skip to content

Commit

Permalink
Back out "Back out "[buck2][python][prelude] Do not include live pars…
Browse files Browse the repository at this point in the history
… in standalone resources""

Summary:
Original commit changeset: 0bd14da72162
"
With inplace by default enabled if a python binary is added as a resource the link tree would included as a hidden_resource. This caused standalone builds to fail - since we do not allow hidden resources in standalone pars.
This diff checks the resources to see if we are including a python binary. If so we add the standalone output as resource.
We track default_resources so that when building an inplace par we can include resources as inplace.
https://fb.workplace.com/groups/python.builds/permalink/3888593271462086/
S461975
#buildmore
"

Reviewed By: cxxxs

Differential Revision: D65785835

fbshipit-source-id: c8c6cb6f7e7c2755f36e4e43cc9d83f7f58edef8
  • Loading branch information
BrandonTheBuilder authored and facebook-github-bot committed Nov 12, 2024
1 parent 33f5824 commit d975873
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 63 deletions.
10 changes: 2 additions & 8 deletions python/interface.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ PythonLibraryInterface = record(
# efficient calls, such as using t-sets projections.
# PythonLibraryManifestsInterface
manifests = field(typing.Callable),

# Returns whether this Python library includes hidden resources.
# bool
has_hidden_resources = field(typing.Callable),

# Converts the hidden resources in this Python library to arguments.
# _arglike of hidden resources
hidden_resources = field(typing.Callable),
)

PythonLibraryManifestsInterface = record(
Expand Down Expand Up @@ -73,6 +65,8 @@ PythonLibraryManifestsInterface = record(
# [_arglike] of resource artifacts
resource_artifacts = field(typing.Callable),
resource_artifacts_with_paths = field(typing.Callable),
hidden_resources = field(typing.Callable),
has_hidden_resources = field(typing.Callable),
)

# Entry point for Python binaries. First component designates if the second
Expand Down
38 changes: 18 additions & 20 deletions python/make_py_package.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ def make_py_package(
pex_modules: PexModules,
shared_libraries: list[(str, SharedLibrary, bool)],
main: EntryPoint,
hidden_resources: list[ArgLike] | None,
allow_cache_upload: bool,
debuginfo_files: list[(str | (str, SharedLibrary, str), Artifact)] = []) -> PexProviders:
"""
Expand All @@ -146,7 +145,6 @@ def make_py_package(
artifact and whether they should be preloaded.
- main: the name of the entry point to execute when running the
resulting binary.
- hidden_resources: extra resources the binary depends on.
"""
srcs = []
srcs.extend(pex_modules.manifests.src_manifests())
Expand Down Expand Up @@ -184,7 +182,6 @@ def make_py_package(
dep_artifacts,
debug_artifacts,
main,
hidden_resources,
manifest_module,
pex_modules,
output_suffix = "",
Expand Down Expand Up @@ -228,7 +225,6 @@ def make_py_package(
repl_dep_artifacts,
repl_debug_artifacts,
(EntryPointKind("function"), ctx.attrs.repl_main),
hidden_resources,
manifest_module,
pex_modules,
output_suffix = "-repl",
Expand All @@ -249,7 +245,6 @@ def make_py_package(
dep_artifacts,
debug_artifacts,
main,
hidden_resources,
manifest_module,
pex_modules,
output_suffix = "-{}".format(style),
Expand All @@ -274,7 +269,6 @@ def _make_py_package_impl(
dep_artifacts: list[ArgLike],
debug_artifacts: list[(str | (str, SharedLibrary, str), ArgLike)],
main: EntryPoint,
hidden_resources: list[ArgLike] | None,
manifest_module: ArgLike | None,
pex_modules: PexModules,
output_suffix: str,
Expand All @@ -284,11 +278,14 @@ def _make_py_package_impl(

runtime_files = []
sub_targets = {}
if standalone and hidden_resources != None:
hidden_resources = []
if standalone and pex_modules.manifests.has_hidden_resources(standalone):
# constructing this error message is expensive, only do it when we abort analysis
error_msg = "standalone builds don't support hidden resources" if output_suffix else _hidden_resources_error_message(ctx.label, hidden_resources)
error_msg = "standalone builds don't support hidden resources" if output_suffix else _hidden_resources_error_message(ctx.label, pex_modules.manifests.hidden_resources(standalone))

return _fail(ctx, python_toolchain, output_suffix, error_msg)
else:
hidden_resources = pex_modules.manifests.hidden_resources(standalone)

if not (standalone or
package_style == PackageStyle("inplace") or
Expand All @@ -313,6 +310,7 @@ def _make_py_package_impl(
common_modules_args,
dep_artifacts,
debug_artifacts,
standalone,
symlink_tree_path,
manifest_module,
pex_modules,
Expand Down Expand Up @@ -340,6 +338,7 @@ def _make_py_package_impl(
# For inplace builds add local artifacts to outputs so they get properly materialized
runtime_files.extend(dep_artifacts)
runtime_files.append(symlink_tree_path)
runtime_files.extend([a[0] for a in pex_modules.manifests.resource_artifacts_with_paths(standalone)])

# For standalone builds, or builds setting make_py_package we generate args for calling make_par.py
if standalone or make_py_package_cmd != None:
Expand Down Expand Up @@ -385,9 +384,6 @@ def _make_py_package_impl(
run_args.append(ctx.attrs._python_toolchain[PythonToolchainInfo].interpreter)
run_args.append(output)

if hidden_resources == None:
hidden_resources = []

if symlink_tree_path != None:
sub_targets["link-tree"] = [DefaultInfo(
default_output = symlink_tree_path,
Expand Down Expand Up @@ -516,18 +512,11 @@ def _pex_modules_common_args(
srcs.extend(extra_manifests)

deps.extend([a[0] for a in src_artifacts])
resources = pex_modules.manifests.resource_manifests()
deps.extend([a[0] for a in pex_modules.manifests.resource_artifacts_with_paths()])

src_manifests_path = ctx.actions.write(
"__src_manifests{}.txt".format(suffix),
_srcs(srcs, format = "--module-manifest={}"),
)
resource_manifests_path = ctx.actions.write(
"__resource_manifests{}.txt".format(suffix),
_srcs(resources, format = "--resource-manifest={}"),
)

native_libraries = gen_shared_libs_action(
actions = ctx.actions,
out = "__native_libraries{}__.txt".format(suffix),
Expand All @@ -548,11 +537,9 @@ def _pex_modules_common_args(
)

src_manifest_args = cmd_args(src_manifests_path, hidden = srcs)
resource_manifest_args = cmd_args(resource_manifests_path, hidden = resources)

cmd = cmd_args()
cmd.add(cmd_args(src_manifest_args, format = "@{}"))
cmd.add(cmd_args(resource_manifest_args, format = "@{}"))
cmd.add(cmd_args(native_libraries, format = "@{}"))

if debuginfo_files:
Expand Down Expand Up @@ -626,6 +613,7 @@ def _pex_modules_args(
common_args: cmd_args,
dep_artifacts: list[ArgLike],
debug_artifacts: list[(str | (str, SharedLibrary, str), ArgLike)],
is_standalone: bool,
symlink_tree_path: Artifact | None,
manifest_module: ArgLike | None,
pex_modules: PexModules,
Expand Down Expand Up @@ -668,6 +656,16 @@ def _pex_modules_args(

hidden.extend([s for _, s in debug_artifacts])

resources = pex_modules.manifests.resource_manifests(is_standalone)
if resources:
hidden.extend([a[0] for a in pex_modules.manifests.resource_artifacts_with_paths(is_standalone)])
resource_manifests_path = ctx.actions.write(
"__resource_manifests{}.txt".format(output_suffix),
_srcs(resources, format = "--resource-manifest={}"),
)
resource_manifest_args = cmd_args(resource_manifests_path, hidden = resources)
cmd.append(cmd_args(resource_manifest_args, format = "@{}"))

return cmd_args(cmd, hidden = hidden)

def _hidden_resources_error_message(current_target: Label, hidden_resources: list[ArgLike] | None) -> str:
Expand Down
82 changes: 69 additions & 13 deletions python/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ PythonLibraryManifests = record(
label = field(Label),
srcs = field([ManifestInfo, None]),
src_types = field([ManifestInfo, None], None),
resources = field([(ManifestInfo, list[ArgLike]), None]),
default_resources = field([(ManifestInfo, list[ArgLike]), None]),
standalone_resources = field([(ManifestInfo, list[ArgLike]), None]),
bytecode = field([dict[PycInvalidationMode, ManifestInfo], None]),
dep_manifest = field([ManifestInfo, None]),
extensions = field([dict[str, typing.Any], None]),
Expand All @@ -42,25 +43,46 @@ def _dep_artifacts(value: PythonLibraryManifests):
return value.dep_manifest.artifacts

def _hidden_resources(value: PythonLibraryManifests):
if value.resources == None:
if value.default_resources == None:
return []
return value.resources[1]
return value.default_resources[1]

def _has_hidden_resources(children: list[bool], value: [PythonLibraryManifests, None]):
if value:
if value.resources and len(value.resources[1]) > 0:
if value.default_resources and len(value.default_resources[1]) > 0:
return True
return any(children)

def _resource_manifests(value: PythonLibraryManifests):
if value.resources == None:
if value.default_resources == None:
return []
return value.resources[0].manifest
return value.default_resources[0].manifest

def _resource_artifacts(value: PythonLibraryManifests):
if value.resources == None:
if value.default_resources == None:
return []
return [a for a, _ in value.resources[0].artifacts]
return [a for a, _ in value.default_resources[0].artifacts]

def _standalone_hidden_resources(value: PythonLibraryManifests):
if value.standalone_resources == None:
return []
return value.standalone_resources[1]

def _standalone_has_hidden_resources(children: list[bool], value: [PythonLibraryManifests, None]):
if value:
if value.standalone_resources and len(value.standalone_resources[1]) > 0:
return True
return any(children)

def _standalone_resource_manifests(value: PythonLibraryManifests):
if value.standalone_resources == None:
return []
return value.standalone_resources[0].manifest

def _standalone_resource_artifacts(value: PythonLibraryManifests):
if value.standalone_resources == None:
return []
return [a for a, _ in value.standalone_resources[0].artifacts]

def _source_manifests(value: PythonLibraryManifests):
if value.srcs == None:
Expand Down Expand Up @@ -102,6 +124,9 @@ args_projections = {
"source_manifests": _source_manifests,
"source_type_artifacts": _source_type_artifacts,
"source_type_manifests": _source_type_manifests,
"standalone_hidden_resources": _standalone_hidden_resources,
"standalone_resource_artifacts": _standalone_resource_artifacts,
"standalone_resource_manifests": _standalone_resource_manifests,
}
args_projections.update({
"{}_artifacts".format(prefix): _bytecode_artifacts(mode)
Expand All @@ -119,6 +144,7 @@ PythonLibraryManifestsTSet = transitive_set(
},
reductions = {
"has_hidden_resources": _has_hidden_resources,
"standalone_has_hidden_resources": _standalone_has_hidden_resources,
},
)

Expand All @@ -136,10 +162,38 @@ def info_to_interface(info: PythonLibraryInfo) -> PythonLibraryInterface:
extension_shared_libraries = lambda: traverse_shared_library_info(info.extension_shared_libraries),
iter_manifests = lambda: info.manifests.traverse(),
manifests = lambda: manifests_to_interface(info.manifests),
has_hidden_resources = lambda: info.manifests.reduce("has_hidden_resources"),
hidden_resources = lambda: [info.manifests.project_as_args("hidden_resources")],
)

def _get_resource_manifests(standalone: typing.Any, manifests: typing.Any) -> typing.Any:
if standalone:
return [manifests.project_as_args("standalone_resource_manifests")]
else:
return [manifests.project_as_args("resource_manifests")]

def _get_resource_artifacts(standalone: typing.Any, manifests: typing.Any) -> typing.Any:
if standalone:
return [manifests.project_as_args("standalone_resource_artifacts")]
else:
return [manifests.project_as_args("resource_artifacts")]

def _get_hidden_resources(standalone: typing.Any, manifests: typing.Any) -> typing.Any:
if standalone:
return [manifests.project_as_args("standalone_hidden_resources")]
else:
return [manifests.project_as_args("hidden_resources")]

def _get_resource_artifacts_with_path(standalone: typing.Any, manifests: typing.Any) -> typing.Any:
if standalone:
return [(a, p) for m in manifests.traverse() if m != None and m.standalone_resources != None for a, p in m.standalone_resources[0].artifacts]
else:
return [(a, p) for m in manifests.traverse() if m != None and m.default_resources != None for a, p in m.default_resources[0].artifacts]

def _get_has_hidden_resources(standalone: typing.Any, manifests: typing.Any) -> typing.Any:
if standalone:
return manifests.reduce("standalone_has_hidden_resources")
else:
return manifests.reduce("has_hidden_resources")

def manifests_to_interface(manifests: PythonLibraryManifestsTSet) -> PythonLibraryManifestsInterface:
return PythonLibraryManifestsInterface(
src_manifests = lambda: [manifests.project_as_args("source_manifests")],
Expand All @@ -151,7 +205,9 @@ def manifests_to_interface(manifests: PythonLibraryManifestsTSet) -> PythonLibra
bytecode_manifests = lambda mode: [manifests.project_as_args("{}_manifests".format(_BYTECODE_PROJ_PREFIX[mode]))],
bytecode_artifacts = lambda mode: [manifests.project_as_args("{}_artifacts".format(_BYTECODE_PROJ_PREFIX[mode]))],
bytecode_artifacts_with_paths = lambda mode: [(a, p) for m in manifests.traverse() if m != None and m.bytecode != None for a, p in m.bytecode[mode].artifacts],
resource_manifests = lambda: [manifests.project_as_args("resource_manifests")],
resource_artifacts = lambda: [manifests.project_as_args("resource_artifacts")],
resource_artifacts_with_paths = lambda: [(a, p) for m in manifests.traverse() if m != None and m.resources != None for a, p in m.resources[0].artifacts],
resource_manifests = lambda standalone: _get_resource_manifests(standalone, manifests),
resource_artifacts = lambda standalone: _get_resource_artifacts(standalone, manifests),
resource_artifacts_with_paths = lambda standalone: _get_resource_artifacts_with_path(standalone, manifests),
has_hidden_resources = lambda standalone: _get_has_hidden_resources(standalone, manifests),
hidden_resources = lambda standalone: _get_hidden_resources(standalone, manifests),
)
30 changes: 20 additions & 10 deletions python/python_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ def python_executable(
ctx: AnalysisContext,
main: EntryPoint,
srcs: dict[str, Artifact],
resources: dict[str, ArtifactOutputs],
default_resources: dict[str, ArtifactOutputs],
standalone_resources: dict[str, ArtifactOutputs] | None,
compile: bool,
allow_cache_upload: bool) -> PexProviders:
# Returns a three tuple: the Python binary, all its potential runtime files,
Expand Down Expand Up @@ -357,19 +358,28 @@ def python_executable(
if python_toolchain.emit_dependency_metadata and srcs:
dep_manifest = create_dep_manifest_for_source_map(ctx, python_toolchain, srcs)

all_resources = {}
all_resources.update(resources)
all_default_resources = {}
all_standalone_resources = {}
cxx_extra_resources = {}
for cxx_resources in gather_resources(ctx.label, deps = raw_deps).values():
for name, resource in cxx_resources.items():
all_resources[paths.join("__cxx_resources__", name)] = resource
cxx_extra_resources[paths.join("__cxx_resources__", name)] = resource
all_default_resources.update(cxx_extra_resources)
all_standalone_resources.update(cxx_extra_resources)

if default_resources:
all_default_resources.update(default_resources)
if standalone_resources:
all_standalone_resources.update(standalone_resources)

library_info = create_python_library_info(
ctx.actions,
ctx.label,
srcs = src_manifest,
src_types = src_manifest,
dep_manifest = dep_manifest,
resources = py_resources(ctx, all_resources) if all_resources else None,
default_resources = py_resources(ctx, all_default_resources) if all_default_resources else None,
standalone_resources = py_resources(ctx, all_standalone_resources, "_standalone") if all_standalone_resources else None,
bytecode = bytecode_manifest,
deps = python_deps,
shared_libraries = shared_deps,
Expand Down Expand Up @@ -786,8 +796,6 @@ def _convert_python_library_to_executable(
) if extensions else None,
)

hidden_resources = library.hidden_resources() if library.has_hidden_resources() else None

# Build the PEX.
pex = make_py_package(
ctx = ctx,
Expand All @@ -798,7 +806,6 @@ def _convert_python_library_to_executable(
pex_modules = pex_modules,
shared_libraries = shared_libs,
main = main,
hidden_resources = hidden_resources,
allow_cache_upload = allow_cache_upload,
debuginfo_files = debuginfo_files,
)
Expand Down Expand Up @@ -842,13 +849,16 @@ def python_binary_impl(ctx: AnalysisContext) -> list[Provider]:
if ctx.attrs.main != None:
srcs[ctx.attrs.main.short_path] = ctx.attrs.main
srcs = qualify_srcs(ctx.label, ctx.attrs.base_module, srcs)
resources = qualify_srcs(ctx.label, ctx.attrs.base_module, py_attr_resources(ctx))
default_resources_map, standalone_resources_map = py_attr_resources(ctx)
standalone_resources = qualify_srcs(ctx.label, ctx.attrs.base_module, standalone_resources_map)
default_resources = qualify_srcs(ctx.label, ctx.attrs.base_module, default_resources_map)

pex = python_executable(
ctx,
main,
srcs,
resources,
default_resources,
standalone_resources,
compile = value_or(ctx.attrs.compile, False),
allow_cache_upload = cxx_attrs_get_allow_cache_upload(ctx.attrs),
)
Expand Down
Loading

0 comments on commit d975873

Please sign in to comment.