comment: address review comments

- Improve the CHANGELOG wording.
- Rename `pin` to `lock`.
- Retain the same naming convention of `requirements.update`.
- Use `py_binary` with the transitions.
- Add a simple validation to the `mrctx.watch` wrapping.

Author: aignas

CHANGELOG.md

@@ -34,9 +34,12 @@ A brief description of the categories of changes:
 * Nothing yet
 
 ### Fixed
-* (bzlmod) get the path to the host python interpreter without calling
-  `mctx.path` on Labels that have differing contents on different OSes.
-* (bzlmod) correctly watch sources when using `pypi_repo_utils`.
+* (bzlmod) get the path to the host python interpreter in a way that results in
+  platform non-dependent hashes in the lock file when the requirement markers need
+  to be evaluated.
+* (bzlmod) correctly watch sources used for evaluating requirement markers for
+  any changes so that the repository rule or module extensions can be
+  re-evaluated when the said files change.
 
 ## [0.35.0] - 2024-08-15
 

docs/BUILD.bazel

@@ -16,7 +16,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load("@dev_pip//:requirements.bzl", "requirement")
 load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")  # buildifier: disable=bzl-visibility
 load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER")  # buildifier: disable=bzl-visibility
-load("//python/uv/private:pin.bzl", "pin")  # buildifier: disable=bzl-visibility
+load("//python/uv/private:lock.bzl", "lock")  # buildifier: disable=bzl-visibility
 load("//sphinxdocs:readthedocs.bzl", "readthedocs_install")
 load("//sphinxdocs:sphinx.bzl", "sphinx_build_binary", "sphinx_docs")
 load("//sphinxdocs:sphinx_stardoc.bzl", "sphinx_stardoc", "sphinx_stardocs")
@@ -139,9 +139,9 @@ sphinx_build_binary(
     ],
 )
 
