diff --git a/.bazelrc b/.bazelrc index 49f98ad7a1..771bf0fe90 100644 --- a/.bazelrc +++ b/.bazelrc @@ -42,6 +42,7 @@ common --enable_bzlmod # Local disk cache greatly speeds up builds if the regular cache is lost common --disk_cache=~/.cache/bazel/bazel-disk-cache + # Additional config to use for readthedocs builds. # See .readthedocs.yml for additional flags that can only be determined from # the runtime environment. @@ -54,3 +55,6 @@ common --incompatible_no_implicit_file_export build --lockfile_mode=update +import %workspace%/specialized_configs.bazelrc + +try-import user.bazelrc diff --git a/CHANGELOG.md b/CHANGELOG.md index 7734a45fe4..f487194929 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,9 @@ END_UNRELEASED_TEMPLATE --windows_enable_symlinks` to your `.bazelrc` to enable Bazel using full symlink support on Windows. * venv-based binaries are created by default ({obj}`--bootstrap_impl=system_python`) - on supported platforms (Linux/Mac with Bazel 8+). + on supported platforms (Linux/Mac with Bazel 8+, or Windows). +* `--build_python_zip` on Windows is ignored. Use {obj}`py_zipapp_binary` to create + zips of Python programs. Other changes: * (pypi) Update dependencies used for `compile_pip_requirements`, building @@ -73,19 +75,21 @@ Other changes: we will from now on fetch the lists of available packages on each index. The used package mappings will be written as facts to the `MODULE.bazel.lock` file on supported bazel versions and it should be done at most once. As a result, - per-package {obj}`experimental_index_url_overrides` is no longer needed if the index URLs are - passed to the `pip.parse` via `experimental_index_url` and `experimental_extra_index_urls`. - What is more, we start implementing the flags for `--index_url` and `--extra_index_urls` more in - line to how it is used in `uv` and `pip`, i.e. we default to `--index_url` if the package is not - found in `--extra_index_urls`. - Fixes - ([#3260](https://github.com/bazel-contrib/rules_python/issues/3260) and + per-package {obj}`experimental_index_url_overrides` is no longer needed if the + index URLs are passed to the `pip.parse` via `experimental_index_url` and + `experimental_extra_index_urls`. What is more, we start implementing the flags + for `--index_url` and `--extra_index_urls` more in line to how it is used in + `uv` and `pip`, i.e. we default to `--index_url` if the package is not found in + `--extra_index_urls`. Fixes + ([#3260](https://github.com/bazel-contrib/rules_python/issues/3260) and [#2632](https://github.com/bazel-contrib/rules_python/issues/2632)). -* (uv) We will now use the download URL specified in the `uv`'s `dist_manifest.json` - file. If you have redirects or blocking rules as part of your downloader setup, - you may need to adjust them. What is more, the default uv version has been bumped - `0.11.2`. -* (runfiles): We are stopping the type annotation testing with `mypy` for Python 3.9. +* (uv) We will now use the download URL specified in the `uv`'s + `dist_manifest.json` file. If you have redirects or blocking rules as part of + your downloader setup, you may need to adjust them. What is more, the default + uv version has been bumped `0.11.2`. +* (runfiles): Type annotations are no longer tested for Python 3.9. +* Windows no longer defaults to creating a zip file and extracting it; a + symlink-based runfiles tree is created, as on unix-like platforms. {#v0-0-0-fixed} ### Fixed @@ -122,11 +126,18 @@ Other changes: {#v0-0-0-added} ### Added * (pypi) Write SimpleAPI contents to the `MODULE.bazel.lock` file if using - {obj}`experimental_index_url` which should speed up consecutive initializations and should no - longer require the network access if the cache is hydrated. - Implements [#2731](https://github.com/bazel-contrib/rules_python/issues/2731). + {obj}`experimental_index_url` which should speed up consecutive + initializations and should no longer require the network access if the cache is + hydrated. Implements + [#2731](https://github.com/bazel-contrib/rules_python/issues/2731). * (wheel) Specifying a path ending in `/` as a destination in `data_files` will now install file(s) to a folder, preserving their basename. +* Various attributes and fields added to support venvs on Windows: + * {obj}`py_runtime.venv_bin_files` and {obj}`PyRuntime.venv_binfiles` + field added to specify additional Python runtime files Windows needs for + venvs. + * {obj}`PyExecutableInfo.venv_interpreter_runfiles`, and + {obj}`PyExecutableInfo.venv_interpreter_symlinks` adde {#v1-9-0} ## [1.9.0] - 2026-02-21 diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index d860983e22..20b0324894 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -240,6 +240,18 @@ def define_hermetic_runtime_toolchain_impl( _IS_FREETHREADED_YES: "cpython-{major}{minor}t".format(**version_dict), _IS_FREETHREADED_NO: "cpython-{major}{minor}".format(**version_dict), }), + # On Windows, a symlink-style venv requires supporting .dll files. + venv_bin_files = select({ + "@platforms//os:windows": native.glob( + include = [ + "*.dll", + ], + # This must be true because glob empty-ness is checked + # during loading phase, before select() filters it out. + allow_empty = True, + ), + "//conditions:default": [], + }), ) py_runtime_pair( diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index 6e39152c89..37b7d2b130 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -74,6 +74,12 @@ def _norm_path(path): def _symlink_libraries(rctx, logger, libraries, shlib_suffix): """Symlinks the shared libraries into the lib/ directory. + Individual files are symlinked instead of the whole directory because + shared_lib_dirs contains multiple search paths for the shared libraries, + and the python files may be missing from any of those directories, and + any of those directories may include non-python runtime libraries, + as would be the case if LIBDIR were, for example, /usr/lib. + Args: rctx: A repository_ctx object logger: A repo_utils.logger object @@ -81,12 +87,6 @@ def _symlink_libraries(rctx, logger, libraries, shlib_suffix): shlib_suffix: Optional. Ensure that the generated symlinks end with this suffix. Returns: A list of library paths (under lib/) linked by the action. - - Individual files are symlinked instead of the whole directory because - shared_lib_dirs contains multiple search paths for the shared libraries, - and the python files may be missing from any of those directories, and - any of those directories may include non-python runtime libraries, - as would be the case if LIBDIR were, for example, /usr/lib. """ result = [] for source in libraries: diff --git a/python/private/local_runtime_repo_setup.bzl b/python/private/local_runtime_repo_setup.bzl index 0922181ffe..5cb7bda200 100644 --- a/python/private/local_runtime_repo_setup.bzl +++ b/python/private/local_runtime_repo_setup.bzl @@ -153,6 +153,19 @@ def define_local_runtime_toolchain_impl( implementation_name = implementation_name, abi_flags = abi_flags, pyc_tag = "{}-{}{}{}".format(implementation_name, major, minor, abi_flags), + venv_bin_files = select({ + "@platforms//os:windows": native.glob( + include = [ + "lib/*.dll", + # The pdb files just provide debugging information + "lib/*.pdb", + ], + # This must be true because glob empty-ness is checked + # during loading phase, before select() filters it out. + allow_empty = True, + ), + "//conditions:default": [], + }), ) py_runtime_pair( diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 805f95fa4c..6c65bf8f59 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -73,6 +73,30 @@ _EXTERNAL_PATH_PREFIX = "external" _ZIP_RUNFILES_DIRECTORY_NAME = "runfiles" _INIT_PY = "__init__.py" +# buildifier: disable=name-conventions +ExplicitSymlink = provider( + doc = """ +A runfile that should be created as a symlink pointing to a specific location. + +This is only needed on Windows, where Bazel doesn't preserve declare_symlink +with relative paths. This is basically manually captures what using +declare_symlink(), symlink() and runfiles like so would capture: + +``` +link = declare_symlink(...) +link_to_path = relative_path(from=link, to=target) +symlink(output=link, target_path=link_to_path) +runfiles.add([link, target]) +``` +""", + fields = { + "files": "depset[File] of files that should be included if this symlink is used", + "link_to_path": "Path the symlink should point to", + "runfiles_path": "runfiles-root-relative path for the symlink", + "venv_path": "venv-root-relative path for the symlink", + }, +) + # Non-Google-specific attributes for executables # These attributes are for rules that accept Python sources. EXECUTABLE_ATTRS = dicts.add( @@ -374,7 +398,10 @@ def _create_executable( extra_default_outputs = [] # NOTE: --build_python_zip defaults to true on Windows - build_zip_enabled = read_possibly_native_flag(ctx, "build_python_zip") + build_zip_enabled = read_possibly_native_flag(ctx, "build_python_zip") and not is_windows + if is_windows: + # The legacy build_python_zip codepath isn't compatible with full venvs on Windows. + build_zip_enabled = False # When --build_python_zip is enabled, then the zip file becomes # one of the default outputs. @@ -473,8 +500,8 @@ WARNING: Target: {} # The interpreter is added this late in the process so that it isn't # added to the zipped files. - if venv and venv.interpreter: - extra_runfiles = extra_runfiles.merge(ctx.runfiles([venv.interpreter])) + if venv and venv.interpreter_runfiles: + extra_runfiles = extra_runfiles.merge(venv.interpreter_runfiles) return struct( # depset[File] of additional files that should be included as default # outputs. @@ -491,6 +518,10 @@ WARNING: Target: {} app_runfiles = app_runfiles.build(ctx), # File|None; the venv `bin/python3` file, if any. venv_python_exe = venv.interpreter if venv else None, + # runfiles|None; runfiles in the venv for the interpreter + venv_interpreter_runfiles = venv.interpreter_runfiles if venv else None, + # depset[ExplicitSymlink]|None; symlinks that should be created + venv_interpreter_symlinks = venv.interpreter_symlinks if venv else None, ) def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv): @@ -523,25 +554,107 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv): # * https://github.com/python/cpython/blob/main/Modules/getpath.py # * https://github.com/python/cpython/blob/main/Lib/site.py def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root_to_sys_path, extra_deps): - venv = "_{}.venv".format(output_prefix.lstrip("_")) - - # The pyvenv.cfg file must be present to trigger the venv site hooks. - # Because it's paths are expected to be absolute paths, we can't reliably - # put much in it. See https://github.com/python/cpython/issues/83650 - pyvenv_cfg = ctx.actions.declare_file("{}/pyvenv.cfg".format(venv)) - ctx.actions.write(pyvenv_cfg, "") + venv_ctx_rel_root = "_{}.venv".format(output_prefix.lstrip("_")) + runtime = runtime_details.effective_runtime + if runtime.interpreter: + interpreter_actual_path = runfiles_root_path(ctx, runtime.interpreter.short_path) + else: + interpreter_actual_path = runtime.interpreter_path - is_bootstrap_script = BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SCRIPT is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints) + if is_windows: + venv_details = _create_venv_windows( + ctx, + venv_ctx_rel_root = venv_ctx_rel_root, + interpreter_actual_path = interpreter_actual_path, + runtime = runtime, + ) + else: + venv_details = _create_venv_unixy( + ctx, + venv_ctx_rel_root = venv_ctx_rel_root, + interpreter_actual_path = interpreter_actual_path, + runtime = runtime, + ) + + site_packages = "{}/{}".format(venv_ctx_rel_root, venv_details.site_packages) + + pth = ctx.actions.declare_file("{}/bazel.pth".format(site_packages)) + ctx.actions.write(pth, "import _bazel_site_init\n") + + site_init = ctx.actions.declare_file("{}/_bazel_site_init.py".format(site_packages)) + computed_subs = ctx.actions.template_dict() + computed_subs.add_joined("%imports%", imports, join_with = ":", map_each = _map_each_identity) + ctx.actions.expand_template( + template = runtime.site_init_template, + output = site_init, + substitutions = { + "%add_runfiles_root_to_sys_path%": add_runfiles_root_to_sys_path, + "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime), + "%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False", + "%site_init_runfiles_path%": runfiles_root_path(ctx, site_init.short_path), + "%workspace_name%": ctx.workspace_name, + }, + computed_substitutions = computed_subs, + ) + venv_dir_map = { + VenvSymlinkKind.BIN: venv_details.bin_dir, + VenvSymlinkKind.LIB: site_packages, + } + venv_app_files = create_venv_app_files( + ctx, + deps = collect_deps(ctx, extra_deps), + venv_dir_map = venv_dir_map, + ) + + files_without_interpreter = [pth, site_init] + venv_app_files.venv_files + if venv_details.pyvenv_cfg: + files_without_interpreter.append(venv_details.pyvenv_cfg) + + return struct( + # File or None; the `bin/python3` executable in the venv. + # None if a full venv isn't created. + interpreter = venv_details.interpreter, + # Files in the venv that need to be created for the interpreter to work + interpreter_runfiles = venv_details.interpreter_runfiles, + # depset[ExplicitSymlink] of symlinks to create. + # This is only used when declare_symlink() can't be used to represent + # creating such a link (i.e Windows) + interpreter_symlinks = venv_details.interpreter_symlinks, + # bool; True if the venv should be recreated at runtime + recreate_venv_at_runtime = venv_details.recreate_venv_at_runtime, + # Runfiles root relative path or absolute path + interpreter_actual_path = interpreter_actual_path, + files_without_interpreter = files_without_interpreter, + # string; venv-relative path to the site-packages directory. + venv_site_packages = venv_details.site_packages, + # string; runfiles-root relative path to venv root. + venv_root = runfiles_root_path( + ctx, + paths.join( + py_internal.get_label_repo_runfiles_path(ctx.label), + venv_ctx_rel_root, + ), + ), + # venv files for user library dependencies (files that are specific + # to the executable bootstrap and python runtime aren't here). + # `root_symlinks` should be used, otherwise, with symlinks files always go + # to `_main` prefix, and binaries from non-root module become broken. + lib_runfiles = ctx.runfiles( + root_symlinks = venv_app_files.runfiles_symlinks, + ), + ) + +def _create_venv_unixy(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_path): + interpreter_runfiles = builders.RunfilesBuilder() + is_bootstrap_script = BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SCRIPT create_full_venv = True # The legacy build_python_zip codepath (enabled by default on windows) isn't # compatible with full venv. # TODO: Use non-build_python_zip codepath for Windows - if is_windows: - create_full_venv = False - elif not rp_config.bazel_8_or_later and not is_bootstrap_script: + if not rp_config.bazel_8_or_later and not is_bootstrap_script: # Full venv for Bazel 7 + system_python is disabled because packaging # it using build_python_zip=true or rules_pkg breaks. # * Using build_python_zip=true breaks because the legacy zipapp support @@ -557,24 +670,18 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root # The pyvenv.cfg file must be present to trigger the venv site hooks. # Because it's paths are expected to be absolute paths, we can't reliably # put much in it. See https://github.com/python/cpython/issues/83650 - pyvenv_cfg = ctx.actions.declare_file("{}/pyvenv.cfg".format(venv)) + pyvenv_cfg = ctx.actions.declare_file("{}/pyvenv.cfg".format(venv_ctx_rel_root)) ctx.actions.write(pyvenv_cfg, "") else: pyvenv_cfg = None - runtime = runtime_details.effective_runtime venvs_use_declare_symlink_enabled = ( VenvsUseDeclareSymlinkFlag.get_value(ctx) == VenvsUseDeclareSymlinkFlag.YES ) - recreate_venv_at_runtime = False - if runtime.interpreter: - interpreter_actual_path = runfiles_root_path(ctx, runtime.interpreter.short_path) - else: - interpreter_actual_path = runtime.interpreter_path - - bin_dir = "{}/bin".format(venv) + recreate_venv_at_runtime = False + bin_dir = "{}/bin".format(venv_ctx_rel_root) if create_full_venv: # Some wrappers around the interpreter (e.g. pyenv) use the program # name to decide what to do, so preserve the name. @@ -587,7 +694,6 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root # needed or used at runtime. However, the zip code uses the interpreter # File object to figure out some paths. interpreter = ctx.actions.declare_file("{}/{}".format(bin_dir, py_exe_basename)) - ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path)) elif runtime.interpreter: @@ -596,6 +702,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root # in runfiles is always a symlink. An RBE implementation, for example, # may choose to write what symlink() points to instead. interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename)) + interpreter_runfiles.add(interpreter) rel_path = relative_path( # dirname is necessary because a relative symlink is relative to @@ -603,10 +710,10 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root from_ = paths.dirname(runfiles_root_path(ctx, interpreter.short_path)), to = interpreter_actual_path, ) - ctx.actions.symlink(output = interpreter, target_path = rel_path) else: interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename)) + interpreter_runfiles.add(interpreter) ctx.actions.symlink(output = interpreter, target_path = runtime.interpreter_path) else: interpreter = None @@ -625,67 +732,108 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root if "t" in runtime.abi_flags: version += "t" - venv_site_packages = "lib/python{}/site-packages".format(version) - site_packages = "{}/{}".format(venv, venv_site_packages) - pth = ctx.actions.declare_file("{}/bazel.pth".format(site_packages)) - ctx.actions.write(pth, "import _bazel_site_init\n") - - site_init = ctx.actions.declare_file("{}/_bazel_site_init.py".format(site_packages)) - computed_subs = ctx.actions.template_dict() - computed_subs.add_joined("%imports%", imports, join_with = ":", map_each = _map_each_identity) - ctx.actions.expand_template( - template = runtime.site_init_template, - output = site_init, - substitutions = { - "%add_runfiles_root_to_sys_path%": add_runfiles_root_to_sys_path, - "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime), - "%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False", - "%site_init_runfiles_path%": runfiles_root_path(ctx, site_init.short_path), - "%workspace_name%": ctx.workspace_name, - }, - computed_substitutions = computed_subs, + site_packages = "lib/python{}/site-packages".format(version) + return _venv_details( + interpreter = interpreter, + pyvenv_cfg = pyvenv_cfg, + site_packages = site_packages, + bin_dir = bin_dir, + recreate_venv_at_runtime = recreate_venv_at_runtime, + interpreter_runfiles = interpreter_runfiles.build(ctx), + interpreter_symlinks = depset(), ) - venv_dir_map = { - VenvSymlinkKind.BIN: bin_dir, - VenvSymlinkKind.LIB: site_packages, - } - venv_app_files = create_venv_app_files( - ctx, - deps = collect_deps(ctx, extra_deps), - venv_dir_map = venv_dir_map, - ) +def _create_venv_windows(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_path): + interpreter_runfiles = builders.RunfilesBuilder() + interpreter_symlinks = builders.DepsetBuilder() - files_without_interpreter = [pth, site_init] + venv_app_files.venv_files - if pyvenv_cfg: - files_without_interpreter.append(pyvenv_cfg) + # Some wrappers around the interpreter (e.g. pyenv) use the program + # name to decide what to do, so preserve the name. + py_exe_basename = paths.basename(interpreter_actual_path) + venv_bin_rel_path = "Scripts" + venv_bin_ctx_rel_path = "{}/{}".format(venv_ctx_rel_root, venv_bin_rel_path) + if runtime.interpreter: + venv_rel_path = paths.join(venv_bin_rel_path, py_exe_basename) + venv_ctx_rel_path = paths.join(venv_ctx_rel_root, venv_rel_path) + interpreter = ctx.actions.declare_file(venv_ctx_rel_path) + interpreter_runfiles.add(interpreter) + ctx.actions.symlink(output = interpreter, target_file = runtime.interpreter) + + rf_path = runfiles_root_path(ctx, interpreter.short_path) + interpreter_symlinks.add(ExplicitSymlink( + runfiles_path = rf_path, + venv_path = venv_rel_path, + link_to_path = interpreter_actual_path, + files = depset([runtime.interpreter]), + )) + else: + # It's OK to use declare_symlink here because an absolute path + # will be written to it, so Bazel won't mangle it. + interpreter = ctx.actions.declare_symlink("{}/{}".format(venv_bin_ctx_rel_path, py_exe_basename)) + interpreter_runfiles.add(interpreter) + ctx.actions.symlink(output = interpreter, target_path = runtime.interpreter_path) + + # NOTE: The .dll files must exist, however, they may not be known at build time + # if the interpreter is resolved at runtime. + for f in runtime.venv_bin_files: + venv_rel_path = paths.join(venv_bin_rel_path, f.basename) + venv_ctx_rel_path = paths.join(venv_ctx_rel_root, venv_rel_path) + + venv_file = ctx.actions.declare_file(venv_ctx_rel_path) + ctx.actions.symlink(output = venv_file, target_file = f) + + interpreter_runfiles.add(venv_file) + + rf_path = runfiles_root_path(ctx, venv_file.short_path) + interpreter_symlinks.add(ExplicitSymlink( + runfiles_path = rf_path, + venv_path = venv_rel_path, + link_to_path = runfiles_root_path(ctx, f.short_path), + files = depset([f]), + )) + # See site.py logic: Windows uses a version/build agnostic site-packages path + site_packages = "Lib/site-packages" + + return _venv_details( + interpreter = interpreter, + pyvenv_cfg = None, + site_packages = site_packages, + bin_dir = venv_bin_ctx_rel_path, + recreate_venv_at_runtime = True, + interpreter_runfiles = interpreter_runfiles.build(ctx), + interpreter_symlinks = interpreter_symlinks.build(), + ) + +def _venv_details( + *, + interpreter, + pyvenv_cfg, + site_packages, + bin_dir, + recreate_venv_at_runtime, + interpreter_runfiles, + interpreter_symlinks): + """Helper to create a struct of platform-specific venv details.""" return struct( - # File or None; the `bin/python3` executable in the venv. - # None if a full venv isn't created. + # File; the `bin/python` executable (or equivalent) within the venv. interpreter = interpreter, - # bool; True if the venv should be recreated at runtime + # File|None; the pyvenv.cfg file, if any. May be none, in which case, + # it's expected that one will be created at runtime. + pyvenv_cfg = pyvenv_cfg, + # str; venv-relative path to the site-packages directory + site_packages = site_packages, + # str; ctx-relative path to the venv's bin directory. + bin_dir = bin_dir, + # bool; True if the venv needs to be recreated at runtime (because the + # build-time construction isn't sufficient). False if the build-time + # constructed venv is sufficient. recreate_venv_at_runtime = recreate_venv_at_runtime, - # Runfiles root relative path or absolute path - interpreter_actual_path = interpreter_actual_path, - files_without_interpreter = files_without_interpreter, - # string; venv-relative path to the site-packages directory. - venv_site_packages = venv_site_packages, - # string; runfiles-root relative path to venv root. - venv_root = runfiles_root_path( - ctx, - paths.join( - py_internal.get_label_repo_runfiles_path(ctx.label), - venv, - ), - ), - # venv files for user library dependencies (files that are specific - # to the executable bootstrap and python runtime aren't here). - # `root_symlinks` should be used, otherwise, with symlinks files always go - # to `_main` prefix, and binaries from non-root module become broken. - lib_runfiles = ctx.runfiles( - root_symlinks = venv_app_files.runfiles_symlinks, - ), + # runfiles; runfiles for interpreter-specific files in the venv. + interpreter_runfiles = interpreter_runfiles, + # depset[ExplicitSymlink] of symlinks specific + # to the interpreter. Only used for Windows. + interpreter_symlinks = interpreter_symlinks, ) def _map_each_identity(v): @@ -787,6 +935,14 @@ def _create_stage1_bootstrap( "%venv_rel_site_packages%": venv.venv_site_packages if venv else "", "%workspace_name%": ctx.workspace_name, } + computed_subs = ctx.actions.template_dict() + if venv: + computed_subs.add_joined( + "%runtime_venv_symlinks%", + venv.interpreter_symlinks, + join_with = "\n", + map_each = _map_runtime_venv_symlink, + ) if stage2_bootstrap: subs["%stage2_bootstrap%"] = runfiles_root_path(ctx, stage2_bootstrap.short_path) @@ -817,9 +973,13 @@ def _create_stage1_bootstrap( template = template, output = output, substitutions = subs, + computed_substitutions = computed_subs, is_executable = True, ) +def _map_runtime_venv_symlink(entry): + return entry.venv_path + "|" + entry.link_to_path + def _create_zip_file(ctx, *, output, zip_main, runfiles): """Create a Python zipapp (zip with __main__.py entry point).""" workspace_name = ctx.workspace_name @@ -1111,6 +1271,8 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = stage2_bootstrap = exec_result.stage2_bootstrap, app_runfiles = app_runfiles, venv_python_exe = exec_result.venv_python_exe, + venv_interpreter_runfiles = exec_result.venv_interpreter_runfiles, + venv_interpreter_symlinks = exec_result.venv_interpreter_symlinks, interpreter_args = ctx.attr.interpreter_args, ) @@ -1657,6 +1819,8 @@ def _create_providers( stage2_bootstrap, app_runfiles, venv_python_exe, + venv_interpreter_runfiles, + venv_interpreter_symlinks, interpreter_args): """Creates the providers an executable should return. @@ -1689,6 +1853,10 @@ def _create_providers( stage2_bootstrap: File; the stage 2 bootstrap script. app_runfiles: runfiles; the runfiles for the application (deps, etc). venv_python_exe: File; the python executable in the venv. + venv_interpreter_runfiles: runfiles; runfiles specific to the interpreter + for the venv. + venv_interpreter_symlinks: depset[ExplicitSymlink]; interpreter-specific symlinks + to create for the venv. interpreter_args: list of strings; arguments to pass to the interpreter. Returns: @@ -1710,14 +1878,16 @@ def _create_providers( create_instrumented_files_info(ctx), _create_run_environment_info(ctx, inherited_environment), PyExecutableInfo( - main = main_py, - runfiles_without_exe = runfiles_details.runfiles_without_exe, + app_runfiles = app_runfiles, build_data_file = runfiles_details.build_data_file, + interpreter_args = interpreter_args, interpreter_path = runtime_details.executable_interpreter_path, + main = main_py, + runfiles_without_exe = runfiles_details.runfiles_without_exe, stage2_bootstrap = stage2_bootstrap, - app_runfiles = app_runfiles, + venv_interpreter_runfiles = venv_interpreter_runfiles, + venv_interpreter_symlinks = venv_interpreter_symlinks, venv_python_exe = venv_python_exe, - interpreter_args = interpreter_args, ), ] diff --git a/python/private/py_executable_info.bzl b/python/private/py_executable_info.bzl index defbd3a05b..0c5931cecd 100644 --- a/python/private/py_executable_info.bzl +++ b/python/private/py_executable_info.bzl @@ -77,6 +77,26 @@ implementation isn't being used. :::{versionadded} 1.9.0 ::: +""", + "venv_interpreter_runfiles": """ +:type: runfiles | None + +Runfiles that are specific to the interpreter within the venv. + +:::{versionadded} VERSION_NEXT_FEATURE +::: +""", + "venv_interpreter_symlinks": """ +:type: depset[ExplicitSymlink] | None + +Symlinks that are specific to the interpreter within the venv. + +Only used with Windows for files that would have used `declare_symlink()` +to create relative symlinks. These may overlap with paths in runfiles; it's +up to the consumer to determine how to handle such overlaps. + +:::{versionadded} VERSION_NEXT_FEATURE +::: """, "venv_python_exe": """ :type: File | None diff --git a/python/private/py_runtime_info.bzl b/python/private/py_runtime_info.bzl index af4e7f0596..8fdbd7bfe2 100644 --- a/python/private/py_runtime_info.bzl +++ b/python/private/py_runtime_info.bzl @@ -66,7 +66,8 @@ def _PyRuntimeInfo_init( zip_main_template = None, abi_flags = "", site_init_template = None, - supports_build_time_venv = True): + supports_build_time_venv = True, + venv_bin_files = None): if (interpreter_path and interpreter) or (not interpreter_path and not interpreter): fail("exactly one of interpreter or interpreter_path must be specified") @@ -119,6 +120,7 @@ def _PyRuntimeInfo_init( "stage2_bootstrap_template": stage2_bootstrap_template, "stub_shebang": stub_shebang, "supports_build_time_venv": supports_build_time_venv, + "venv_bin_files": venv_bin_files, "zip_main_template": zip_main_template, } @@ -334,6 +336,14 @@ to meet two criteria: :::{versionadded} 1.5.0 ::: +""", + "venv_bin_files": """ +:type: list[File] + +Files that should be added to the venv's `bin/` (or platform-specific equivalent) +directory (using the file's basename). + +:::{versionadded} VERSION_NEXT_FEATURE """, "zip_main_template": """ :type: File diff --git a/python/private/py_runtime_rule.bzl b/python/private/py_runtime_rule.bzl index 09e245a58e..dfc915f463 100644 --- a/python/private/py_runtime_rule.bzl +++ b/python/private/py_runtime_rule.bzl @@ -130,6 +130,7 @@ def _py_runtime_impl(ctx): abi_flags = abi_flags, site_init_template = ctx.file.site_init_template, supports_build_time_venv = ctx.attr.supports_build_time_venv, + venv_bin_files = ctx.files.venv_bin_files, )) providers = [ @@ -360,6 +361,7 @@ See {obj}`PyRuntimeInfo.supports_build_time_venv` for docs. """, default = True, ), + "venv_bin_files": attr.label_list(allow_files = True), "zip_main_template": attr.label( default = "//python/private/zipapp:zip_main_template", allow_single_file = True, diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt index 51c9013e9d..accc11e0df 100644 --- a/python/private/python_bootstrap_template.txt +++ b/python/private/python_bootstrap_template.txt @@ -5,13 +5,13 @@ from __future__ import absolute_import from __future__ import division from __future__ import print_function -import sys +# Generated file from @rules_python//python/private:python_bootstrap_template.txt +from os.path import dirname, join, basename, normpath import os -from os.path import dirname, join, basename -import subprocess -import uuid import shutil +import subprocess +import sys # NOTE: The sentinel strings are split (e.g., "%stage2" + "_bootstrap%") so that # the substitution logic won't replace them. This allows runtime detection of @@ -74,22 +74,41 @@ if _INTERPRETER_ARGS_RAW == _INTERPRETER_ARGS_SENTINEL: else: INTERPRETER_ARGS = [arg for arg in _INTERPRETER_ARGS_RAW.split("\n") if arg] +# Symlinks to create at runtime in the venv. +# Line delimited entries +# Each entry is "venv_relative_path|path_to_symlink_to" +RUNTIME_VENV_SYMLINKS = """ +%runtime_venv_symlinks% +""".strip().split("\n") +RUNTIME_VENV_SYMLINKS = dict(line.split("|") for line in RUNTIME_VENV_SYMLINKS if line) + ADDITIONAL_INTERPRETER_ARGS = os.environ.get("RULES_PYTHON_ADDITIONAL_INTERPRETER_ARGS", "") EXTRACT_ROOT = os.environ.get("RULES_PYTHON_EXTRACT_ROOT") -def is_running_from_zip(): - return IS_ZIPFILE - -if is_running_from_zip(): +if IS_ZIPFILE: import shutil import tempfile import zipfile else: import re -# Return True if running on Windows -def is_windows(): - return os.name == 'nt' +IS_WINDOWS = os.name == "nt" + +BIN_DIR_NAME = "bin" if not IS_WINDOWS else "Scripts" + +# Windows APIs can be picky about slashes depending on the context, +# so convert to backslashes to avoid any issues. +if IS_WINDOWS: + def norm_slashes(s): + return s.replace("/", "\\") + + STAGE2_BOOTSTRAP = norm_slashes(STAGE2_BOOTSTRAP) + PYTHON_BINARY = norm_slashes(PYTHON_BINARY) + PYTHON_BINARY_ACTUAL = norm_slashes(PYTHON_BINARY_ACTUAL) + RUNTIME_VENV_SYMLINKS = { + norm_slashes(k): norm_slashes(v) + for k, v in RUNTIME_VENV_SYMLINKS.items() + } def get_windows_path_with_unc_prefix(path): """Adds UNC prefix after getting a normalized absolute Windows path. @@ -100,7 +119,7 @@ def get_windows_path_with_unc_prefix(path): # No need to add prefix for non-Windows platforms. # And \\?\ doesn't work in python 2 or on mingw - if not is_windows() or sys.version_info[0] < 3: + if not IS_WINDOWS or sys.version_info[0] < 3: return path # Starting in Windows 10, version 1607(OS build 14393), MAX_PATH limitations have been @@ -131,12 +150,6 @@ def get_windows_path_with_unc_prefix(path): # os.path.abspath returns a normalized absolute path return unicode_prefix + os.path.abspath(path) -def has_windows_executable_extension(path): - return path.endswith('.exe') or path.endswith('.com') or path.endswith('.bat') - -if PYTHON_BINARY and is_windows() and not has_windows_executable_extension(PYTHON_BINARY): - PYTHON_BINARY = PYTHON_BINARY + '.exe' - def search_path(name): """Finds a file in a given search path.""" search_path = os.getenv('PATH', os.defpath).split(os.pathsep) @@ -156,25 +169,27 @@ def find_python_binary(runfiles_root): def print_verbose(*args, mapping=None, values=None): - if os.environ.get("RULES_PYTHON_BOOTSTRAP_VERBOSE"): - if mapping is not None: - for key, value in sorted((mapping or {}).items()): - print( - "bootstrap: stage 1:", - *(list(args) + ["{}={}".format(key, repr(value))]), - file=sys.stderr, - flush=True - ) - elif values is not None: - for i, v in enumerate(values): - print( - "bootstrap: stage 1:", - *(list(args) + ["[{}] {}".format(i, repr(v))]), - file=sys.stderr, - flush=True - ) - else: - print("bootstrap: stage 1:", *args, file=sys.stderr, flush=True) + if not os.environ.get("RULES_PYTHON_BOOTSTRAP_VERBOSE"): + return + + if mapping is not None: + for key, value in sorted((mapping or {}).items()): + print( + "bootstrap: stage 1:", + *(list(args) + ["{}={}".format(key, repr(value))]), + file=sys.stderr, + flush=True + ) + elif values is not None: + for i, v in enumerate(values): + print( + "bootstrap: stage 1:", + *(list(args) + ["[{}] {}".format(i, repr(v))]), + file=sys.stderr, + flush=True + ) + else: + print("bootstrap: stage 1:", *args, file=sys.stderr, flush=True) def find_binary(runfiles_root, bin_name): """Finds the real binary if it's not a normal absolute path.""" @@ -225,18 +240,18 @@ def find_runfiles_root(main_rel_path): # On Windows, the path may contain both forward and backslashes. # Normalize to the OS separator because the regex used later assumes # the OS-specific separator. - if is_windows(): + if IS_WINDOWS: stub_filename = stub_filename.replace("/", os.sep) if not os.path.isabs(stub_filename): stub_filename = os.path.join(os.getcwd(), stub_filename) while True: - runfiles_root = stub_filename + ('.exe' if is_windows() else '') + '.runfiles' + runfiles_root = stub_filename + ('.exe' if IS_WINDOWS else '') + '.runfiles' if os.path.isdir(runfiles_root): return runfiles_root - runfiles_pattern = r'(.*\.runfiles)' + (r'\\' if is_windows() else '/') + '.*' + runfiles_pattern = r'(.*\.runfiles)' + (r'\\' if IS_WINDOWS else '/') + '.*' matchobj = re.match(runfiles_pattern, stub_filename) if matchobj: return matchobj.group(1) @@ -293,56 +308,102 @@ def create_runfiles_root(): # important that deletion code be in sync with this directory structure return os.path.join(temp_dir, 'runfiles') -def _create_venv(runfiles_root): - runfiles_venv = join(runfiles_root, dirname(dirname(PYTHON_BINARY))) +def _create_venv(runfiles_root, delete_dirs): + rel_runfiles_venv = dirname(dirname(PYTHON_BINARY)) + runfiles_venv = join(runfiles_root, rel_runfiles_venv) + print_verbose("create_venv: runfiles venv:", runfiles_venv) if EXTRACT_ROOT: - venv = join(EXTRACT_ROOT, runfiles_venv) + venv = join(EXTRACT_ROOT, rel_runfiles_venv) os.makedirs(venv, exist_ok=True) - cleanup_dir = None else: import tempfile venv = tempfile.mkdtemp("", f"bazel.{basename(runfiles_venv)}.") - cleanup_dir = venv + delete_dirs.append(venv) + + print_verbose("create_venv: created venv:", venv) python_exe_actual = find_binary(runfiles_root, PYTHON_BINARY_ACTUAL) + if python_exe_actual is None: + raise AssertionError('Could not find python binary: ' + repr(PYTHON_BINARY_ACTUAL)) # See stage1_bootstrap_template.sh for details on this code path. In short, # this handles when the build-time python version doesn't match runtime # and if the initially resolved python_exe_actual is a wrapper script. if RESOLVE_PYTHON_BINARY_AT_RUNTIME: + venv_src = venv.replace("\\", "\\\\") # Escape backslashes src = f""" import sys, site print(sys.executable) -print(site.getsitepackages(["{venv}"])[-1]) +print(sys.base_prefix) +print(site.getsitepackages(["{venv_src}"])[-1]) """ + print_verbose("prog:", src) output = subprocess.check_output([python_exe_actual, "-I"], shell=True, encoding = "utf8", input=src) output = output.strip().split("\n") python_exe_actual = output[0] - venv_site_packages = output[1] + python_home = output[1] if IS_WINDOWS else None + venv_site_packages = output[2] os.makedirs(dirname(venv_site_packages), exist_ok=True) runfiles_venv_site_packages = join(runfiles_venv, VENV_REL_SITE_PACKAGES) else: - python_exe_actual = find_binary(runfiles_root, PYTHON_BINARY_ACTUAL) + # On unixy, Python can find home based on the symlink. + python_home = dirname(python_exe_actual) if IS_WINDOWS else None venv_site_packages = join(venv, "lib") runfiles_venv_site_packages = join(runfiles_venv, "lib") - if python_exe_actual is None: - raise AssertionError('Could not find python binary: ' + repr(PYTHON_BINARY_ACTUAL)) - - venv_bin = join(venv, "bin") + venv_bin = join(venv, BIN_DIR_NAME) try: os.mkdir(venv_bin) except FileExistsError as e: pass - # Match the basename; some tools, e.g. pyvenv key off the executable name venv_python_exe = join(venv_bin, os.path.basename(python_exe_actual)) _symlink_exist_ok(from_=venv_python_exe, to=python_exe_actual) + + # Windows requires supporting .dll files in the venv bin dir, but + # they aren't known in advance when the interpreter is resolved at runtime. + if RESOLVE_PYTHON_BINARY_AT_RUNTIME and IS_WINDOWS: + files = os.listdir(python_home) + for f in files: + if not f.endswith((".dll", ".pdb")): continue + venv_path = join(venv, BIN_DIR_NAME, f) + target = join(python_home, f) + _symlink_exist_ok(from_=venv_path, to=target) + + runfiles_venv_bin = join(runfiles_venv, BIN_DIR_NAME) + # The runfiles bin directory may not exist if it's empty, e.g. + # supports_build_time_venv=False and the interpreter is resolved at + # runtime. + if os.path.exists(runfiles_venv_bin): + # Add any missing build-time entries under bin/. Do this before + # the manual symlink creation to better mimic what the + # regular runfiles directory looks like. + for f_basename in os.listdir(runfiles_venv_bin): + venv_path = join(venv, BIN_DIR_NAME, f_basename) + target = join(runfiles_venv_bin, f_basename) + _symlink_exist_ok(from_=venv_path, to=target) + + for venv_rel_path, link_to_rf_path in RUNTIME_VENV_SYMLINKS.items(): + venv_abs_path = join(venv, venv_rel_path) + link_to = normpath(join(runfiles_venv, link_to_rf_path)) + os.makedirs(dirname(venv_abs_path), exist_ok=True) + _symlink_exist_ok(from_=venv_abs_path, to=link_to) + _symlink_exist_ok(from_=join(venv, "lib"), to=join(runfiles_venv, "lib")) _symlink_exist_ok(from_=venv_site_packages, to=runfiles_venv_site_packages) - _symlink_exist_ok(from_=join(venv, "pyvenv.cfg"), to=join(runfiles_venv, "pyvenv.cfg")) - return cleanup_dir, venv_python_exe + + if IS_WINDOWS: + print_verbose("create_venv: pyvenv.cfg home: ", python_home) + venv_pyvenv_cfg = join(venv, "pyvenv.cfg") + with open(venv_pyvenv_cfg, "w") as fp: + # Until Windows supports a build-time generated venv using symlinks + # to directories, we have to write the full, absolute, path to PYTHONHOME + # so that support directories (e.g. DLLs, libs) can be found. + fp.write("home = {}\n".format(python_home)) + else: + _symlink_exist_ok(from_=join(venv, "pyvenv.cfg"), to=join(runfiles_venv, "pyvenv.cfg")) + return venv_python_exe def runfiles_envvar(runfiles_root): """Finds the runfiles manifest or the runfiles directory. @@ -363,7 +424,7 @@ def runfiles_envvar(runfiles_root): return ('RUNFILES_DIR', runfiles) # If running from a zip, there's no manifest file. - if is_running_from_zip(): + if IS_ZIPFILE: return ('RUNFILES_DIR', runfiles_root) # Look for the runfiles "output" manifest, argv[0] + ".runfiles_manifest" @@ -427,7 +488,7 @@ def execute_file(python_program, main_filename, args, env, runfiles_root, # can't execv because we need control to return here. This only # happens for targets built in the host config. # - if not (is_windows() or workspace or delete_dirs): + if not (IS_WINDOWS or workspace or delete_dirs): _run_execv(python_program, argv, env) print_verbose("run: subproc: environ:", mapping=os.environ) @@ -435,6 +496,7 @@ def execute_file(python_program, main_filename, args, env, runfiles_root, print_verbose("run: subproc: argv:", values=argv) ret_code = subprocess.call( argv, env=env, cwd=workspace) + print_verbose("run: subproc: exit code:", ret_code) if delete_dirs: for delete_dir in delete_dirs: @@ -463,19 +525,21 @@ def _symlink_exist_ok(*, from_, to): pass - def main(): print_verbose("sys.version:", sys.version) print_verbose("initial argv:", values=sys.argv) print_verbose("initial cwd:", os.getcwd()) print_verbose("initial environ:", mapping=os.environ) print_verbose("initial sys.path:", values=sys.path) - print_verbose("STAGE2_BOOTSTRAP:", STAGE2_BOOTSTRAP) + print_verbose("IS_ZIPFILE:", IS_ZIPFILE) print_verbose("PYTHON_BINARY:", PYTHON_BINARY) print_verbose("PYTHON_BINARY_ACTUAL:", PYTHON_BINARY_ACTUAL) - print_verbose("IS_ZIPFILE:", IS_ZIPFILE) print_verbose("RECREATE_VENV_AT_RUNTIME:", RECREATE_VENV_AT_RUNTIME) - print_verbose("WORKSPACE_NAME :", WORKSPACE_NAME ) + print_verbose("RESOLVE_PYTHON_BINARY_AT_RUNTIME:", RESOLVE_PYTHON_BINARY_AT_RUNTIME) + print_verbose("RUNTIME_VENV_SYMLINKS size:", len(RUNTIME_VENV_SYMLINKS)) + print_verbose("STAGE2_BOOTSTRAP:", STAGE2_BOOTSTRAP) + print_verbose("VENV_REL_SITE_PACKAGES:", VENV_REL_SITE_PACKAGES) + print_verbose("WORKSPACE_NAME:", WORKSPACE_NAME ) print_verbose("bootstrap sys.executable:", sys.executable) print_verbose("bootstrap sys._base_executable:", sys._base_executable) print_verbose("bootstrap sys.version:", sys.version) @@ -494,7 +558,7 @@ def main(): delete_dirs = [] - if is_running_from_zip(): + if IS_ZIPFILE: runfiles_root = create_runfiles_root() # NOTE: dirname() is called because create_runfiles_root() creates a # sub-directory within a temporary directory, and we want to remove the @@ -523,20 +587,19 @@ def main(): assert os.access(main_filename, os.R_OK), \ 'Cannot exec() %r: file not readable.' % main_filename - python_program = find_python_binary(runfiles_root) - if python_program is None: - raise AssertionError("Could not find python binary: {} or {}".format( - repr(PYTHON_BINARY), - repr(PYTHON_BINARY_ACTUAL) - )) - if RECREATE_VENV_AT_RUNTIME: # When the venv is created at runtime, python_program is PYTHON_BINARY_ACTUAL # so we have to re-point it to the symlink in the venv - venv, python_program = _create_venv(runfiles_root) - delete_dirs.append(venv) + python_program = _create_venv(runfiles_root, delete_dirs) else: python_program = find_python_binary(runfiles_root) + if python_program is None: + raise AssertionError("Could not find python binary: {} or {}".format( + repr(PYTHON_BINARY), + repr(PYTHON_BINARY_ACTUAL) + )) + + python_program = get_windows_path_with_unc_prefix(python_program) # Some older Python versions on macOS (namely Python 3.7) may unintentionally # leave this environment variable set after starting the interpreter, which @@ -548,7 +611,7 @@ def main(): new_env.update((key, val) for key, val in os.environ.items() if key not in new_env) workspace = None - if is_running_from_zip(): + if IS_ZIPFILE: # If RUN_UNDER_RUNFILES equals 1, it means we need to # change directory to the right runfiles directory. # (So that the data files are accessible) diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index 00f43f3521..a558fa08e1 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -311,12 +311,19 @@ def _which_unchecked(mrctx, binary_name): ) def _which_describe_failure(binary_name, path): + if "\\" in path or ";" in path: + path_parts = path.split(";") + else: + path_parts = path.split(":") + for i, v in enumerate(path_parts): + path_parts[i] = " [{}]: {}".format(i, v) return ( "Unable to find the binary '{binary_name}' on PATH.\n" + - " PATH = {path}" + " PATH entries:\n" + + "{path_str}" ).format( binary_name = binary_name, - path = path, + path_str = "\n".join(path_parts), ) def _mkdir(mrctx, path): diff --git a/python/private/stage2_bootstrap_template.py b/python/private/stage2_bootstrap_template.py index 3c2d6807cc..dec356d3ac 100644 --- a/python/private/stage2_bootstrap_template.py +++ b/python/private/stage2_bootstrap_template.py @@ -67,7 +67,7 @@ def get_build_data(self): from python.runfiles import runfiles rlocation_path = self.BUILD_DATA_FILE path = runfiles.Create().Rlocation(rlocation_path) - if is_windows(): + if IS_WINDOWS: path = os.path.normpath(path) try: # Use utf-8-sig to handle Windows BOM @@ -84,17 +84,14 @@ def get_build_data(self): sys.modules["bazel_binary_info"] = BazelBinaryInfoModule("bazel_binary_info") - -# Return True if running on Windows -def is_windows(): - return os.name == "nt" +IS_WINDOWS = os.name == "nt" def get_windows_path_with_unc_prefix(path): path = path.strip() # No need to add prefix for non-Windows platforms. - if not is_windows() or sys.version_info[0] < 3: + if not IS_WINDOWS or sys.version_info[0] < 3: return path # Starting in Windows 10, version 1607(OS build 14393), MAX_PATH limitations have been @@ -189,19 +186,19 @@ def find_runfiles_root(main_rel_path): # not found. These can be correctly set for a parent Python process, but # inherited by the child, and not correct for it. Later bootstrap code # assumes they're are correct if set. - os.environ.pop('RUNFILES_DIR', None) - os.environ.pop('RUNFILES_MANIFEST_FILE', None) + os.environ.pop("RUNFILES_DIR", None) + os.environ.pop("RUNFILES_MANIFEST_FILE", None) stub_filename = sys.argv[0] if not os.path.isabs(stub_filename): stub_filename = os.path.join(os.getcwd(), stub_filename) while True: - module_space = stub_filename + (".exe" if is_windows() else "") + ".runfiles" + module_space = stub_filename + (".exe" if IS_WINDOWS else "") + ".runfiles" if os.path.isdir(module_space): return module_space - runfiles_pattern = r"(.*\.runfiles)" + (r"\\" if is_windows() else "/") + ".*" + runfiles_pattern = r"(.*\.runfiles)" + (r"\\" if IS_WINDOWS else "/") + ".*" matchobj = re.match(runfiles_pattern, stub_filename) if matchobj: return matchobj.group(1) @@ -454,7 +451,7 @@ def main(): # runfiles root if MAIN_PATH: main_rel_path = MAIN_PATH - if is_windows(): + if IS_WINDOWS: main_rel_path = main_rel_path.replace("/", os.sep) runfiles_root = find_runfiles_root(main_rel_path) diff --git a/python/private/zipapp/py_zipapp_rule.bzl b/python/private/zipapp/py_zipapp_rule.bzl index 1655f63cc9..f26b050165 100644 --- a/python/private/zipapp/py_zipapp_rule.bzl +++ b/python/private/zipapp/py_zipapp_rule.bzl @@ -18,7 +18,7 @@ def _is_symlink(f): else: return "-1" -def _create_zipapp_main_py(ctx, py_runtime, py_executable, stage2_bootstrap, runfiles): +def _create_zipapp_main_py(ctx, py_runtime, py_executable, stage2_bootstrap, runfiles, explicit_symlinks): venv_python_exe = py_executable.venv_python_exe if venv_python_exe: venv_python_exe_path = runfiles_root_path(ctx, venv_python_exe.short_path) @@ -55,7 +55,7 @@ def _create_zipapp_main_py(ctx, py_runtime, py_executable, stage2_bootstrap, run inputs = builders.DepsetBuilder() inputs.add(py_runtime.zip_main_template) - _build_manifest(ctx, hash_files_manifest, runfiles, inputs) + _build_manifest(ctx, hash_files_manifest, runfiles, explicit_symlinks, inputs) actions_run( ctx, @@ -80,7 +80,10 @@ def _map_zip_symlinks(entry): def _map_zip_root_symlinks(entry): return "rf-root-symlink|" + _is_symlink(entry.target_file) + "|" + entry.path + "|" + entry.target_file.path -def _build_manifest(ctx, manifest, runfiles, inputs): +def _map_explicit_symlinks(entry): + return "symlink|" + entry.runfiles_path + "|" + entry.link_to_path + +def _build_manifest(ctx, manifest, runfiles, explicit_symlinks, inputs): manifest.add_all( # NOTE: Accessing runfiles.empty_filenames materializes them. A lambda # is used to defer that. @@ -92,10 +95,13 @@ def _build_manifest(ctx, manifest, runfiles, inputs): manifest.add_all(runfiles.files, map_each = _map_zip_runfiles) manifest.add_all(runfiles.symlinks, map_each = _map_zip_symlinks) manifest.add_all(runfiles.root_symlinks, map_each = _map_zip_root_symlinks) + manifest.add_all(explicit_symlinks, map_each = _map_explicit_symlinks) inputs.add(runfiles.files) inputs.add([entry.target_file for entry in runfiles.symlinks.to_list()]) inputs.add([entry.target_file for entry in runfiles.root_symlinks.to_list()]) + for entry in explicit_symlinks.to_list(): + inputs.add(entry.files) zip_repo_mapping_manifest = maybe_create_repo_mapping( ctx = ctx, @@ -121,6 +127,9 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap): runfiles.add(py_runtime.files) if py_executable.venv_python_exe: runfiles.add(py_executable.venv_python_exe) + + if py_executable.venv_interpreter_runfiles: + runfiles.add(py_executable.venv_interpreter_runfiles) runfiles.add(py_executable.app_runfiles) runfiles.add(stage2_bootstrap) @@ -132,11 +141,12 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap): py_executable, stage2_bootstrap, runfiles, + py_executable.venv_interpreter_symlinks, ) inputs = builders.DepsetBuilder() manifest.add("regular|0|__main__.py|{}".format(zip_main.path)) inputs.add(zip_main) - _build_manifest(ctx, manifest, runfiles, inputs) + _build_manifest(ctx, manifest, runfiles, py_executable.venv_interpreter_symlinks, inputs) zipper_args = ctx.actions.args() zipper_args.add(output) @@ -149,6 +159,9 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap): zipper_args.add(ctx.attr.compression, format = "--compression=%s") zipper_args.add("--runfiles-dir=runfiles") + is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints) + zipper_args.add("\\" if is_windows else "/", format = "--target-platform-pathsep=%s") + actions_run( ctx, executable = ctx.attr._zipper, @@ -220,18 +233,23 @@ def _py_zipapp_executable_impl(ctx): if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints): executable = ctx.actions.declare_file(ctx.label.name + ".exe") - python_exe = py_executable.venv_python_exe - if python_exe: - python_exe_path = runfiles_root_path(ctx, python_exe.short_path) - elif py_runtime.interpreter: - python_exe_path = runfiles_root_path(ctx, py_runtime.interpreter.short_path) + # The zipapp is an opaque zip file, so the Bazel Python launcher doesn't + # know how to look inside it to find the Python interpreter. This means + # we can only use system paths or programs on PATH to bootstrap. + if py_runtime.interpreter_path: + bootstrap_python_path = py_runtime.interpreter_path else: - python_exe_path = py_runtime.interpreter_path + # A special value the Bazel Python launcher recognized to skip + # lookup in the runfiles and uses `python.exe` from PATH. + bootstrap_python_path = "python" create_windows_exe_launcher( ctx, output = executable, - python_binary_path = python_exe_path, + # The path to a python to use to invoke e.g. `python.exe foo.zip` + python_binary_path = bootstrap_python_path, + # Tell the launcher to invoke `python_binary_path` on itself + # after removing its file extension and appending `.zip`. use_zip_file = True, ) default_outputs = [executable, zip_file] diff --git a/python/private/zipapp/zip_main_template.py b/python/private/zipapp/zip_main_template.py index 97c37fee0b..6d12bc9c08 100644 --- a/python/private/zipapp/zip_main_template.py +++ b/python/private/zipapp/zip_main_template.py @@ -1,5 +1,7 @@ # Template for the __main__.py file inserted into zip files # +# Generated file from @rules_python//python/private/zipapp:zip_main_template.py +# # NOTE: This file is a "stage 1" bootstrap, so it's responsible for locating the # desired runtime and having it run the stage 2 bootstrap. This means it can't # assume much about the current runtime and environment. e.g., the current @@ -27,7 +29,7 @@ import subprocess import tempfile import zipfile -from os.path import dirname, join, basename +from os.path import basename, dirname, join # runfiles-root-relative path _STAGE2_BOOTSTRAP = "%stage2_bootstrap%" @@ -45,29 +47,48 @@ IS_WINDOWS = os.name == "nt" -def print_verbose(*args, mapping=None, values=None): - if bool(os.environ.get("RULES_PYTHON_BOOTSTRAP_VERBOSE")): - if mapping is not None: - for key, value in sorted((mapping or {}).items()): - print( - "bootstrap: stage 1:", - *args, - f"{key}={value!r}", - file=sys.stderr, - flush=True, - ) - elif values is not None: - for i, v in enumerate(values): - print( - "bootstrap: stage 1:", - *args, - f"[{i}] {v!r}", - file=sys.stderr, - flush=True, - ) - else: - print("bootstrap: stage 1:", *args, file=sys.stderr, flush=True) +EXTRACT_ROOT = os.environ.get("RULES_PYTHON_EXTRACT_ROOT") + +# Change the paths with Unix-style forward slashes to backslashes for Windows. +# Windows usually transparently rewrites them, but e.g. `\\?\` paths require +# backslashes to be properly understood by Windows APIs. +if IS_WINDOWS: + + def norm_slashes(s): + if not s: + return s + return s.replace("/", "\\") + _STAGE2_BOOTSTRAP = norm_slashes(_STAGE2_BOOTSTRAP) + _PYTHON_BINARY_VENV = norm_slashes(_PYTHON_BINARY_VENV) + _PYTHON_BINARY_ACTUAL = norm_slashes(_PYTHON_BINARY_ACTUAL) + EXTRACT_DIR = norm_slashes(EXTRACT_DIR) + EXTRACT_ROOT = norm_slashes(EXTRACT_ROOT) + + +def print_verbose(*args, mapping=None, values=None): + if not bool(os.environ.get("RULES_PYTHON_BOOTSTRAP_VERBOSE")): + return + if mapping is not None: + for key, value in sorted((mapping or {}).items()): + print( + "bootstrap: stage 1:", + *args, + f"{key}={value!r}", + file=sys.stderr, + flush=True, + ) + elif values is not None: + for i, v in enumerate(values): + print( + "bootstrap: stage 1:", + *args, + f"[{i}] {v!r}", + file=sys.stderr, + flush=True, + ) + else: + print("bootstrap: stage 1:", *args, file=sys.stderr, flush=True) def get_windows_path_with_unc_prefix(path): @@ -105,18 +126,6 @@ def get_windows_path_with_unc_prefix(path): return unicode_prefix + os.path.abspath(path) -def has_windows_executable_extension(path): - return path.endswith(".exe") or path.endswith(".com") or path.endswith(".bat") - - -if ( - _PYTHON_BINARY_VENV - and IS_WINDOWS - and not has_windows_executable_extension(_PYTHON_BINARY_VENV) -): - _PYTHON_BINARY_VENV = _PYTHON_BINARY_VENV + ".exe" - - def search_path(name): """Finds a file in a given search path.""" search_path = os.getenv("PATH", os.defpath).split(os.pathsep) @@ -254,6 +263,7 @@ def execute_file( print_verbose("subprocess cwd:", workspace) print_verbose("subprocess argv:", values=subprocess_argv) ret_code = subprocess.call(subprocess_argv, env=env, cwd=workspace) + print_verbose("subprocess exit code:", ret_code) sys.exit(ret_code) finally: if not EXTRACT_ROOT: @@ -265,6 +275,43 @@ def execute_file( shutil.rmtree(extract_root, True) +def finish_venv_setup(runfiles_root): + python_program = os.path.join(runfiles_root, _PYTHON_BINARY_VENV) + # When a venv is used, the `bin/python3` symlink may need to be created. + # This case occurs when "create venv at runtime" or "resolve python at + # runtime" modes are enabled. + if not os.path.exists(python_program): + # The venv bin/python3 interpreter should always be under runfiles, but + # double check. We don't want to accidentally create symlinks elsewhere + if not python_program.startswith(runfiles_root): + raise AssertionError( + "Program's venv binary not under runfiles: {python_program}" + ) + symlink_to = find_binary(runfiles_root, _PYTHON_BINARY_ACTUAL) + os.makedirs(dirname(python_program), exist_ok=True) + if os.path.lexists(python_program): + os.remove(python_program) + try: + os.symlink(symlink_to, python_program) + except OSError as e: + raise Exception( + f"Unable to create venv python interpreter symlink: {python_program} -> {symlink_to}" + ) from e + venv_root = dirname(dirname(python_program)) + pyvenv_cfg = join(venv_root, "pyvenv.cfg") + if not os.path.exists(pyvenv_cfg): + print_verbose("finish_venv_setup: create pyvenv.cfg:", pyvenv_cfg) + python_home = join(runfiles_root, dirname(_PYTHON_BINARY_ACTUAL)) + print_verbose("finish_venv_setup: pyvenv.cfg home:", python_home) + with open(pyvenv_cfg, "w") as fp: + # Until Windows supports a build-time generated venv using symlinks + # to directories, we have to write the full, absolute, path to PYTHONHOME + # so that support directories (e.g. DLLs, libs) can be found. + fp.write("home = {}\n".format(python_home)) + + return python_program + + def main(): print_verbose("running zip main bootstrap") print_verbose("initial argv:", values=sys.argv) @@ -304,26 +351,7 @@ def main(): ) if _PYTHON_BINARY_VENV: - python_program = join(runfiles_root, _PYTHON_BINARY_VENV) - # When a venv is used, the `bin/python3` symlink may need to be created. - # This case occurs when "create venv at runtime" or "resolve python at - # runtime" modes are enabled. - if not os.path.lexists(python_program): - # The venv bin/python3 interpreter should always be under runfiles, but - # double check. We don't want to accidentally create symlinks elsewhere - if not python_program.startswith(runfiles_root): - raise AssertionError( - "Program's venv binary not under runfiles: {python_program}" - ) - symlink_to = find_binary(runfiles_root, _PYTHON_BINARY_ACTUAL) - os.makedirs(dirname(python_program), exist_ok=True) - try: - os.symlink(symlink_to, python_program) - except OSError as e: - raise Exception( - f"Unable to create venv python interpreter symlink: {python_program} -> {symlink_to}" - ) from e - + python_program = finish_venv_setup(runfiles_root) else: python_program = find_binary(runfiles_root, _PYTHON_BINARY_ACTUAL) if python_program is None: diff --git a/specialized_configs.bazelrc b/specialized_configs.bazelrc new file mode 100644 index 0000000000..46334e0a95 --- /dev/null +++ b/specialized_configs.bazelrc @@ -0,0 +1,12 @@ + +# Helper config to run most tests without waiting an inordinate amount +# of time or freezing the system +common:fast-tests --build_tests_only=true +common:fast-tests --build_tag_filters=-large,-enormous,-integration-test +common:fast-tests --test_tag_filters=-large,-enormous,-integration-test + +# Helper config for running a single test locally and investigating resulting state +common:testone --test_output=streamed +common:testone --test_strategy=standalone +common:testone --spawn_strategy=standalone +common:testone --strategy=standalone diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl index f0e2ae9faf..d6b5aedf0c 100644 --- a/tests/base_rules/py_executable_base_tests.bzl +++ b/tests/base_rules/py_executable_base_tests.bzl @@ -44,19 +44,12 @@ def _test_basic_windows(name, config): impl = _test_basic_windows_impl, target = name + "_subject", config_settings = { - # NOTE: The default for this flag is based on the Bazel host OS, not - # the target platform. For windows, it defaults to true, so force - # it to that to match behavior when this test runs on other - # platforms. - # Pass value to both native and starlark versions of the flag until - # the native one is removed. - labels.BUILD_PYTHON_ZIP: True, "//command_line_option:cpu": "windows_x86_64", "//command_line_option:crosstool_top": CROSSTOOL_TOP, "//command_line_option:extra_execution_platforms": [platform_targets.WINDOWS_X86_64], "//command_line_option:extra_toolchains": [CC_TOOLCHAIN], "//command_line_option:platforms": [platform_targets.WINDOWS_X86_64], - } | maybe_builtin_build_python_zip("true"), + }, attr_values = {}, ) @@ -64,7 +57,7 @@ def _test_basic_windows_impl(env, target): target = env.expect.that_target(target) target.executable().path().contains(".exe") target.runfiles().contains_predicate(matching.str_endswith( - target.meta.format_str("/{name}.zip"), + target.meta.format_str("/{name}"), )) target.runfiles().contains_predicate(matching.str_endswith( target.meta.format_str("/{name}.exe"), @@ -246,9 +239,9 @@ def _test_debugger_impl(env, targets): # #3481: Ensure that venv site-packages is setup correctly, if the dependency is coming # from pip integration. - env.expect.that_target(targets.target_venv).runfiles().contains_at_least([ - "{workspace}/{package}/_{name}.venv/lib/python3.13/site-packages/{test_name}_debugger_venv.py", - ]) + env.expect.that_target(targets.target_venv).runfiles().contains_predicate( + matching.str_endswith("site-packages/test_debugger_debugger_venv.py"), + ) # 3. Subject exec diff --git a/tests/bootstrap_impls/run_binary_zip_yes_test.sh b/tests/bootstrap_impls/run_binary_zip_yes_test.sh index ca278083dd..77fe4d3609 100755 --- a/tests/bootstrap_impls/run_binary_zip_yes_test.sh +++ b/tests/bootstrap_impls/run_binary_zip_yes_test.sh @@ -34,8 +34,8 @@ actual=$($bin) # How we detect if a zip file was executed from depends on which bootstrap # is used. # bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR: -# bootstrap_impl=system_python outputs file:.*Bazel.runfiles -expected_pattern="RULES_PYTHON_ZIP_DIR:/\|file:.*Bazel.runfiles" +# bootstrap_impl=system_python outputs file:.*Bazel.runfiles (or .exe.runfiles on Windows) +expected_pattern="RULES_PYTHON_ZIP_DIR:/\|file:.*Bazel.runfiles\|file:.*\.exe\.runfiles" if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then echo "expected output to match: $expected_pattern" echo "but got: $actual" diff --git a/tests/bootstrap_impls/sys_path_order_test.py b/tests/bootstrap_impls/sys_path_order_test.py index 9ae03bb129..a9018c39ce 100644 --- a/tests/bootstrap_impls/sys_path_order_test.py +++ b/tests/bootstrap_impls/sys_path_order_test.py @@ -33,9 +33,13 @@ def test_sys_path_order(self): # error messages are more informative. categorized_paths = [] for i, value in enumerate(sys.path): - # The runtime's root repo may be added to sys.path, but it - # counts as a user directory, not stdlib directory. - if value in (sys.prefix, sys.base_prefix): + # On Windows, the `pythonXY.zip` entry shows up as `$venv/Scripts/pythonXY.zip` + # While it's technically part of the venv, it's considered the stdlib. + if os.name == "nt" and re.search("python.*[.]zip$", value): + category = "stdlib" + elif value in (sys.prefix, sys.base_prefix): + # The runtime's root repo may be added to sys.path, but it + # counts as a user directory, not stdlib directory. category = "user" elif value.startswith(sys.base_prefix): # The runtime's site-package directory might be called diff --git a/tests/bootstrap_impls/system_python_nodeps_test.py b/tests/bootstrap_impls/system_python_nodeps_test.py index 7dc46d6e73..d9b43e0f27 100644 --- a/tests/bootstrap_impls/system_python_nodeps_test.py +++ b/tests/bootstrap_impls/system_python_nodeps_test.py @@ -1 +1,11 @@ print("Hello, world") + +# Verify py code from the stdlib can be imported. +import pathlib + +print(pathlib) + +# Verify a C-implemented module can be imported. +# Socket isn't implement in C, but requires `_socket`, +# which is implemented in C +import socket diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index 3bb69f2f59..2b4f73a225 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -120,27 +120,6 @@ def _test_py_binary_windows_build_python_zip_false_impl(env, target): _tests.append(_test_py_binary_windows_build_python_zip_false) -def _test_py_binary_windows_build_python_zip_true(name): - _setup_py_binary_windows( - name, - build_python_zip = True, - impl = _test_py_binary_windows_build_python_zip_true_impl, - ) - -def _test_py_binary_windows_build_python_zip_true_impl(env, target): - default_outputs = env.expect.that_target(target).default_outputs() - - # TODO: These outputs aren't correct. The outputs shouldn't - # have the "_" prefix on them (those are coming from the underlying - # wrapped binary). - default_outputs.contains_exactly([ - "{package}/{test_name}_subject.exe", - "{package}/{test_name}_subject.py", - "{package}/{test_name}_subject.zip", - ]) - -_tests.append(_test_py_binary_windows_build_python_zip_true) - def multi_version_test_suite(name): test_suite( name = name, diff --git a/tests/integration/local_toolchains/.bazelrc b/tests/integration/local_toolchains/.bazelrc index aed08b0790..0fbb7678d1 100644 --- a/tests/integration/local_toolchains/.bazelrc +++ b/tests/integration/local_toolchains/.bazelrc @@ -1,4 +1,6 @@ +startup --windows_enable_symlinks common --action_env=RULES_PYTHON_BZLMOD_DEBUG=1 +common --repo_env=RULES_PYTHON_BZLMOD_DEBUG=1 common --lockfile_mode=off test --test_output=errors # Windows requires these for multi-python support: diff --git a/tests/integration/local_toolchains/MODULE.bazel b/tests/integration/local_toolchains/MODULE.bazel index c818942748..fe90fa235a 100644 --- a/tests/integration/local_toolchains/MODULE.bazel +++ b/tests/integration/local_toolchains/MODULE.bazel @@ -37,6 +37,8 @@ local_runtime_repo( pbs_archive = use_repo_rule("//:pbs_archive.bzl", "pbs_archive") +# "pbs" means "python-build-standalone" +# This maps the different platform runtimes to URLS and SHAs pbs_archive( name = "pbs_runtime", sha256 = { @@ -47,7 +49,7 @@ pbs_archive( urls = { "linux": "https://github.com/astral-sh/python-build-standalone/releases/download/20250918/cpython-3.13.7+20250918-x86_64-unknown-linux-gnu-install_only.tar.gz", "mac os x": "https://github.com/astral-sh/python-build-standalone/releases/download/20250918/cpython-3.13.7+20250918-aarch64-apple-darwin-install_only.tar.gz", - "windows server 2022": "https://github.com/astral-sh/python-build-standalone/releases/download/20250918/cpython-3.13.7+20250918-x86_64-pc-windows-msvc-install_only.tar.gz", + "windows": "https://github.com/astral-sh/python-build-standalone/releases/download/20250918/cpython-3.13.7+20250918-x86_64-pc-windows-msvc-install_only.tar.gz", }, ) diff --git a/tests/integration/local_toolchains/pbs_archive.bzl b/tests/integration/local_toolchains/pbs_archive.bzl index 8bd0c1eb10..7d817b1b40 100644 --- a/tests/integration/local_toolchains/pbs_archive.bzl +++ b/tests/integration/local_toolchains/pbs_archive.bzl @@ -16,6 +16,10 @@ def _pbs_archive_impl(repository_ctx): urls = repository_ctx.attr.urls sha256s = repository_ctx.attr.sha256 + # os.name for windows contain build and version; simplify it + if "windows" in os_name: + os_name = "windows" + if os_name not in urls: fail("Unsupported OS: '{}'. Available OSs are: {}".format( os_name, diff --git a/tests/py_zipapp/BUILD.bazel b/tests/py_zipapp/BUILD.bazel index 708d322f41..fc1809b69f 100644 --- a/tests/py_zipapp/BUILD.bazel +++ b/tests/py_zipapp/BUILD.bazel @@ -5,9 +5,6 @@ load("//python:py_test.bzl", "py_test") load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") # buildifier: disable=bzl-visibility load("//python/zipapp:py_zipapp_binary.bzl", "py_zipapp_binary") load("//tests/support:support.bzl", "NOT_WINDOWS") -# todo: add windows support. Windows support will be a bit odd. -# It previously worked by having special logic in the exe launcher -# that knew to look for .zip and running that through python py_binary( name = "venv_bin", diff --git a/tests/repl/repl_test.py b/tests/repl/repl_test.py index 01d0442922..319dab561a 100644 --- a/tests/repl/repl_test.py +++ b/tests/repl/repl_test.py @@ -21,24 +21,58 @@ foo = 1234 """ +IS_WINDOWS = os.name == "nt" + class ReplTest(unittest.TestCase): def setUp(self): - self.repl = rfiles.Rlocation("rules_python/python/bin/repl") + rpath = "rules_python/python/bin/repl" + if IS_WINDOWS: + rpath += ".exe" + self.repl = rfiles.Rlocation(rpath) assert self.repl + if IS_WINDOWS: + self.repl = os.path.normpath(self.repl) def run_code_in_repl(self, lines: Iterable[str], *, env=None) -> str: """Runs the lines of code in the REPL and returns the text output.""" + input = "\n".join(lines) try: return subprocess.check_output( [self.repl], text=True, stderr=subprocess.STDOUT, - input="\n".join(lines), + input=input, env=env, ).strip() except subprocess.CalledProcessError as error: raise RuntimeError(f"Failed to run the REPL:\n{error.stdout}") from error + except Exception as exc: + if env: + env_str = "\n".join( + f"{key}={value!r}" for key, value in sorted(env.items()) + ) + else: + env_str = "" + if isinstance(exc, subprocess.CalledProcessError): + stdout = exc.stdout + else: + stdout = "" + exc.add_note( + f""" +===== env start ===== +{env_str} +===== env end ===== +===== input start ===== +{input} +===== input end ===== +commmand: {self.repl} +===== stdout start ===== +{stdout} +===== stdout end ===== +""" + ) + raise def test_repl_version(self): """Validates that we can successfully execute arbitrary code on the REPL.""" diff --git a/tests/toolchains/defs.bzl b/tests/toolchains/defs.bzl index 25863d18c4..fb4b3beb94 100644 --- a/tests/toolchains/defs.bzl +++ b/tests/toolchains/defs.bzl @@ -57,4 +57,5 @@ def define_toolchain_tests(name): deps = ["//python/runfiles"], data = ["//tests/support:current_build_settings"], target_compatible_with = select(target_compatible_with), + size = "large", ) diff --git a/tests/tools/zipapp/zipper_test.py b/tests/tools/zipapp/zipper_test.py index e0d6653aa7..8b441c4456 100644 --- a/tests/tools/zipapp/zipper_test.py +++ b/tests/tools/zipapp/zipper_test.py @@ -9,6 +9,10 @@ from tools.private.zipapp import zipper +def symlink_target_path(p): + return p.replace("/", os.sep) + + class ZipperTest(unittest.TestCase): def setUp(self): self.test_dir = pathlib.Path(tempfile.mkdtemp()) @@ -26,6 +30,8 @@ def _create_zip(self, **kwargs): "workspace_name": "my_ws", "legacy_external_runfiles": False, "runfiles_dir": "runfiles", + # We need to generate paths for the platform we're running on. + "platform_pathsep": os.sep, } defaults.update(kwargs) zipper.create_zip(**defaults) @@ -90,6 +96,77 @@ def test_create_zip_with_files_and_symlinks(self): self.assertZipFileContent(zf, "runfiles/root_file", content="content1") self.assertZipFileContent(zf, "runfiles/my_ws/empty_file", content="") + def test_create_zip_with_direct_symlink(self): + # Test the 'symlink' manifest entry type + manifest_content = [ + "symlink|path/to/link|target/path", + ] + self.manifest_path.write_text("\n".join(manifest_content)) + + self._create_zip() + + with zipfile.ZipFile(self.output_zip, "r") as zf: + self.assertEqual(zf.namelist(), ["runfiles/path/to/link"]) + self.assertZipFileContent( + zf, + "runfiles/path/to/link", + is_symlink=True, + target=symlink_target_path("../../target/path"), + ) + + def test_pathsep_normalization(self): + # Test that pathsep="\\" normalizes paths + file1_path = self.test_dir / "file1.txt" + file1_path.write_text("content1") + + manifest_content = [ + f"regular|0|dir/file.txt|{file1_path}", + "symlink|link/path|target/path", + ] + self.manifest_path.write_text("\n".join(manifest_content)) + + # Use backslash as platform_pathsep + self._create_zip(platform_pathsep="\\") + + with zipfile.ZipFile(self.output_zip, "r") as zf: + # zipfile.namelist() always returns with forward slashes + # But the content of the symlink should be normalized if it was passed through path_norm + self.assertEqual( + set(zf.namelist()), + {"dir/file.txt", "runfiles/link/path"}, + ) + # The target of the symlink should have backslashes + self.assertZipFileContent( + zf, + "runfiles/link/path", + is_symlink=True, + target="..\\target\\path", + ) + + def test_symlink_precedence(self): + # Test that 'symlink' entries take precedence over others for the same path + file1_path = self.test_dir / "file1.txt" + file1_path.write_text("content1") + + manifest_content = [ + # Same zip path: runfiles/my_ws/path/to/file + f"rf-file|0|path/to/file|{file1_path}", + "symlink|my_ws/path/to/file|symlink/target", + ] + self.manifest_path.write_text("\n".join(manifest_content)) + + self._create_zip() + + with zipfile.ZipFile(self.output_zip, "r") as zf: + self.assertEqual(zf.namelist(), ["runfiles/my_ws/path/to/file"]) + # It should be the symlink, not the file + self.assertZipFileContent( + zf, + "runfiles/my_ws/path/to/file", + is_symlink=True, + target=symlink_target_path("../../../symlink/target"), + ) + def test_timestamps_are_deterministic(self): # Create a content file with a specific recent timestamp file1_path = self.test_dir / "file1.txt" @@ -209,6 +286,54 @@ def test_output_deterministic(self): ], ) + def _extract_zip(self, zip_path, extract_dir): + # Manually extract to preserve symlinks + with zipfile.ZipFile(zip_path, "r") as zf: + for info in zf.infolist(): + extract_path = extract_dir / info.filename + extract_path.parent.mkdir(parents=True, exist_ok=True) + if self.is_symlink(info): + target = zf.read(info).decode() + # On Windows, relative symlinks must use backslashes to be readable + os.symlink(target, extract_path) + else: + with zf.open(info) as src, open(extract_path, "wb") as dst: + shutil.copyfileobj(src, dst) + + def test_symlink_extraction(self): + # Test that 'symlink' entries extract correctly as relative symlinks + # Create a file that the symlink will point to + target_file = self.test_dir / "target_file.txt" + target_file.write_text("target content") + + manifest_content = [ + f"rf-file|0|target/path|{target_file}", + "symlink|my_ws/path/to/link|my_ws/target/path", + f"rf-file|0|same_dir_target|{target_file}", + "symlink|my_ws/same_dir_link|my_ws/same_dir_target", + ] + self.manifest_path.write_text("\n".join(manifest_content)) + + self._create_zip(workspace_name="my_ws") + + extract_dir = self.test_dir / "extract" + extract_dir.mkdir() + + self._extract_zip(self.output_zip, extract_dir) + + link_path = extract_dir / "runfiles/my_ws/path/to/link" + self.assertTrue(link_path.is_symlink(), f"{link_path} should be a symlink") + self.assertEqual( + os.readlink(link_path), "../../target/path".replace("/", os.path.sep) + ) + self.assertEqual(link_path.read_text(), "target content") + + link2_path = extract_dir / "runfiles/my_ws/same_dir_link" + self.assertTrue(link2_path.is_symlink(), f"{link2_path} should be a symlink") + # Relative path from runfiles/my_ws/ to runfiles/my_ws/same_dir_target is just same_dir_target + self.assertEqual(os.readlink(link2_path), "same_dir_target") + self.assertEqual(link2_path.read_text(), "target content") + def is_symlink(self, zip_info): # Check upper 4 bits of external_attr for S_IFLNK # S_IFLNK is 0o120000 = 0xA000 diff --git a/tools/private/zipapp/zip_main_maker.py b/tools/private/zipapp/zip_main_maker.py index 78ac17eb17..ae112ffc66 100644 --- a/tools/private/zipapp/zip_main_maker.py +++ b/tools/private/zipapp/zip_main_maker.py @@ -39,6 +39,10 @@ def compute_inputs_hash(manifest_path: str) -> str: if type_ == "rf-empty": continue + if type_ == "symlink": + # The symlink path and the target it points to + # are captured by hashing the entire line above. + continue is_symlink_str = parts[0] path = parts[-1] diff --git a/tools/private/zipapp/zipper.py b/tools/private/zipapp/zipper.py index 6f41c1e663..870861bc07 100644 --- a/tools/private/zipapp/zipper.py +++ b/tools/private/zipapp/zipper.py @@ -4,12 +4,17 @@ import stat import sys import zipfile +from os.path import dirname # Unix permission bit for symlink (S_IFLNK) # S_IFLNK is usually 0o120000 S_IFLNK = 0o120000 +def unix_join(*parts): + return "/".join(parts) + + def _get_zip_runfiles_path( path, workspace_name, legacy_external_runfiles, runfiles_dir ): @@ -18,8 +23,8 @@ def _get_zip_runfiles_path( elif path.startswith("../"): path = path[3:] else: - path = os.path.join(workspace_name, path) - return os.path.join(runfiles_dir, path) + path = unix_join(workspace_name, path) + return unix_join(runfiles_dir, path) def _parse_entry( @@ -52,10 +57,16 @@ def _parse_entry( ) elif type_ == "rf-symlink": _, is_symlink_str, runfile_path, content_path = parts - zip_path = os.path.join(runfiles_dir, workspace_name, runfile_path) + zip_path = unix_join(runfiles_dir, workspace_name, runfile_path) elif type_ == "rf-root-symlink": _, is_symlink_str, runfile_path, content_path = parts - zip_path = os.path.join(runfiles_dir, runfile_path) + zip_path = unix_join(runfiles_dir, runfile_path) + elif type_ == "symlink": + _, runfile_path, link_to_rf_path = parts + zip_path = unix_join(runfiles_dir, runfile_path) + link_to_rf_path = unix_join(runfiles_dir, link_to_rf_path) + content_path = os.path.relpath(link_to_rf_path, start=dirname(zip_path)) + is_symlink_str = "2" else: raise ValueError( f"Error: Unknown entry type or invalid format at line {line_idx + 1}: {line}" @@ -84,13 +95,40 @@ def read_manifest( e.add_note(f"Error processing line {line_idx + 1}: {line.strip()}") raise - # Sort by zip path (3rd element in tuple) - entries.sort(key=lambda x: x[2]) + # Sort symlink entries first so they have precedence. + # Then sort by zip path + entries.sort(key=lambda x: (x[2], 0 if x[0] == "symlink" else 1)) return entries -def _write_entry(zf, entry, compress_type): +def convert_symlink_target(path, platform_pathsep): + """Converts the path a symlink points to the target-platform format. + + On Windows, relative symlinks must use backslashes. + """ + if platform_pathsep == "/": + # Convert Windows to Unix + return path.replace("\\", platform_pathsep) + else: + # Convert Unix to Windows + return path.replace("/", "\\") + + +# Zip files use forward slash for the entries, even on Windows +def normalize_zip_path(path): + return path.replace("\\", "/") + + +def _write_entry(zf, entry, compress_type, seen, platform_pathsep): type_, is_symlink_str, zip_path, content_path = entry + # Normalize slashes, otherwise the `seen` logic doesn't + # work correctly. + zip_path = normalize_zip_path(zip_path) + if zip_path in seen: + # This can occur because symlink entries have precedence + # over non-symlink entries. + return + seen.add(zip_path) if type_ == "rf-empty": zi = zipfile.ZipInfo(zip_path) @@ -101,6 +139,16 @@ def _write_entry(zf, entry, compress_type): zi.external_attr = (0o644 & 0xFFFF) << 16 zf.writestr(zi, "") return + if type_ == "symlink": + zi = zipfile.ZipInfo(zip_path) + zi.date_time = (1980, 1, 1, 0, 0, 0) + zi.create_system = 3 # Unix + zi.compress_type = compress_type + target = convert_symlink_target(content_path, platform_pathsep) + # Set permissions to 777 for symlink (standard) + zi.external_attr = (S_IFLNK | 0o777) << 16 + zf.writestr(zi, target) + return if is_symlink_str == "-1": if not os.path.exists(content_path): @@ -115,7 +163,7 @@ def _write_entry(zf, entry, compress_type): zi.date_time = (1980, 1, 1, 0, 0, 0) zi.create_system = 3 # Unix zi.compress_type = compress_type - target = os.readlink(content_path) + target = convert_symlink_target(os.readlink(content_path), platform_pathsep) # Set permissions to 777 for symlink (standard) zi.external_attr = (S_IFLNK | 0o777) << 16 zf.writestr(zi, target) @@ -139,6 +187,7 @@ def create_zip( workspace_name, legacy_external_runfiles, runfiles_dir, + platform_pathsep, ): compress_type = zipfile.ZIP_STORED if compress_level == 0 else zipfile.ZIP_DEFLATED zf_level = compress_level if compress_level != 0 else None @@ -147,11 +196,12 @@ def create_zip( manifest_path, workspace_name, legacy_external_runfiles, runfiles_dir ) + seen = set() with zipfile.ZipFile( output_zip, "w", compress_type, allowZip64=True, compresslevel=zf_level ) as zf: for entry in entries: - _write_entry(zf, entry, compress_type) + _write_entry(zf, entry, compress_type, seen, platform_pathsep) def main(): @@ -176,6 +226,9 @@ def main(): 5. `rf-root-symlink|is_symlink|runfile_root_path|content_path`: Store a runfiles-root-relative path in the zip. +6. `symlink|runfile_root_path|link_to_path_rf_path`: Store a symlink that + stores a relative path from `runfile_root_path` to `link_to_rf_path` + In all cases, `is_symlink` has the following values: * `1` means it should be stored as a symlink whose value is read (using `readlink()`) from `content_path`. @@ -210,6 +263,9 @@ def main(): parser.add_argument( "--runfiles-dir", default="runfiles", help="Name of the runfiles directory" ) + parser.add_argument( + "--target-platform-pathsep", help="The path separator for the target platform" + ) args = parser.parse_args() try: @@ -220,6 +276,7 @@ def main(): workspace_name=args.workspace_name, legacy_external_runfiles=args.legacy_external_runfiles == "1", runfiles_dir=args.runfiles_dir, + platform_pathsep=args.target_platform_pathsep, ) except Exception as e: e.add_note(f"Error creating zip {args.output}")