Skip to content

feat: some windows fixes (wip) #622

New issue

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

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

Already on GitHub? Sign in to your account

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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ test --test_output=errors

# TODO(alex): enable
common --noenable_bzlmod
common --enable_workspace

# Windows settings
startup --windows_enable_symlinks

# Point tools such as coursier (used in rules_jvm_external) to Bazel's downloaded JDK
# suggested in https://github.com/bazelbuild/rules_jvm_external/issues/445
common --repo_env=JAVA_HOME=../bazel_tools/jdk
common --action_env=JAVA_HOME=../bazel_tools/jdk

# Define value used by tests
build --define=SOME_VAR=SOME_VALUE
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
7.0.2
7.6.1
# The first line of this file is used by Bazelisk and Bazel to be sure
# the right version of Bazel is used to build and test this repo.
# This also defines which version is used on CI.
Expand Down
6 changes: 3 additions & 3 deletions Cargo.Bazel.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"checksum": "32b494853fa6e3f2c2ea61a76720e873e7384da14889aadcba90bfcd4f1916e3",
"checksum": "aa56303cf8eaea6e963e8ba41da8c3381eff6977a795dadf3fe0b3f846012e31",
"crates": {
"addr2line 0.21.0": {
"name": "addr2line",
Expand Down Expand Up @@ -8536,9 +8536,9 @@
"version": "0.6.0",
"repository": {
"Git": {
"remote": "https://github.com/prefix-dev/rip",
"remote": "https://github.com/peakschris/rip",
"commitish": {
"Rev": "b047c9ec0b42125a67d35346f08b7e7848ac24f4"
"Rev": "3de9ee95eab577c9e48526464013e217287c6937"
},
"strip_prefix": "crates/rattler_installs_packages"
}
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module(
# When bumping, add a comment explaining what's required from the newer release.
bazel_dep(name = "aspect_bazel_lib", version = "1.40.0")
bazel_dep(name = "bazel_skylib", version = "1.4.2")
bazel_dep(name = "rules_python", version = "0.29.0")
bazel_dep(name = "rules_python", version = "0.40.0")
bazel_dep(name = "platforms", version = "0.0.7")


Expand All @@ -21,6 +21,12 @@ python.toolchain(
python_version = "3.8.12",
)

python_stdlib_list = use_extension("@rules_python//python:extensions.bzl", "python_stdlib_list")
use_repo(
python_stdlib_list,
"python_stdlib_list",
)

tools = use_extension("//py:extensions.bzl", "py_tools")
tools.rules_py_tools()
use_repo(tools, "rules_py_tools")
Expand All @@ -38,9 +44,10 @@ register_toolchains(
# NOTE: when publishing to BCR, we patch these to be dev_dependency, as we publish pre-built binaries
# along with our releases.

# Versions newer than 0.58 fail with https://github.com/bazelbuild/rules_rust/issues/3441
bazel_dep(
name = "rules_rust",
version = "0.38.0",
version = "0.58.0",
# In released versions: dev_dependency = True
)

Expand Down
3 changes: 3 additions & 0 deletions e2e/repository-rule-deps/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ common --enable_bzlmod

# The imports=[".."] only works properly when this matches the python toolchain major version.
common --@aspect_rules_py//py:interpreter_version=3.11.6

# Windows settings
startup --windows_enable_symlinks
11 changes: 11 additions & 0 deletions e2e/smoke/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,13 @@
# Under Bazel 7 or later, this flag is ignored as the PyRuntimeInfo gives this information.
common --@aspect_rules_py//py:interpreter_version=3.9.18

# Windows settings
startup --windows_enable_symlinks

# Attempt to shorten paths as much as possible to avoid windows limits
common --nolegacy_external_runfiles
common --experimental_sibling_repository_layout

common --override_module=rules_rust=d:/workdir/github/rules_rust2

common --incompatible_use_plus_in_repo_names=false
3 changes: 2 additions & 1 deletion e2e/smoke/.bazelversion
3 changes: 2 additions & 1 deletion e2e/smoke/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ local_path_override(
)

# Because we use a prerelease of rules_py, we must compile the rust tools from source.
bazel_dep(name = "rules_rust", version = "0.40.0")
# Versions newer than 0.58 fail with https://github.com/bazelbuild/rules_rust/issues/3441
bazel_dep(name = "rules_rust", version = "0.58.0")

rust = use_extension("@rules_rust//rust:extensions.bzl", "rust")
rust.toolchain(
Expand Down
3 changes: 3 additions & 0 deletions e2e/system-interpreter/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
# Under Bazel 7 or later, this flag is ignored as the PyRuntimeInfo gives this information.
common --@aspect_rules_py//py:interpreter_version=3.9.6

# Windows settings
startup --windows_enable_symlinks
6 changes: 3 additions & 3 deletions internal_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ def rules_py_internal_deps():

http_archive(
name = "rules_python_gazelle_plugin",
sha256 = "c68bdc4fbec25de5b5493b8819cfc877c4ea299c0dcb15c244c5a00208cde311",
strip_prefix = "rules_python-0.31.0/gazelle",
url = "https://github.com/bazelbuild/rules_python/releases/download/0.31.0/rules_python-0.31.0.tar.gz",
sha256 = "690e0141724abb568267e003c7b6d9a54925df40c275a870a4d934161dc9dd53",
strip_prefix = "rules_python-0.40.0/gazelle",
url = "https://github.com/bazelbuild/rules_python/releases/download/0.40.0/rules_python-0.40.0.tar.gz",
)

http_archive(
Expand Down
21 changes: 17 additions & 4 deletions py/private/py_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
load("@rules_python//python:defs.bzl", "PyInfo")
load("@aspect_bazel_lib//lib:paths.bzl", "BASH_RLOCATION_FUNCTION", "to_rlocation_path")
load("@aspect_bazel_lib//lib:expand_make_vars.bzl", "expand_locations", "expand_variables")
load("@aspect_bazel_lib//lib:windows_utils.bzl", "create_windows_native_launcher_script")
load("//py/private:py_library.bzl", _py_library = "py_library_utils")
load("//py/private:py_semantics.bzl", _py_semantics = "semantics")
load("//py/private/toolchain:types.bzl", "PY_TOOLCHAIN", "VENV_TOOLCHAIN")
Expand All @@ -14,6 +15,7 @@ def _dict_to_exports(env):
]

def _py_binary_rule_impl(ctx):
is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])
venv_toolchain = ctx.toolchains[VENV_TOOLCHAIN]
py_toolchain = _py_semantics.resolve_toolchain(ctx)

Expand Down Expand Up @@ -62,10 +64,15 @@ def _py_binary_rule_impl(ctx):
attribute_name = "env",
)

executable_launcher = ctx.actions.declare_file(ctx.attr.name)
print(py_toolchain.python)
print(py_toolchain.runfiles_interpreter)
print(py_toolchain.python.path)
print(to_rlocation_path(ctx, py_toolchain.python))

bash_launcher = ctx.actions.declare_file(ctx.attr.name)
ctx.actions.expand_template(
template = ctx.file._run_tmpl,
output = executable_launcher,
output = bash_launcher,
substitutions = {
"{{BASH_RLOCATION_FN}}": BASH_RLOCATION_FUNCTION,
"{{INTERPRETER_FLAGS}}": " ".join(py_toolchain.flags),
Expand All @@ -88,6 +95,8 @@ def _py_binary_rule_impl(ctx):
is_executable = True,
)

launcher = create_windows_native_launcher_script(ctx, bash_launcher) if is_windows else bash_launcher

srcs_depset = _py_library.make_srcs_depset(ctx)

runfiles = _py_library.make_merged_runfiles(
Expand All @@ -97,6 +106,8 @@ def _py_binary_rule_impl(ctx):
srcs_depset,
] + virtual_resolution.srcs + virtual_resolution.runfiles,
extra_runfiles = [
launcher,
bash_launcher,
site_packages_pth_file,
] + ctx.files._runfiles_lib,
extra_runfiles_depsets = [
Expand All @@ -112,11 +123,11 @@ def _py_binary_rule_impl(ctx):
return [
DefaultInfo(
files = depset([
executable_launcher,
launcher,
ctx.file.main,
site_packages_pth_file,
]),
executable = executable_launcher,
executable = launcher,
runfiles = runfiles,
),
PyInfo(
Expand Down Expand Up @@ -189,6 +200,7 @@ python.toolchain(python_version = "3.9", is_default = True)
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
"_windows_constraint": attr.label(default = "@platforms//os:windows"),
})

_attrs.update(**_py_library.attrs)
Expand All @@ -210,6 +222,7 @@ py_base = struct(
toolchains = [
PY_TOOLCHAIN,
VENV_TOOLCHAIN,
"@bazel_tools//tools/sh:toolchain_type",
],
cfg = _python_version_transition
)
Expand Down
5 changes: 3 additions & 2 deletions py/tools/py/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ homepage.workspace = true
repository.workspace = true
license.workspace = true
edition.workspace = true
readme.workspace = true
rust-version.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
miette = { version = "5.10", features = ["fancy"] }
rattler_installs_packages = { git = "https://github.com/prefix-dev/rip", rev = "b047c9ec0b42125a67d35346f08b7e7848ac24f4", default-features = false, features = ["rustls-tls"] }
# workaround issue with this package building with rules_rust on windows; the snap files used in their unit tests have
# too long a path and import of this module is failing (even when tests are not run)
rattler_installs_packages = { git = "https://github.com/peakschris/rip", rev = "3de9ee95eab577c9e48526464013e217287c6937", default-features = false, features = ["rustls-tls"] }
42 changes: 35 additions & 7 deletions py/tools/py/src/pth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,7 @@ fn create_symlink(e: &DirEntry, root_dir: &Path, dst_dir: &Path) -> miette::Resu
);
}

std::os::unix::fs::symlink(&tgt, &link)
.into_diagnostic()
.wrap_err(format!(
"Unable to create symlink: {} -> {}",
tgt.to_string_lossy(),
link.to_string_lossy()
))?;
create_cross_platform_symlink(&tgt, &link)?;

Ok(())
}
Expand Down Expand Up @@ -186,3 +180,37 @@ fn is_same_file(p1: &Path, p2: &Path) -> miette::Result<bool> {

return Ok(true);
}

fn create_cross_platform_symlink(tgt: &Path, link: &Path) -> miette::Result<()> {
#[cfg(unix)]
{
std::os::unix::fs::symlink(tgt, link)
.into_diagnostic()
.wrap_err(format!(
"Unable to create symlink: {} -> {}",
tgt.to_string_lossy(),
link.to_string_lossy()
))
}

#[cfg(windows)]
{
if tgt.is_dir() {
std::os::windows::fs::symlink_dir(tgt, link)
.into_diagnostic()
.wrap_err(format!(
"Unable to create directory symlink: {} -> {}",
tgt.to_string_lossy(),
link.to_string_lossy()
))
} else {
std::os::windows::fs::symlink_file(tgt, link)
.into_diagnostic()
.wrap_err(format!(
"Unable to create file symlink: {} -> {}",
tgt.to_string_lossy(),
link.to_string_lossy()
))
}
}
}
2 changes: 1 addition & 1 deletion py/tools/py/src/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn create_venv(
.into_diagnostic()
.wrap_err("Unable to determine absolute directory to venv directory")?;

let install_paths = interpreter.install_paths(false);
let install_paths = interpreter.install_paths(cfg!(windows));

VEnv::create_install_paths(&venv_location, &install_paths)
.into_diagnostic()
Expand Down
7 changes: 7 additions & 0 deletions py/tools/unpack_bin/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@ multi_arch_rust_binary_release(
os = "linux",
visibility = ["//tools/release:__pkg__"],
)

multi_arch_rust_binary_release(
name = "windows",
src = ":unpack",
os = "windows",
visibility = ["//tools/release:__pkg__"],
)
2 changes: 1 addition & 1 deletion py/tools/unpack_bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ fn unpack_cmd_handler(args: UnpackArgs) -> miette::Result<()> {

fn main() -> miette::Result<()> {
let args = UnpackArgs::parse();
unpack_cmd_handler(args).wrap_err("Unable to run command:")
unpack_cmd_handler(args).wrap_err("Unable to run command (unpack):")
}
7 changes: 7 additions & 0 deletions py/tools/venv_bin/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@ multi_arch_rust_binary_release(
os = "linux",
visibility = ["//tools/release:__pkg__"],
)

multi_arch_rust_binary_release(
name = "windows",
src = ":venv",
os = "windows",
visibility = ["//tools/release:__pkg__"],
)
4 changes: 2 additions & 2 deletions py/tools/venv_bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use miette::Context;

use py;

#[derive(Parser, Debug)]
#[derive(Parser, Debug, Clone)]
struct VenvArgs {
/// Source Python interpreter path to symlink into the venv.
#[arg(long)]
Expand Down Expand Up @@ -41,5 +41,5 @@ fn venv_cmd_handler(args: VenvArgs) -> miette::Result<()> {

fn main() -> miette::Result<()> {
let args = VenvArgs::parse();
venv_cmd_handler(args).wrap_err("Unable to run command:")
venv_cmd_handler(args.clone()).wrap_err_with(|| format!("Unable to run command (venv): {:?}", args))
}
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
django~=4.2.7
django==4.2.7
colorama~=0.4.0
click
pytest
Expand Down
Loading