diff --git a/e2e/MODULE.bazel b/e2e/MODULE.bazel index da1cdc5d4..92ce50e51 100644 --- a/e2e/MODULE.bazel +++ b/e2e/MODULE.bazel @@ -346,6 +346,17 @@ uv.unstable_annotate_packages( ) # }}} +# For cases/uv-extra-marker +# Regression: extra-only markers must not evaluate to false when the active +# dependency group (venv) has a different name than the extra. +# {{{ +uv.project( + hub_name = "pypi", + lock = "//cases/uv-extra-marker:uv.lock", + pyproject = "//cases/uv-extra-marker:pyproject.toml", +) +# }}} + # For cases/uv-abi3-compat-853 # Regression: abi3 wheel compatibility must propagate to newer Python minors. # {{{ diff --git a/e2e/cases/uv-extra-marker/BUILD.bazel b/e2e/cases/uv-extra-marker/BUILD.bazel new file mode 100644 index 000000000..005bf64cc --- /dev/null +++ b/e2e/cases/uv-extra-marker/BUILD.bazel @@ -0,0 +1,14 @@ +load("@aspect_rules_py//py/unstable:defs.bzl", "py_venv_test") + +py_venv_test( + name = "extra_marker", + srcs = [ + "__test__.py", + ], + main = "__test__.py", + python_version = "3.11", + venv = "ci", + deps = [ + "@pypi//colorama", + ], +) diff --git a/e2e/cases/uv-extra-marker/__test__.py b/e2e/cases/uv-extra-marker/__test__.py new file mode 100644 index 000000000..5244124d6 --- /dev/null +++ b/e2e/cases/uv-extra-marker/__test__.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +"""Reproduce omitted extra-only markers. + +The dev dependency group activates the project's `build` extra, which should +make `colorama` available. If rules_py evaluates `extra == 'build'` to false +because the active venv is named `dev`, the dependency is dropped and this +import fails. +""" + +import colorama + +print(f"colorama: {colorama.__file__}") diff --git a/e2e/cases/uv-extra-marker/pyproject.toml b/e2e/cases/uv-extra-marker/pyproject.toml new file mode 100644 index 000000000..1702531c6 --- /dev/null +++ b/e2e/cases/uv-extra-marker/pyproject.toml @@ -0,0 +1,15 @@ +[project] +name = "extra-marker" +version = "0.0.0" +requires-python = ">=3.11" +dependencies = [] + +[project.optional-dependencies] +build = [ + "colorama", +] + +[dependency-groups] +ci = [ + "extra-marker[build]", +] diff --git a/e2e/cases/uv-extra-marker/uv.lock b/e2e/cases/uv-extra-marker/uv.lock new file mode 100644 index 000000000..8f463cdfb --- /dev/null +++ b/e2e/cases/uv-extra-marker/uv.lock @@ -0,0 +1,34 @@ +version = 1 +revision = 3 +requires-python = ">=3.11" + +[[package]] +name = "colorama" +version = "0.4.6" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d8/53/6f443c9a4a8358a93a6792e2acffb9d9d5cb0a5cfd8802644b7b1c9a02e4/colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44", size = 27697, upload-time = "2022-10-25T02:36:22.414Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, +] + +[[package]] +name = "extra-marker" +version = "0.0.0" +source = { virtual = "." } + +[package.optional-dependencies] +build = [ + { name = "colorama", marker = "extra == 'build'" }, +] + +[package.dev-dependencies] +ci = [ + { name = "extra-marker", extra = ["build"] }, +] + +[package.metadata] +requires-dist = [{ name = "colorama", marker = "extra == 'build'" }] +provides-extras = ["build"] + +[package.metadata.requires-dev] +ci = [{ name = "extra-marker", extras = ["build"] }] diff --git a/uv/private/extension/BUILD.bazel b/uv/private/extension/BUILD.bazel index d0220fbd9..c782217c8 100644 --- a/uv/private/extension/BUILD.bazel +++ b/uv/private/extension/BUILD.bazel @@ -1,9 +1,12 @@ load("@bazel_lib//:bzl_library.bzl", "bzl_library") load(":test_graph_utils.bzl", "graph_utils_test_suite") +load(":test_marker_simplify.bzl", "marker_simplify_test_suite") load(":test_projectfile.bzl", "projectfile_test_suite") graph_utils_test_suite() +marker_simplify_test_suite() + projectfile_test_suite() bzl_library( @@ -78,6 +81,16 @@ bzl_library( ], ) +bzl_library( + name = "test_marker_simplify", + srcs = ["test_marker_simplify.bzl"], + visibility = ["//uv:__subpackages__"], + deps = [ + ":marker_simplify", + "@bazel_skylib//lib:unittest", + ], +) + bzl_library( name = "test_projectfile", srcs = ["test_projectfile.bzl"], @@ -99,6 +112,13 @@ bzl_library( ], ) +bzl_library( + name = "marker_simplify", + srcs = ["marker_simplify.bzl"], + visibility = ["//uv:__subpackages__"], + deps = ["//uv/private/markers:pep508_evaluate"], +) + bzl_library( name = "dep_groups", srcs = ["dep_groups.bzl"], diff --git a/uv/private/extension/graph_utils.bzl b/uv/private/extension/graph_utils.bzl index 0a6d4aeed..d050161c1 100644 --- a/uv/private/extension/graph_utils.bzl +++ b/uv/private/extension/graph_utils.bzl @@ -1,5 +1,6 @@ load("//uv/private:sha1.bzl", "sha1") load("//uv/private/graph:sccs.bzl", "sccs") +load(":marker_simplify.bzl", "simplify_markers_for_extras") def collect_sccs(marker_graph): """Computes Strongly Connected Components (SCCs) for a dependency marker_graph. @@ -139,11 +140,24 @@ def activate_extras(marker_graph, activated_extras, cfg): # For the current (base!) package, look up the closure of activated # extras and merge the _dependencies_ of those extras in. extras = activated_extras.get(pkg, {}).get(cfg, {}) + + # Collect the names of every extra that is active for this package in + # this configuration. We use them to resolve `extra == '...'` markers + # that come from optional dependencies, since at this point we already + # know which extras were requested. + active_extra_names = [dep[3] for dep in extras.keys() if dep[3] != "__base__"] + for extra, extra_markers in extras.items(): # Merge in deps from the requested extra for implied_dep, implied_markers in marker_graph.get(extra, {}).items(): # Normalize since the source graph isn't normalized_implied_dep = (implied_dep[0], implied_dep[1], implied_dep[2], "__base__") - acc[pkg].setdefault(normalized_implied_dep, {}).update(combine_markers(extra_markers, implied_markers)) + + # Resolve any `extra` markers using the extras we know are + # active. This avoids relying on the venv-name heuristic in + # decide_marker later. + simplified_implied_markers = simplify_markers_for_extras(implied_markers, active_extra_names) + if simplified_implied_markers: + acc[pkg].setdefault(normalized_implied_dep, {}).update(combine_markers(extra_markers, simplified_implied_markers)) return acc diff --git a/uv/private/extension/marker_simplify.bzl b/uv/private/extension/marker_simplify.bzl new file mode 100644 index 000000000..e77f0b914 --- /dev/null +++ b/uv/private/extension/marker_simplify.bzl @@ -0,0 +1,125 @@ +"""Helpers for partially evaluating PEP 508 markers. + +These are used during graph construction to resolve `extra == '...'` markers +once we already know which extras are being activated. Resolving them early +means `decide_marker` does not need to know about extras at build time. +""" + +load("//uv/private/markers:pep508_evaluate.bzl", "evaluate", "tokenize") + +def simplify_extra_marker(marker, extras): + """Partially evaluate a marker binding the `extra` PEP 508 variable. + + Once we know which extras of a package are activated for a configuration, + any `extra == '...'` sub-expression in the optional-dependency markers of + those extras becomes a known boolean. This prevents `decide_marker` from + later evaluating `extra` markers based only on the venv name heuristic. + + Args: + marker: The PEP 508 marker string to simplify. + extras: A list of extra names that are active for the package. + + Returns: + - `""` if the marker simplifies to true. + - `None` if the marker simplifies to false. + - A residual marker string if parts of it cannot be evaluated (e.g. + because they also depend on platform markers). + """ + if not marker or "extra" not in marker: + return marker + + residuals = [] + seen_true = False + for extra in extras: + result = evaluate(marker, env = {"extra": extra}, strict = False) + if type(result) == type(True): + if result: + seen_true = True + break + continue + if type(result) == type(""): + if not result: + # The evaluator returns an empty string when the expression + # reduced to true but other terms were unresolved. + seen_true = True + break + if result not in residuals: + residuals.append(result) + else: + fail("Unexpected marker evaluation result for {}: {}".format(marker, result)) + + if seen_true: + return "" + if not residuals: + return None + if len(residuals) == 1: + return residuals[0] + return "({})".format(") or (".join(residuals)) + +def simplify_markers_for_extras(markers, extras): + """Simplify a collection of markers given a set of active extras. + + Args: + markers: A dictionary mapping marker strings to 1. + extras: A list of active extra names. + + Returns: + A dictionary of simplified markers, or an empty dictionary if all + markers simplified to false. + """ + acc = {} + for marker in markers.keys(): + simplified = simplify_extra_marker(marker, extras) + if simplified != None: + acc[simplified] = 1 + return acc + +# Tokens that are allowed in an "extra-only" marker. +_EXTRA_ONLY_TOKENS = { + "extra": True, + "==": True, + "!=": True, + "<": True, + ">": True, + "<=": True, + ">=": True, + "~=": True, + "===": True, + "in": True, + "not in": True, + "and": True, + "or": True, + "not": True, + "(": True, + ")": True, +} + +def is_extra_only_marker(marker): + """Return True if a marker only references the `extra` variable. + + uv uses `extra == '...'` markers both for real package extras and for + internal conflict-routing labels. These markers cannot be evaluated at + Bazel build time because `extra` is not part of the execution environment. + Detecting them lets the BUILD generator treat them as already resolved by + the active dependency group. + + Args: + marker: The PEP 508 marker string to inspect. + + Returns: + True if the marker only depends on `extra`, False otherwise. + """ + if not marker or "extra" not in marker: + return False + + for token in tokenize(marker): + if token in _EXTRA_ONLY_TOKENS: + continue + if token.startswith('"'): + # Quoted string literal. + continue + + # Any other token is an environment variable or unknown operator. + return False + + return True diff --git a/uv/private/extension/projectfile.bzl b/uv/private/extension/projectfile.bzl index 3fd849e28..9d7ac8f58 100644 --- a/uv/private/extension/projectfile.bzl +++ b/uv/private/extension/projectfile.bzl @@ -5,6 +5,7 @@ Machinery specific to interacting with a pyproject.toml load("//uv/private:normalize_name.bzl", "normalize_name") load("//uv/private/versions:versions.bzl", "find_matching_version") load(":dep_groups.bzl", "resolve_dependency_group_specs") +load(":marker_simplify.bzl", "simplify_markers_for_extras") def extract_requirement_marker_pairs(projectfile, lock_id, req_string, version_map, package_versions = {}, preferred_versions = {}): """Parses a requirement string into a list of dependency-marker pairs. @@ -214,6 +215,10 @@ def collect_activated_extras(projectfile, lock_id, project_data, lock_data, defa it = worklist[idx] visited[it] = 1 + # If we reached this node via an extra, any `extra == '...'` marker + # on outgoing edges can be resolved using that extra. + origin_extra = it[3] if it[3] != "__base__" else None + for next_dep, markers in graph.get(it, {}).items(): pkg_name = next_dep[1] pref = group_prefs.get(pkg_name) @@ -223,7 +228,8 @@ def collect_activated_extras(projectfile, lock_id, project_data, lock_data, defa base = (target_dep[0], target_dep[1], target_dep[2], "__base__") - activated_extras.setdefault(base, {}).setdefault(group_name, {}).setdefault(target_dep, {}).update(markers) + simplified_markers = simplify_markers_for_extras(markers, [origin_extra]) if origin_extra else markers + activated_extras.setdefault(base, {}).setdefault(group_name, {}).setdefault(target_dep, {}).update(simplified_markers) if target_dep not in visited: visited[target_dep] = 1 worklist.append(target_dep) diff --git a/uv/private/extension/test_marker_simplify.bzl b/uv/private/extension/test_marker_simplify.bzl new file mode 100644 index 000000000..05d015f76 --- /dev/null +++ b/uv/private/extension/test_marker_simplify.bzl @@ -0,0 +1,98 @@ +"""Tests for marker_simplify.bzl.""" + +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load(":marker_simplify.bzl", "is_extra_only_marker", "simplify_extra_marker", "simplify_markers_for_extras") + +def _simplify_extra_marker_test_impl(ctx): + env = unittest.begin(ctx) + + # Markers that don't reference `extra` pass through untouched. + asserts.equals(env, "", simplify_extra_marker("", ["build"])) + asserts.equals(env, "sys_platform == 'win32'", simplify_extra_marker("sys_platform == 'win32'", ["build"])) + + # Extra-only markers collapse to true ("") or false (None). + asserts.equals(env, "", simplify_extra_marker("extra == 'build'", ["build"])) + asserts.equals(env, "", simplify_extra_marker("extra == 'build'", ["other", "build"])) + asserts.equals(env, "", simplify_extra_marker("extra != 'build'", ["other"])) + asserts.equals(env, None, simplify_extra_marker("extra == 'build'", ["other"])) + asserts.equals(env, None, simplify_extra_marker("extra == 'build'", [])) + + # Mixed markers reduce to their platform residual. Residuals are re-rendered + # by the evaluator with normalized (double) quoting. + asserts.equals(env, "sys_platform == \"win32\"", simplify_extra_marker("extra == 'build' and sys_platform == 'win32'", ["build"])) + asserts.equals(env, None, simplify_extra_marker("extra == 'build' and sys_platform == 'win32'", ["other"])) + asserts.equals(env, "", simplify_extra_marker("extra == 'build' or sys_platform == 'win32'", ["build"])) + asserts.equals(env, "sys_platform == \"win32\"", simplify_extra_marker("extra == 'build' or sys_platform == 'win32'", ["other"])) + + # Identical residuals from several active extras are deduplicated. + asserts.equals(env, "sys_platform == \"win32\"", simplify_extra_marker("extra == 'build' or sys_platform == 'win32'", ["a", "b"])) + + # Distinct residuals are joined under `or`. + asserts.equals( + env, + "((sys_platform == \"win32\")) or ((os_name == \"nt\"))", + simplify_extra_marker("(extra == 'a' and sys_platform == 'win32') or (extra == 'b' and os_name == 'nt')", ["a", "b"]), + ) + + return unittest.end(env) + +simplify_extra_marker_test = unittest.make( + _simplify_extra_marker_test_impl, +) + +def _simplify_markers_for_extras_test_impl(ctx): + env = unittest.begin(ctx) + + # True markers collapse to the unconditional "" marker; untouched markers + # keep their original spelling. + asserts.equals( + env, + {"": 1, "sys_platform == 'win32'": 1}, + simplify_markers_for_extras({"extra == 'build'": 1, "sys_platform == 'win32'": 1}, ["build"]), + ) + + # False markers are dropped; all-false collections become empty. + asserts.equals( + env, + {}, + simplify_markers_for_extras({"extra == 'build'": 1}, ["other"]), + ) + + return unittest.end(env) + +simplify_markers_for_extras_test = unittest.make( + _simplify_markers_for_extras_test_impl, +) + +def _is_extra_only_marker_test_impl(ctx): + env = unittest.begin(ctx) + + asserts.true(env, is_extra_only_marker("extra == 'build'")) + asserts.true(env, is_extra_only_marker("'build' == extra")) + asserts.true(env, is_extra_only_marker("extra != 'build'")) + + # uv conflict-routing markers, including conjunctions. + asserts.true(env, is_extra_only_marker("extra == 'group-9-project-dev'")) + asserts.true(env, is_extra_only_marker("extra == 'a' and extra != 'b'")) + asserts.true(env, is_extra_only_marker("(extra == 'a') or (extra == 'b')")) + + asserts.false(env, is_extra_only_marker("")) + asserts.false(env, is_extra_only_marker("sys_platform == 'win32'")) + asserts.false(env, is_extra_only_marker("extra == 'build' and sys_platform == 'win32'")) + + # "extra" appearing only inside a string literal doesn't qualify. + asserts.false(env, is_extra_only_marker("platform_machine == 'extra'")) + + return unittest.end(env) + +is_extra_only_marker_test = unittest.make( + _is_extra_only_marker_test_impl, +) + +def marker_simplify_test_suite(): + unittest.suite( + "marker_simplify_tests", + simplify_extra_marker_test, + simplify_markers_for_extras_test, + is_extra_only_marker_test, + ) diff --git a/uv/private/uv_project/repository.bzl b/uv/private/uv_project/repository.bzl index f36ea5e29..e7191c709 100644 --- a/uv/private/uv_project/repository.bzl +++ b/uv/private/uv_project/repository.bzl @@ -4,6 +4,7 @@ load("@bazel_features//:features.bzl", features = "bazel_features") load("//uv/private:sha1.bzl", "sha1") +load("//uv/private/extension:marker_simplify.bzl", "is_extra_only_marker") load("//uv/private/pprint:defs.bzl", "pprint") def indent(text, space = " "): @@ -61,13 +62,24 @@ def _project_impl(repository_ctx): def _conditionalize(it, markers, cond_id_thunk, no_match = None): if "" in markers: return it + elif all([is_extra_only_marker(m) for m in markers.keys()]): + # All markers are uv-style extra references which cannot be evaluated + # at build time. Treat as unconditional since group/extra routing is + # already handled by the per-venv select() layer above. + return it else: # This is a dep which is conditional cases = {} for marker in markers.keys(): - cases[_marker(marker)] = it + if is_extra_only_marker(marker): + # Extra-only markers are resolved by the active venv, so they + # match the default case. + if "//conditions:default" not in cases: + cases["//conditions:default"] = it + else: + cases[_marker(marker)] = it - if no_match: + if no_match and "//conditions:default" not in cases: cases["//conditions:default"] = no_match cond_id = cond_id_thunk() @@ -143,13 +155,30 @@ load("@aspect_rules_py//py:defs.bzl", "py_library") cfg_arms["//conditions:default"] = "//private/sccs:" + scc + elif all([is_extra_only_marker(m) for m in markers.keys()]): + # Extra-only markers cannot be evaluated at build time; they + # are already resolved by the active venv. Treat the SCC as + # the default for this configuration. + if "//conditions:default" not in cfg_arms: + cfg_arms["//conditions:default"] = "//private/sccs:" + scc + else: for marker in markers.keys(): - marker = _marker(marker) - if marker in cfg_arms: - fail("Configuration conflict! Package {} specifies two or more configurations for the same marker!\n{}".format(package, pprint(cfgs))) - - cfg_arms[marker] = "//private/sccs:" + scc + if is_extra_only_marker(marker): + # Already resolved by the active venv. + if "//conditions:default" not in cfg_arms: + cfg_arms["//conditions:default"] = "//private/sccs:" + scc + else: + marker = _marker(marker) + if marker in cfg_arms: + fail("Configuration conflict! Package {} specifies two or more configurations for the same marker!\n{}".format(package, pprint(cfgs))) + + cfg_arms[marker] = "//private/sccs:" + scc + + # Ensure the alias resolves in exec configurations where the venv + # flag is not set. Pick any available arm as the default. + if cfg_arms and "//conditions:default" not in cfg_arms: + cfg_arms["//conditions:default"] = list(cfg_arms.values())[0] # Now we can just build one big choice alias from that arm set. content.append(""" @@ -160,7 +189,12 @@ alias( ) """.format(name = cfg_name, arms = indent(pprint(cfg_arms), " " * 4).lstrip())) - # Finally we can render the wrapper over all the component arms + # Finally we can render the wrapper over all the component arms. + # Add a default condition so the alias resolves in exec configuration + # (e.g. when used as a build tool dependency for sdist compilation). + if main_arms and "//conditions:default" not in main_arms: + main_arms["//conditions:default"] = list(main_arms.values())[0] + content.append(""" alias( name = "{name}",