diff --git a/.agents/rules/starlark.md b/.agents/rules/starlark.md index b04cacb11f..bc440b599e 100644 --- a/.agents/rules/starlark.md +++ b/.agents/rules/starlark.md @@ -33,6 +33,11 @@ globs: "*.bzl,BUILD,BUILD.bazel,*.bazel" * **Why**: This avoids inlining the normalization as part of a complex expression later in the macro expansion. +## `executable = True` Attributes +* Use `allow_files = True` (with an explanatory comment), not + `allow_single_file = True`, when the attribute may accept `py_binary` (whose + `DefaultInfo.files` has multiple files). + ## native.test_suite API * Don't forward target `tags` to `native.test_suite`; tags on test suites filter tests instead of setting target execution behavior. @@ -64,6 +69,8 @@ globs: "*.bzl,BUILD,BUILD.bazel,*.bazel" existing files unless explicitly directed by the user. * **Line Length & Wrapping**: Wrap Markdown and Starlark lines to 80 columns in accordance with `.editorconfig`. + * **Long ternaries**: Use `if`/`else` statements when `a if cond else b` + exceeds 80 columns (`buildifier` collapses multi-line ternaries). ## Starlark Testing (`rules_testing`) * **`rules_testing` over `bazel_skylib`**: Always use `@rules_testing` (analysis diff --git a/.agents/skills/buildkite-get-results/scripts/get_buildkite_results.py b/.agents/skills/buildkite-get-results/scripts/get_buildkite_results.py index 1af829a8e6..e45273adac 100755 --- a/.agents/skills/buildkite-get-results/scripts/get_buildkite_results.py +++ b/.agents/skills/buildkite-get-results/scripts/get_buildkite_results.py @@ -68,11 +68,14 @@ def main(): target = args.pr if target.isdigit() and len(target) < 10: - print(f"šŸ” Inspecting PR #{target} via gh to find Buildkite URL...") + print( + f"šŸ” Inspecting PR #{target} via gh to find Buildkite URL...", + file=sys.stderr, + ) target = get_build_url_from_pr(target) build_id = normalize_build_target(target) - print(f"šŸš€ Querying Buildkite for build: {build_id}\n") + print(f"šŸš€ Querying Buildkite for build: {build_id}\n", file=sys.stderr) # Run bk build view res = subprocess.run(["bk", "build", "view", build_id]) @@ -84,11 +87,12 @@ def main(): sys.exit(res.returncode) if args.download: - print(f"\nšŸ“„ Downloading logs for build: {build_id}") + print(f"\nšŸ“„ Downloading logs for build: {build_id}", file=sys.stderr) dl_res = subprocess.run(["bk", "build", "download", build_id]) if dl_res.returncode != 0: print( - "āš ļø 'bk build download' failed or not supported. Try using 'bk job log ' for specific jobs." + "āš ļø 'bk build download' failed or not supported. Try using 'bk job log ' for specific jobs.", + file=sys.stderr, ) diff --git a/.agents/skills/merge-pr/SKILL.md b/.agents/skills/merge-pr/SKILL.md index 8be67821e4..399b329350 100644 --- a/.agents/skills/merge-pr/SKILL.md +++ b/.agents/skills/merge-pr/SKILL.md @@ -27,9 +27,7 @@ When the user asks to merge a pull request (e.g., "merge PR ", "merge th branch. 4. **Queue Shepherding**: Periodically check `gh pr view --json state,autoMergeRequest,mergeStateStatus,mergeable`. While `state` is - `"OPEN"`, ensure auto-merge is enabled / queued by running `gh pr merge - --auto --squash`. If `autoMergeRequest` is null (e.g., ejected - from the merge queue due to a CI flake in the temporary queue branch), - re-enqueue it for merge by running `gh pr merge --auto --squash` - once checks are retried or green. + `"OPEN"`, if `autoMergeRequest` is `null` and no + `gh-readonly-queue/.*/pr--` branch exists, re-enqueue with + `gh pr merge --auto --squash` once checks are green. 5. **Completion Notification**: Once `state` becomes `"MERGED"`, send a high-priority message back to the parent conversation. diff --git a/news/4181.fixed.md b/news/4181.fixed.md new file mode 100644 index 0000000000..1323770842 --- /dev/null +++ b/news/4181.fixed.md @@ -0,0 +1,4 @@ +(`uv`) Support executable wrappers with multiple outputs and preserve their +runfiles in lock actions and runnable targets. Export downloaded `uv` binaries +so wrappers can declare them as dependencies. +([#4181](https://github.com/bazel-contrib/rules_python/issues/4181)) diff --git a/python/uv/private/lock.bzl b/python/uv/private/lock.bzl index 3d39788d9d..b044151299 100644 --- a/python/uv/private/lock.bzl +++ b/python/uv/private/lock.bzl @@ -26,12 +26,30 @@ load(":toolchain_types.bzl", "UV_TOOLCHAIN_TYPE") visibility(["//..."]) _RunLockInfo = provider( - doc = "", + doc = "Information for running the lock command directly via `bazel run`.", fields = { - "args": "The args passed to the `uv` by default when running the runnable target.", - "env": "The env passed to the execution.", - "srcs": "Source files required to run the runnable target.", - "template": "The template file for writing a script.", + "args": """ +:type: list[str | File] + +The args passed to `uv` by default when running the runnable target. +""", + "env": """ +:type: dict[str, str] + +The env passed to the execution. +""", + # Preserve the wrapper's runtime files and symlink mappings together; + # a regular depset cannot represent the full runfiles layout. + "runfiles": """ +:type: runfiles + +Runtime files required by the runnable target. +""", + "template": """ +:type: File + +The template file for writing a script. +""", }, ) @@ -129,7 +147,8 @@ def _common_lock(ctx, locker): output = ctx.actions.declare_file(fname) toolchain_info = ctx.toolchains[UV_TOOLCHAIN_TYPE] - uv = toolchain_info.uv_toolchain_info.uv[DefaultInfo].files_to_run.executable + uv_default_info = toolchain_info.uv_toolchain_info.uv[DefaultInfo] + uv = uv_default_info.files_to_run.executable args = _args(ctx) args.add(uv) @@ -261,7 +280,7 @@ def _common_lock(ctx, locker): # exec "$@" in the .sh script. arguments = [args.run_shell] if not ctx.attr.is_windows else [], tools = [ - uv, + uv_default_info.files_to_run, python_files, script, ], @@ -280,10 +299,10 @@ def _common_lock(ctx, locker): _RunLockInfo( args = args.run_info, env = ctx.attr.env, - srcs = depset( - srcs + [uv], - transitive = [python_files], - ), + runfiles = ctx.runfiles( + files = srcs + [uv], + transitive_files = python_files, + ).merge(uv_default_info.default_runfiles), template = ctx.files._template[0], ), ] @@ -503,7 +522,7 @@ def _run_impl(ctx): return [ DefaultInfo( executable = executable, - runfiles = ctx.runfiles(transitive_files = info.srcs), + runfiles = info.runfiles, ), RunEnvironmentInfo( environment = info.env, diff --git a/python/uv/private/uv_repository.bzl b/python/uv/private/uv_repository.bzl index 79a6495bdc..839e582a0a 100644 --- a/python/uv/private/uv_repository.bzl +++ b/python/uv/private/uv_repository.bzl @@ -24,6 +24,11 @@ UV_BUILD_TMPL = """\ # Generated by repositories.bzl load("@rules_python//python/uv:uv_toolchain.bzl", "uv_toolchain") +# Exposed for users (e.g. custom wrapper scripts), though depending on a +# specific toolchain repository's binary directly is ill-advised because +# toolchain resolution may resolve a different target. +exports_files(["{binary}"], visibility = ["//visibility:public"]) + uv_toolchain( name = "uv_toolchain", uv = "{binary}", diff --git a/python/uv/private/uv_toolchain.bzl b/python/uv/private/uv_toolchain.bzl index bd82e7452f..0e5a5dc5a1 100644 --- a/python/uv/private/uv_toolchain.bzl +++ b/python/uv/private/uv_toolchain.bzl @@ -49,9 +49,21 @@ uv_toolchain = rule( implementation = _uv_toolchain_impl, attrs = { "uv": attr.label( - doc = "A static uv binary.", + doc = """ +The `uv` executable or a wrapper that forwards its arguments to `uv`. +Runtime dependencies belong in the executable target's runfiles. + +:::{versionchanged} VERSION_NEXT_PATCH +Executable targets with multiple output files are supported. Lock actions +and runnable targets include the executable target's runfiles. +::: +""", mandatory = True, - allow_single_file = True, + # allow_files = True is used instead of allow_single_file = True + # because executable rules like py_binary include additional files + # (e.g. srcs) in DefaultInfo.files. executable = True still enforces + # a single executable. + allow_files = True, executable = True, cfg = "exec", ), diff --git a/tests/uv/lock/lock_tests.bzl b/tests/uv/lock/lock_tests.bzl index 3e15ef2053..f14d784ebf 100644 --- a/tests/uv/lock/lock_tests.bzl +++ b/tests/uv/lock/lock_tests.bzl @@ -17,12 +17,38 @@ load("@bazel_skylib//rules:diff_test.bzl", "diff_test") load("@bazel_skylib//rules:native_binary.bzl", "native_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") +load("//python:py_binary.bzl", "py_binary") load("//python/uv:lock.bzl", "lock") +load("//python/uv:uv_toolchain.bzl", "uv_toolchain") load("//python/uv/private:lock.bzl", lock_testing = "testing") # buildifier: disable=bzl-visibility load("//tests/support:py_reconfig.bzl", "py_reconfig_test") _basic_tests = [] +def _extra_runfiles_impl(ctx): + symlink_payload = ctx.actions.declare_file( + ctx.label.name + ".symlink_payload", + ) + ctx.actions.write(symlink_payload, "symlink payload\n") + root_symlink_payload = ctx.actions.declare_file( + ctx.label.name + ".root_symlink_payload", + ) + ctx.actions.write(root_symlink_payload, "root symlink payload\n") + + # Keep the payloads out of ordinary runfiles to require their symlink mappings. + return [DefaultInfo( + runfiles = ctx.runfiles( + symlinks = {"uv_wrapper/symlink_payload.txt": symlink_payload}, + root_symlinks = { + "uv_wrapper/root_symlink_payload.txt": root_symlink_payload, + }, + ), + )] + +_extra_runfiles = rule( + implementation = _extra_runfiles_impl, +) + def _test_reroot(env): reroot = lock_testing.reroot env.expect.that_str( @@ -190,6 +216,53 @@ def lock_test_suite(name): }), ) + _extra_runfiles( + name = "uv_extra_runfiles", + ) + + py_binary( + name = "uv_with_runfiles", + srcs = ["uv_with_runfiles.py"], + data = [ + "testdata/toolchain_payload.txt", + ":uv_extra_runfiles", + ], + deps = ["//python/runfiles"], + ) + + uv_toolchain( + name = "uv_with_runfiles_impl", + uv = ":uv_with_runfiles", + version = "0.0.0", + ) + + native.toolchain( + name = "uv_with_runfiles_toolchain", + toolchain = ":uv_with_runfiles_impl", + toolchain_type = "//python/uv:uv_toolchain_type", + ) + + lock( + name = "toolchain_requirements", + srcs = ["testdata/requirements.in"], + out = "toolchain_requirements.txt", + directory = None, + ) + + for mode in ["run", "update"]: + py_reconfig_test( + name = "toolchain_runfiles_" + mode + "_test", + srcs = ["toolchain_runfiles_test.py"], + main = "toolchain_runfiles_test.py", + args = ["$(rlocationpath :toolchain_requirements." + mode + ")"], + data = [":toolchain_requirements." + mode], + deps = ["//python/runfiles"], + extra_toolchains = [ + str(Label(":uv_with_runfiles_toolchain")), + str(Label("//tests/support/cc_toolchains:all")), + ], + ) + test_suite( name = name + "_basic", basic_tests = _basic_tests, @@ -199,6 +272,8 @@ def lock_test_suite(name): name = name, tests = [ ":" + name + "_basic", + ":toolchain_runfiles_run_test", + ":toolchain_runfiles_update_test", ":requirements_test", ":requirements_directory_test", "//tests/uv/lock/pyproject_toml:requirements_test", diff --git a/tests/uv/lock/testdata/toolchain_payload.txt b/tests/uv/lock/testdata/toolchain_payload.txt new file mode 100644 index 0000000000..9cfc5c3101 --- /dev/null +++ b/tests/uv/lock/testdata/toolchain_payload.txt @@ -0,0 +1 @@ +custom uv toolchain runfiles diff --git a/tests/uv/lock/toolchain_runfiles_test.py b/tests/uv/lock/toolchain_runfiles_test.py new file mode 100644 index 0000000000..ae0ebda4f2 --- /dev/null +++ b/tests/uv/lock/toolchain_runfiles_test.py @@ -0,0 +1,32 @@ +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +from python import runfiles + +LAUNCHER = sys.argv.pop(1) + + +class ToolchainRunfilesTest(unittest.TestCase): + def test_lock_with_toolchain_runfiles(self): + files = runfiles.CreateOrRaise() + launcher = files.root() / LAUNCHER + with tempfile.TemporaryDirectory() as directory: + output = Path(directory, "tests/uv/lock/toolchain_requirements.txt") + output.parent.mkdir(parents=True) + env = dict(os.environ, BUILD_WORKSPACE_DIRECTORY=directory) + env.update(files.EnvVars()) + env.pop("TEST_SRCDIR", None) + command = [str(launcher)] + if os.name == "nt" and launcher.suffix == ".bat": + command = ["cmd.exe", "/c", str(launcher)] + result = subprocess.run(command, env=env, capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(output.read_text(), "custom uv toolchain runfiles\n") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/uv/lock/uv_with_runfiles.py b/tests/uv/lock/uv_with_runfiles.py new file mode 100644 index 0000000000..4e38f3fdbb --- /dev/null +++ b/tests/uv/lock/uv_with_runfiles.py @@ -0,0 +1,22 @@ +import argparse +from pathlib import Path + +from python import runfiles + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--output-file", type=Path, required=True) + args, _ = parser.parse_known_args() + root = runfiles.CreateOrRaise().root() + for location, expected in [ + ("_main/uv_wrapper/symlink_payload.txt", "symlink payload\n"), + ("uv_wrapper/root_symlink_payload.txt", "root symlink payload\n"), + ]: + assert (root / location).read_text() == expected, location + payload = root / "_main/tests/uv/lock/testdata/toolchain_payload.txt" + args.output_file.write_bytes(payload.read_bytes()) + + +if __name__ == "__main__": + main()