address review comments

docs/project/usage.md

@@ -71,17 +71,17 @@ You can pass multiple packages to `micropip.install`:
 await micropip.install(["pkg1", "pkg2"])
 ```
 
-A dependency can specify be refined per the [PEP-508] spec:
+A dependency can be refined as per the [PEP-508] spec:
 
+[pep-508]: https://peps.python.org/pep-0508
 
 ```python
 await micropip.install("snowballstemmer==2.2.0")
 await micropip.install("snowballstemmer>=2.2.0")
+await micropip.install("snowballstemmer @ https://.../snowballstemmer.*.whl")
 await micropip.install("snowballstemmer[all]")
 ```
 
-[PEP-508]: https://peps.python.org/pep-0508
-
 ### Disabling dependency resolution
 
 micropip does dependency resolution by default, but you can disable it,
@@ -94,31 +94,52 @@ await micropip.install("pkg", deps=False)
 
 ### Constraining indirect dependencies
 
-Dependency resolution can be further customized with optional `constraints`: as
-described in the [`pip`](https://pip.pypa.io/en/stable/user_guide/#constraints-files)
-documentation, these must provide a name and version (or URL), and may not request
-`[extras]`.
+Dependency resolution can be further customized with optional `constraints`:
+these modify both _direct_ and _indirect_ dependency resolutions, while direct URLs
+in either a requirement or constraint will generally bypass any other specifiers.
+
+As described in the [`pip` documentation][pip-constraints], each constraint:
+
+[pip-constraints]: https://pip.pypa.io/en/stable/user_guide/#constraints-files
+
+  - _must_ provide a name
+  - _must_ provide exactly one of
+    - a set of version specifiers
+    - a URL, and _may not_ request
+  - _must not_  request any `[extras]`
+
+
+Invalid constraints will be silently discarded, or logged if `verbose` is provided.
 
 ```python
 await micropip.install(
     "pkg",
     constraints=[
-        "other-pkg ==0.1.1",
-        "some-other-pkg <2",
+        "other-pkg==0.1.1",
+        "some-other-pkg<2",
         "yet-another-pkg@https://example.com/yet_another_pkg-0.1.2-py3-none-any.whl",
-        # invalid examples                         # why?
-        # yet_another_pkg-0.1.2-py3-none-any.whl   # missing name
-        # something-completely[different] ==0.1.1  # extras
-        # package-with-no-version                  # missing version or URL
+        # silently discarded                          # why?
+        "yet_another_pkg-0.1.2-py3-none-any.whl",     # missing name
+        "something-completely[different] ==0.1.1",    # extras
+        "package-with-no-version",                    # missing version or URL
     ]
 )
 ```
 
+Over-constrained requirements will fail to resolve, leaving the environment unmodified.
+
+```python
+await micropip.install("pkg ==1", constraints=["pkg ==2"])
+# ValueError: Can't find a pure Python 3 wheel for 'pkg==1,==2'.
+```
+
+### Setting default constraints
+
 `micropip.set_constraints` replaces any default constraints for all subsequent
-calls to `micropip.install` that don't specify constraints:
+calls to `micropip.install` that don't specify `constraints`:
 
 ```python