-# Run bazel run //docs:requirements_update
-pin(
-    name = "requirements_update",
+# Run bazel run //docs:requirements.update
+lock(
+    name = "requirements",
     srcs = ["pyproject.toml"],
     out = "requirements.txt",
     upgrade = True,

examples/BUILD.bazel

@@ -13,12 +13,12 @@
 # limitations under the License.
 
 # The following is experimental API and currently not intended for use outside this example.
-load("@rules_python//python/uv/private:pin.bzl", "pin")  # buildifier: disable=bzl-visibility
+load("@rules_python//python/uv/private:lock.bzl", "lock")  # buildifier: disable=bzl-visibility
 
 licenses(["notice"])  # Apache 2.0
 
-pin(
-    name = "bzlmod_requirements_3_9_update",
+lock(
+    name = "bzlmod_requirements_3_9",
     srcs = ["bzlmod/requirements.in"],
     out = "bzlmod/requirements_lock_3_9.txt",
     python_version = "3.9.19",

examples/bzlmod/requirements_lock_3_9.txt

@@ -1,5 +1,5 @@
 # This file was autogenerated by uv via the following command:
-#    bazel run //examples:bzlmod_requirements_3_9_update
+#    bazel run //examples:bzlmod_requirements_3_9.update
 --index-url https://pypi.org/simple
 --extra-index-url https://pypi.org/simple/
 

python/private/repo_utils.bzl

@@ -404,12 +404,18 @@ def _get_platforms_cpu_name(mrctx):
 # TODO: Remove after Bazel 6 support dropped
 def _watch(mrctx, *args, **kwargs):
     """Calls mrctx.watch, if available."""
+    if not args and not kwargs:
+        fail("'watch' needs at least a single argument.")
+
     if hasattr(mrctx, "watch"):
         mrctx.watch(*args, **kwargs)
 
 # TODO: Remove after Bazel 6 support dropped
 def _watch_tree(mrctx, *args, **kwargs):
     """Calls mrctx.watch_tree, if available."""
+    if not args and not kwargs:
+        fail("'watch_tree' needs at least a single argument.")
+
     if hasattr(mrctx, "watch_tree"):
         mrctx.watch_tree(*args, **kwargs)
 

python/uv/private/pin.bzl renamed to python/uv/private/lock.bzl

@@ -12,20 +12,19 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""A simple macro to pin the requirements.
+"""A simple macro to lock the requirements.
 """
 
 load("@bazel_skylib//rules:write_file.bzl", "write_file")
 load("//python:py_binary.bzl", "py_binary")
+load("//python/config_settings:transition.bzl", transition_py_binary = "py_binary")
 load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")  # buildifier: disable=bzl-visibility
 
-_REQUIREMENTS_TARGET_COMPATIBLE_WITH = select({
-    "@platforms//os:linux": [],
-    "@platforms//os:macos": [],
-    "//conditions:default": ["@platforms//:incompatible"],
-}) if BZLMOD_ENABLED else ["@platforms//:incompatible"]
+visibility(["//..."])
 
-def pin(*, name, srcs, out, upgrade = False, universal = True, python_version = None):
+_REQUIREMENTS_TARGET_COMPATIBLE_WITH = [] if BZLMOD_ENABLED else ["@platforms//:incompatible"]
+
+def lock(*, name, srcs, out, upgrade = False, universal = True, python_version = None):
     """Pin the requirements based on the src files.
 
     Args:
@@ -44,24 +43,23 @@ def pin(*, name, srcs, out, upgrade = False, universal = True, python_version =
     - Supports transitions out of the box.
     """
     pkg = native.package_name()
-    _out = "_" + out
+    update_target = name + ".update"
 
     args = [
-        "--custom-compile-command='bazel run //{}:{}'".format(pkg, name),
+        "--custom-compile-command='bazel run //{}:{}'".format(pkg, update_target),
         "--generate-hashes",
         "--emit-index-url",
         "--no-strip-extras",
         "--python=$(PYTHON3)",
     ] + [
         "$(location {})".format(src)
         for src in srcs
-    ] + [
-        "--output-file=$(location {})".format(_out),
     ]
     if upgrade:
         args.append("--upgrade")
     if universal:
         args.append("--universal")
+    args.append("--output-file=$@")
     cmd = "$(UV_BIN) pip compile " + " ".join(args)
 
     # Check if the output file already exists, if yes, first copy it to the
@@ -72,9 +70,9 @@ def pin(*, name, srcs, out, upgrade = False, universal = True, python_version =
         srcs.append(out)
 
     native.genrule(
-        name = name + ".uv.out",
+        name = name,
         srcs = srcs,
-        outs = [_out],
+        outs = [out + ".new"],
         cmd_bash = cmd,
         tags = [
             "local",
@@ -88,97 +86,36 @@ def pin(*, name, srcs, out, upgrade = False, universal = True, python_version =
         ],
     )
     if python_version:
-        transitioned_name = "{}.uv.out.{}".format(name, python_version)
-        _versioned(
-            name = transitioned_name,
-            src = _out,
-            python_version = python_version,
-            tags = ["manual"],
-        )
-        _out = transitioned_name
+        py_binary_rule = lambda *args, **kwargs: transition_py_binary(python_version = python_version, *args, **kwargs)
+    else:
+        py_binary_rule = py_binary
 
     # Write a script that can be used for updating the in-tree version of the
     # requirements file
     write_file(
-        name = name + ".gen",
-        out = name + ".gen.py",
+        name = name + ".update_gen",
+        out = update_target + ".py",
         content = [
             "from os import environ",
             "from pathlib import Path",
             "from sys import stderr",
             "",
             'src = Path(environ["REQUIREMENTS_FILE"])',
+            'assert src.exists(), f"the {src} file does not exist"',
             'dst = Path(environ["BUILD_WORKSPACE_DIRECTORY"]) / "{}" / "{}"'.format(pkg, out),
             'print(f"Writing requirements contents\\n  from {src.absolute()}\\n  to {dst.absolute()}", file=stderr)',
             "dst.write_text(src.read_text())",
             'print("Success!", file=stderr)',
         ],
-        target_compatible_with = _REQUIREMENTS_TARGET_COMPATIBLE_WITH,
     )
 
-    py_binary(
-        name = name,
-        srcs = [name + ".gen.py"],
-        main = name + ".gen.py",
-        data = [_out],
+    py_binary_rule(
+        name = update_target,
+        srcs = [update_target + ".py"],
+        main = update_target + ".py",
+        data = [name],
         env = {
-            "REQUIREMENTS_FILE": "$(location {})".format(_out),
+            "REQUIREMENTS_FILE": "$(rootpath {})".format(name),
         },
         tags = ["manual"],
-        target_compatible_with = _REQUIREMENTS_TARGET_COMPATIBLE_WITH,
     )
-
-def _transition_python_version_impl(_, attr):
-    return {"//python/config_settings:python_version": str(attr.python_version)}
-
-_transition_python_version = transition(
-    implementation = _transition_python_version_impl,
-    inputs = [],
-    outputs = ["//python/config_settings:python_version"],
-)
-
-def _impl(ctx):
-    target = ctx.attr.src
-
-    default_info = target[0][DefaultInfo]
-    files = default_info.files
-    original_executable = default_info.files_to_run.executable
-    runfiles = default_info.default_runfiles
-
-    new_executable = ctx.actions.declare_file(ctx.attr.name)
-
-    ctx.actions.symlink(
-        output = new_executable,
-        target_file = original_executable,
-        is_executable = True,
-    )
-
-    files = depset(direct = [new_executable], transitive = [files])
-    runfiles = runfiles.merge(ctx.runfiles([new_executable]))
-
-    return [
-        DefaultInfo(
-            files = files,
-            runfiles = runfiles,
-            executable = new_executable,
-        ),
-    ]
-
-_versioned = rule(
-    implementation = _impl,
-    attrs = {
-        "python_version": attr.string(
-            mandatory = True,
-        ),
-        "src": attr.label(
-            allow_single_file = True,
-            executable = False,
-            mandatory = True,
-            cfg = _transition_python_version,
-        ),
-        "_allowlist_function_transition": attr.label(
-            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
-        ),
-    },
-    executable = True,
-)