From b451db7a006085b3bf2564a5ffcd6985daf98324 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Dec 2025 23:47:26 -0800 Subject: [PATCH] refactor: remove semantics --- python/private/common.bzl | 83 +----------------------------- python/private/py_executable.bzl | 88 +++++++------------------------- python/private/py_library.bzl | 27 +++------- 3 files changed, 28 insertions(+), 170 deletions(-) diff --git a/python/private/common.bzl b/python/private/common.bzl index ddeea6ed2d..19f2f39215 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl @@ -43,22 +43,10 @@ PYTHON_FILE_EXTENSIONS = [ def create_binary_semantics_struct( *, - create_executable, - get_cc_details_for_binary, get_central_uncachable_version_file, - get_coverage_deps, get_debugger_deps, - get_extra_common_runfiles_for_binary, - get_extra_providers, - get_extra_write_build_data_env, - get_interpreter_path, - get_imports, get_native_deps_dso_name, - get_native_deps_user_link_flags, - get_stamp_flag, - maybe_precompile, should_build_native_deps_dso, - should_create_init_files, should_include_build_data): """Helper to ensure a semantics struct has all necessary fields. @@ -66,42 +54,15 @@ def create_binary_semantics_struct( the necessary functions are being correctly provided. Args: - create_executable: Callable; creates a binary's executable output. See - py_executable.bzl#py_executable_base_impl for details. - get_cc_details_for_binary: Callable that returns a `CcDetails` struct; see - `create_cc_detail_struct`. get_central_uncachable_version_file: Callable that returns an optional Artifact; this artifact is special: it is never cached and is a copy of `ctx.version_file`; see py_builtins.copy_without_caching - get_coverage_deps: Callable that returns a list of Targets for making - coverage work; only called if coverage is enabled. get_debugger_deps: Callable that returns a list of Targets that provide custom debugger support; only called for target-configuration. - get_extra_common_runfiles_for_binary: Callable that returns a runfiles - object of extra runfiles a binary should include. - get_extra_providers: Callable that returns extra providers; see - py_executable.bzl#_create_providers for details. - get_extra_write_build_data_env: Callable that returns a dict[str, str] - of additional environment variable to pass to build data generation. - get_interpreter_path: Callable that returns an optional string, which is - the path to the Python interpreter to use for running the binary. - get_imports: Callable that returns a list of the target's import - paths (from the `imports` attribute, so just the target's own import - path strings, not from dependencies). get_native_deps_dso_name: Callable that returns a string, which is the basename (with extension) of the native deps DSO library. - get_native_deps_user_link_flags: Callable that returns a list of strings, - which are any extra linker flags to pass onto the native deps DSO - linking action. - get_stamp_flag: Callable that returns bool of if the --stamp flag was - enabled or not. - maybe_precompile: Callable that may optional precompile the input `.py` - sources and returns the full set of desired outputs derived from - the source files (e.g., both py and pyc, only one of them, etc). should_build_native_deps_dso: Callable that returns bool; True if building a native deps DSO is supported, False if not. - should_create_init_files: Callable that returns bool; True if - `__init__.py` files should be generated, False if not. should_include_build_data: Callable that returns bool; True if build data should be generated, False if not. Returns: @@ -109,50 +70,13 @@ def create_binary_semantics_struct( """ return struct( # keep-sorted - create_executable = create_executable, - get_cc_details_for_binary = get_cc_details_for_binary, get_central_uncachable_version_file = get_central_uncachable_version_file, - get_coverage_deps = get_coverage_deps, get_debugger_deps = get_debugger_deps, - get_extra_common_runfiles_for_binary = get_extra_common_runfiles_for_binary, - get_extra_providers = get_extra_providers, - get_extra_write_build_data_env = get_extra_write_build_data_env, - get_imports = get_imports, - get_interpreter_path = get_interpreter_path, get_native_deps_dso_name = get_native_deps_dso_name, - get_native_deps_user_link_flags = get_native_deps_user_link_flags, - get_stamp_flag = get_stamp_flag, - maybe_precompile = maybe_precompile, should_build_native_deps_dso = should_build_native_deps_dso, - should_create_init_files = should_create_init_files, should_include_build_data = should_include_build_data, ) -def create_library_semantics_struct( - *, - get_cc_info_for_library, - get_imports, - maybe_precompile): - """Create a `LibrarySemantics` struct. - - Call this instead of a raw call to `struct(...)`; it'll help ensure all - the necessary functions are being correctly provided. - - Args: - get_cc_info_for_library: Callable that returns a CcInfo for the library; - see py_library_impl for arg details. - get_imports: Callable; see create_binary_semantics_struct. - maybe_precompile: Callable; see create_binary_semantics_struct. - Returns: - a `LibrarySemantics` struct. - """ - return struct( - # keep sorted - get_cc_info_for_library = get_cc_info_for_library, - get_imports = get_imports, - maybe_precompile = maybe_precompile, - ) - def create_cc_details_struct( *, cc_info_for_propagating, @@ -255,12 +179,11 @@ def collect_cc_info(ctx, extra_deps = []): return cc_common.merge_cc_infos(cc_infos = cc_infos) -def collect_imports(ctx, semantics): +def collect_imports(ctx): """Collect the direct and transitive `imports` strings. Args: ctx: {type}`ctx` the current target ctx - semantics: semantics object for fetching direct imports. Returns: {type}`depset[str]` of import paths @@ -271,13 +194,11 @@ def collect_imports(ctx, semantics): transitive.append(dep[PyInfo].imports) if BuiltinPyInfo != None and BuiltinPyInfo in dep: transitive.append(dep[BuiltinPyInfo].imports) - return depset(direct = semantics.get_imports(ctx), transitive = transitive) + return depset(direct = get_imports(ctx), transitive = transitive) def get_imports(ctx): """Gets the imports from a rule's `imports` attribute. - See create_binary_semantics_struct for details about this function. - Args: ctx: Rule ctx. diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 9084454c65..2e167b99ab 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -47,7 +47,6 @@ load( "create_py_info", "csv", "filter_to_py_srcs", - "get_imports", "is_bool", "relative_path", "runfiles_root_path", @@ -267,42 +266,18 @@ def py_executable_impl(ctx, *, is_test, inherited_environment): def create_binary_semantics(): return create_binary_semantics_struct( # keep-sorted start - create_executable = _create_executable, - get_cc_details_for_binary = _get_cc_details_for_binary, get_central_uncachable_version_file = lambda ctx: None, - get_coverage_deps = _get_coverage_deps, get_debugger_deps = _get_debugger_deps, - get_extra_common_runfiles_for_binary = lambda ctx: ctx.runfiles(), - get_extra_providers = _get_extra_providers, - get_extra_write_build_data_env = lambda ctx: {}, - get_imports = get_imports, - get_interpreter_path = _get_interpreter_path, get_native_deps_dso_name = _get_native_deps_dso_name, - get_native_deps_user_link_flags = _get_native_deps_user_link_flags, - get_stamp_flag = _get_stamp_flag, - maybe_precompile = maybe_precompile, should_build_native_deps_dso = lambda ctx: False, - should_create_init_files = _should_create_init_files, should_include_build_data = lambda ctx: False, # keep-sorted end ) -def _get_coverage_deps(ctx, runtime_details): - _ = ctx, runtime_details # @unused - return [] - def _get_debugger_deps(ctx, runtime_details): _ = ctx, runtime_details # @unused return [] -def _get_extra_providers(ctx, main_py, runtime_details): - _ = ctx, main_py, runtime_details # @unused - return [] - -def _get_stamp_flag(ctx): - # NOTE: Undocumented API; private to builtins - return ctx.configuration.stamp_binaries - def _should_create_init_files(ctx): if ctx.attr.legacy_create_init == -1: return not read_possibly_native_flag(ctx, "default_to_explicit_init_py") @@ -994,10 +969,6 @@ def _get_native_deps_dso_name(ctx): _ = ctx # @unused fail("Building native deps DSO not supported.") -def _get_native_deps_user_link_flags(ctx): - _ = ctx # @unused - fail("Building native deps DSO not supported.") - def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = []): """Base rule implementation for a Python executable. @@ -1022,7 +993,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = else: main_py = None direct_sources = filter_to_py_srcs(ctx.files.srcs) - precompile_result = semantics.maybe_precompile(ctx, direct_sources) + precompile_result = maybe_precompile(ctx, direct_sources) required_py_files = precompile_result.keep_srcs required_pyc_files = [] @@ -1046,20 +1017,17 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = default_outputs.add(precompile_result.keep_srcs) default_outputs.add(required_pyc_files) - imports = collect_imports(ctx, semantics) + imports = collect_imports(ctx) - runtime_details = _get_runtime_details(ctx, semantics) - if ctx.configuration.coverage_enabled: - extra_deps = semantics.get_coverage_deps(ctx, runtime_details) - else: - extra_deps = [] + runtime_details = _get_runtime_details(ctx) + extra_deps = [] # The debugger dependency should be prevented by select() config elsewhere, # but just to be safe, also guard against adding it to the output here. if not _is_tool_config(ctx): extra_deps.extend(semantics.get_debugger_deps(ctx, runtime_details)) - cc_details = semantics.get_cc_details_for_binary(ctx, extra_deps = extra_deps) + cc_details = _get_cc_details_for_binary(ctx, extra_deps = extra_deps) native_deps_details = _get_native_deps_details( ctx, semantics = semantics, @@ -1078,11 +1046,10 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = runtime_details.runfiles, cc_details.extra_runfiles, native_deps_details.runfiles, - semantics.get_extra_common_runfiles_for_binary(ctx), ], semantics = semantics, ) - exec_result = semantics.create_executable( + exec_result = _create_executable( ctx, executable = executable, main_py = main_py, @@ -1122,7 +1089,6 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = runtime_details = runtime_details, cc_info = cc_details.cc_info_for_propagating, inherited_environment = inherited_environment, - semantics = semantics, output_groups = exec_result.output_groups, ) @@ -1154,7 +1120,7 @@ def _declare_executable_file(ctx): return executable -def _get_runtime_details(ctx, semantics): +def _get_runtime_details(ctx): """Gets various information about the Python runtime to use. While most information comes from the toolchain, various legacy and @@ -1162,7 +1128,6 @@ def _get_runtime_details(ctx, semantics): Args: ctx: Rule ctx - semantics: A `BinarySemantics` struct; see `create_binary_semantics_struct` Returns: A struct; see inline-field comments of the return value for details. @@ -1203,7 +1168,7 @@ def _get_runtime_details(ctx, semantics): else: runtime_files = depset() - executable_interpreter_path = semantics.get_interpreter_path( + executable_interpreter_path = _get_interpreter_path( ctx, runtime = effective_runtime, flag_interpreter_path = flag_interpreter_path, @@ -1336,7 +1301,7 @@ def _get_base_runfiles_for_binary( common_runfiles = common_runfiles.build(ctx) - if semantics.should_create_init_files(ctx): + if _should_create_init_files(ctx): common_runfiles = _py_builtins.merge_runfiles_with_generated_inits_empty_files_supplier( ctx = ctx, runfiles = common_runfiles, @@ -1352,11 +1317,10 @@ def _get_base_runfiles_for_binary( # removed and another way found to locate the underlying build data file. data_runfiles = runfiles_with_exe - if is_stamping_enabled(ctx, semantics) and semantics.should_include_build_data(ctx): + if is_stamping_enabled(ctx) and semantics.should_include_build_data(ctx): build_data_file, build_data_runfiles = _create_runfiles_with_build_data( ctx, semantics.get_central_uncachable_version_file(ctx), - semantics.get_extra_write_build_data_env(ctx), ) default_runfiles = runfiles_with_exe.merge(build_data_runfiles) else: @@ -1372,19 +1336,17 @@ def _get_base_runfiles_for_binary( def _create_runfiles_with_build_data( ctx, - central_uncachable_version_file, - extra_write_build_data_env): + central_uncachable_version_file): build_data_file = _write_build_data( ctx, central_uncachable_version_file, - extra_write_build_data_env, ) build_data_runfiles = ctx.runfiles(files = [ build_data_file, ]) return build_data_file, build_data_runfiles -def _write_build_data(ctx, central_uncachable_version_file, extra_write_build_data_env): +def _write_build_data(ctx, central_uncachable_version_file): # TODO: Remove this logic when a central file is always available if not central_uncachable_version_file: version_file = ctx.actions.declare_file(ctx.label.name + "-uncachable_version_file.txt") @@ -1430,7 +1392,7 @@ def _write_build_data(ctx, central_uncachable_version_file, extra_write_build_da ctx.actions.run( executable = ctx.executable._build_data_gen, - env = dicts.add({ + env = { # NOTE: ctx.info_file is undocumented; see # https://github.com/bazelbuild/bazel/issues/9363 "INFO_FILE": ctx.info_file.path, @@ -1438,7 +1400,7 @@ def _write_build_data(ctx, central_uncachable_version_file, extra_write_build_da "PLATFORM": cc_helper.find_cpp_toolchain(ctx).toolchain_id, "TARGET": str(ctx.label), "VERSION_FILE": version_file.path, - }, extra_write_build_data_env), + }, inputs = depset( direct = direct_inputs, ), @@ -1489,12 +1451,9 @@ def _get_native_deps_details(ctx, *, semantics, cc_details, is_test): feature_configuration = cc_feature_config.feature_configuration, cc_toolchain = cc_details.cc_toolchain, test_only_target = is_test, # private - stamp = 1 if is_stamping_enabled(ctx, semantics) else 0, + stamp = 1 if is_stamping_enabled(ctx) else 0, main_output = linked_lib, # private use_shareable_artifact_factory = True, # private - # NOTE: Only flags not captured by cc_info.linking_context need to - # be manually passed - user_link_flags = semantics.get_native_deps_user_link_flags(ctx), ) return struct( dso = dso, @@ -1634,12 +1593,11 @@ def _path_endswith(path, endswith): # "ab/c.py".endswith("b/c.py") from incorrectly matching. return ("/" + path).endswith("/" + endswith) -def is_stamping_enabled(ctx, semantics): +def is_stamping_enabled(ctx): """Tells if stamping is enabled or not. Args: ctx: The rule ctx - semantics: a semantics struct (see create_semantics_struct). Returns: bool; True if stamping is enabled, False if not. """ @@ -1652,7 +1610,8 @@ def is_stamping_enabled(ctx, semantics): elif stamp == 0: return False elif stamp == -1: - return semantics.get_stamp_flag(ctx) + # NOTE: Undocumented API; private to builtins + return ctx.configuration.stamp_binaries else: fail("Unsupported `stamp` value: {}".format(stamp)) @@ -1678,8 +1637,7 @@ def _create_providers( cc_info, inherited_environment, runtime_details, - output_groups, - semantics): + output_groups): """Creates the providers an executable should return. Args: @@ -1708,7 +1666,6 @@ def _create_providers( is run within. runtime_details: struct of runtime information; see _get_runtime_details() output_groups: dict[str, depset[File]]; used to create OutputGroupInfo - semantics: BinarySemantics struct; see create_binary_semantics() Returns: A list of modern providers. @@ -1783,13 +1740,6 @@ def _create_providers( if builtin_py_info: providers.append(builtin_py_info) providers.append(create_output_group_info(py_info.transitive_sources, output_groups)) - - extra_providers = semantics.get_extra_providers( - ctx, - main_py = main_py, - runtime_details = runtime_details, - ) - providers.extend(extra_providers) return providers def _create_run_environment_info(ctx, inherited_environment): diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 6edb25abae..de73a53720 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -32,11 +32,9 @@ load( "collect_imports", "collect_runfiles", "create_instrumented_files_info", - "create_library_semantics_struct", "create_output_group_info", "create_py_info", "filter_to_py_srcs", - "get_imports", ) load(":common_labels.bzl", "labels") load(":flags.bzl", "AddSrcsToRunfilesFlag", "PrecompileFlag", "VenvsSitePackages") @@ -121,29 +119,18 @@ This allows optimizing the generation of symlinks to be cheaper at analysis time }, ) -def _py_library_impl_with_semantics(ctx): - return py_library_impl( - ctx, - semantics = create_library_semantics_struct( - get_imports = get_imports, - maybe_precompile = maybe_precompile, - get_cc_info_for_library = collect_cc_info, - ), - ) - -def py_library_impl(ctx, *, semantics): +def py_library_impl(ctx): """Abstract implementation of py_library rule. Args: ctx: The rule ctx - semantics: A `LibrarySemantics` struct; see `create_library_semantics_struct` Returns: A list of modern providers to propagate. """ direct_sources = filter_to_py_srcs(ctx.files.srcs) - precompile_result = semantics.maybe_precompile(ctx, direct_sources) + precompile_result = maybe_precompile(ctx, direct_sources) required_py_files = precompile_result.keep_srcs required_pyc_files = [] @@ -172,9 +159,9 @@ def py_library_impl(ctx, *, semantics): imports = [] venv_symlinks = [] - imports, venv_symlinks = _get_imports_and_venv_symlinks(ctx, semantics) + imports, venv_symlinks = _get_imports_and_venv_symlinks(ctx) - cc_info = semantics.get_cc_info_for_library(ctx) + cc_info = collect_cc_info(ctx) py_info, builtins_py_info = create_py_info( ctx, original_sources = direct_sources, @@ -243,7 +230,7 @@ def _get_package_and_version(ctx): version.normalize(version_str), # will have no dashes either ) -def _get_imports_and_venv_symlinks(ctx, semantics): +def _get_imports_and_venv_symlinks(ctx): imports = depset() venv_symlinks = [] if VenvsSitePackages.is_enabled(ctx): @@ -268,7 +255,7 @@ def _get_imports_and_venv_symlinks(ctx, semantics): namespace_package_files = ctx.files.namespace_package_files, ) else: - imports = collect_imports(ctx, semantics) + imports = collect_imports(ctx) return imports, venv_symlinks _MaybeBuiltinPyInfo = [BuiltinPyInfo] if BuiltinPyInfo != None else [] @@ -288,7 +275,7 @@ def create_py_library_rule_builder(): for creating a `py_library` rule. """ builder = ruleb.Rule( - implementation = _py_library_impl_with_semantics, + implementation = py_library_impl, doc = _DEFAULT_PY_LIBRARY_DOC, exec_groups = dict(REQUIRED_EXEC_GROUP_BUILDERS), attrs = LIBRARY_ATTRS,