-micropip.set_constraints = ["other-pkg ==0.1.1"]
-await micropip.install("pkg")                         # uses defaults
+micropip.set_constraints(["other-pkg ==0.1.1"])
+await micropip.install("pkg")                         # uses defaults, if needed
 await micropip.install("another-pkg", constraints=[]) # ignores defaults
 ```

micropip/_utils.py

@@ -4,7 +4,7 @@
 from pathlib import Path
 from sysconfig import get_config_var, get_platform
 
-from packaging.requirements import Requirement
+from packaging.requirements import InvalidRequirement, Requirement
 from packaging.tags import Tag
 from packaging.tags import sys_tags as sys_tags_orig
 from packaging.utils import BuildTag, InvalidWheelFilename, canonicalize_name
@@ -268,3 +268,76 @@ def fix_package_dependencies(
     (get_dist_info(dist) / "PYODIDE_REQUIRES").write_text(
         json.dumps(sorted(x for x in depends))
     )
+
+
+def validate_constraints(
+    constraints: list[str] | None,
+) -> tuple[dict[str, Requirement], dict[str, str]]:
+    """Build a validated ``Requirement`` dictionary from raw constraint strings.
+
+    Parameters
+    ----------
+    constraints (list):
+        A list of PEP-508 dependency specs, expected to contain both a package
+        name and at least one speicifier.
+
+    Returns
+    -------
+        A 2-tuple of:
+        - a dictionary of ``Requirement`` objects, keyed by canonical name
+        - a dictionary of messages strings, keyed by constraint
+    """
+    constrained_reqs: dict[str, Requirement] = {}
+    ignore_messages: dict[str, str] = {}
+
+    for raw_constraint in constraints or []:
+        try:
+            req = Requirement(raw_constraint)
+            req.name = canonicalize_name(req.name)
+        except InvalidRequirement as err:
+            ignore_messages[raw_constraint] = f"failed to parse: {err}"
+            continue
+
+        if req.extras:
+            ignore_messages[raw_constraint] = "may not provide [extras]"
+            continue
+
+        if not (req.url or len(req.specifier)):
+            ignore_messages[raw_constraint] = "no version or URL"
+            continue
+
+        constrained_reqs[req.name] = req
+
+    return constrained_reqs, ignore_messages
+
+
+def constrain_requirement(
+    requirement: Requirement, constrained_requirements: dict[str, Requirement]
+) -> Requirement:
+    """Refine or replace a requirement from a set of constraints.
+
+    Parameters
+    ----------
+    requirement (list):
+        A list of PEP-508 dependency specs, expected to contain both a package
+        name and at least one speicifier.
+
+    Returns
+    -------
+        A 2-tuple of:
+        - a dictionary of ``Requirement`` objects, keyed by canonical name
+        - a dictionary of messages strings, keyed by constraint
+    """
+    # URLs cannot be merged
+    if requirement.url:
+        return requirement
+
+    as_constrained = constrained_requirements.get(canonicalize_name(requirement.name))
+
+    if as_constrained:
+        if as_constrained.url:
+            requirement = as_constrained
+        else:
+            requirement.specifier = requirement.specifier & as_constrained.specifier
+
+    return requirement

micropip/transaction.py

@@ -11,7 +11,12 @@
 
 from . import package_index
 from ._compat import REPODATA_PACKAGES
-from ._utils import best_compatible_tag_index, check_compatible
+from ._utils import (
+    best_compatible_tag_index,
+    check_compatible,
+    constrain_requirement,
+    validate_constraints,
+)
 from .constants import FAQ_URLS
 from .package import PackageMetadata
 from .package_index import ProjectInfo
@@ -45,21 +50,13 @@ def __post_init__(self) -> None:
             self.index_urls == package_index.DEFAULT_INDEX_URLS
         )
 
-        self.constrained_reqs: dict[str, Requirement] = {}
-
-        for constraint in self.constraints or []:
-            con = Requirement(constraint)
-            if not con.name:
-                logger.debug("Transaction: discarding nameless constraint: %s", con)
-                continue
-            if con.extras:
-                logger.debug("Transaction: discarding [extras] constraint: %s", con)
-                continue
-            if not (con.url or len(con.specifier)):
-                logger.debug("Transaction: discarding versionless constraint: %s", con)
-                continue
-            con.name = canonicalize_name(con.name)
-            self.constrained_reqs[con.name] = con
+        self.constrained_reqs, messages = validate_constraints(self.constraints)
+
+        if self.verbose and messages:
+            for constraint, message in messages.items():
+                logger.info(
+                    "Transaction: constraint %s discarded: %s", constraint, message
+                )
 
     async def gather_requirements(
         self,
@@ -76,7 +73,7 @@ async def add_requirement(self, req: str | Requirement) -> None:
             return await self.add_requirement_inner(req)
 
         try:
-            req = self.constrain_requirement(Requirement(req))
+            req = constrain_requirement(Requirement(req), self.constrained_reqs)
             url = req.url
         except InvalidRequirement:
             url = f"{req}"
@@ -112,14 +109,6 @@ def check_version_satisfied(self, req: Requirement) -> tuple[bool, str]:
             f"Requested '{req}', " f"but {req.name}=={ver} is already installed"
         )
 
-    def constrain_requirement(self, req: Requirement) -> Requirement:
-        """Provide a constrained requirement, if available, or the original."""
-        constrained_req = self.constrained_reqs.get(canonicalize_name(req.name))
-        if constrained_req:
-            logger.debug("Transaction: %s constrained to %s", req, constrained_req)
-            return constrained_req
-        return req
-
     async def add_requirement_inner(
         self,
         req: Requirement,
@@ -129,10 +118,11 @@ async def add_requirement_inner(
         See PEP 508 for a description of the requirements.
         https://www.python.org/dev/peps/pep-0508
         """
+        # add [extras] first, as constraints will never add them
         for e in req.extras:
             self.ctx_extras.append({"extra": e})
 
-        req = self.constrain_requirement(req)
+        req = constrain_requirement(req, self.constrained_reqs)
 
         if self.pre:
             req.specifier.prereleases = True

tests/conftest.py

@@ -13,6 +13,7 @@
 from packaging.utils import parse_wheel_filename
 from pytest_httpserver import HTTPServer
 from pytest_pyodide import spawn_web_server
+from pytest_pyodide.runner import JavascriptException
 
 
 def pytest_addoption(parser):
@@ -408,3 +409,45 @@ def mock_package_index_simple_html_api(httpserver):
         suffix="_simple.html",
         content_type="text/html",
     )
+
+
+@pytest.fixture(
+    params=[
+        None,
+        "pytest ==7.2.2",
+        "pytest >=7.2.1,<7.2.3",
+        "pytest @ {url}",
+        "pytest @ emfs:{wheel}",
+    ]
+)
+def valid_constraint(request, wheel_catalog):
+    wheel = wheel_catalog.get("pytest")
+    if not request.param:
+        return request.param
+    return request.param.format(url=wheel.url, wheel=wheel.url.split("/")[-1])
+
+
+INVALID_CONSTRAINT_MESSAGES = {
+    "": "parse",
+    "http://example.com": "name",
+    "a-package[with-extra]": "[extras]",
+    "a-package": "no version or URL",
+}
+
+
+@pytest.fixture(params=[*INVALID_CONSTRAINT_MESSAGES.keys()])
+def invalid_constraint(request):
+    return request.param
+
+
+@pytest.fixture
+def run_async_py_in_js(selenium_standalone_micropip):
+    def _run(*lines, error_match=None):
+        js = "\n".join(["await pyodide.runPythonAsync(`", *lines, "`);"])
+        if error_match:
+            with pytest.raises(JavascriptException, match=error_match):
+                selenium_standalone_micropip.run_js(js)
+        else:
+            selenium_standalone_micropip.run_js(js)
+
+    return _run