[PATCH] mkvenv: pass first missing package to diagnose()

[PATCH] mkvenv: pass first missing package to diagnose()

[Paolo Bonzini]

If sphinx is present but the theme is not, mkvenv will print an
inaccurate diagnostic:

ERROR: Could not find a version that satisfies the requirement sphinx-rtd-theme>=0.5.0 (from versions: none)
ERROR: No matching distribution found for sphinx-rtd-theme>=0.5.0

'sphinx>=1.6.0' not found:
 • Python package 'sphinx' version '5.3.0' was found, but isn't suitable.
 • mkvenv was configured to operate offline and did not check PyPI.

Instead, ignore the packages that were found to be present, and report
an error based on the first absent package.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 python/scripts/mkvenv.py | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 6c78a2c1120e..3a9aef46a513 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -722,7 +722,8 @@ def _do_ensure(
     dep_specs: Sequence[str],
     online: bool = False,
     wheels_dir: Optional[Union[str, Path]] = None,
-) -> None:
+    prog: Optional[str] = None,
+) -> Optional[Tuple[str, bool]]:
     """
     Use pip to ensure we have the package specified by @dep_specs.
 
@@ -752,10 +753,24 @@ def _do_ensure(
         generate_console_scripts(present)
 
     if absent:
-        # Some packages are missing or aren't a suitable version,
-        # install a suitable (possibly vendored) package.
-        print(f"mkvenv: installing {', '.join(absent)}", file=sys.stderr)
-        pip_install(args=absent, online=online, wheels_dir=wheels_dir)
+        if online or wheels_dir:
+            # Some packages are missing or aren't a suitable version,
+            # install a suitable (possibly vendored) package.
+            print(f"mkvenv: installing {', '.join(absent)}", file=sys.stderr)
+            try:
+                pip_install(args=absent, online=online, wheels_dir=wheels_dir)
+                return None
+            except subprocess.CalledProcessError:
+                pass
+
+        return diagnose(
+            absent[0],
+            online,
+            wheels_dir,
+            prog if absent[0] == dep_specs[0] else None,
+        )
+
+    return None
 
 
 def ensure(
@@ -785,14 +800,12 @@ def ensure(
     if not HAVE_DISTLIB:
         raise Ouch("a usable distlib could not be found, please install it")
 
-    try:
-        _do_ensure(dep_specs, online, wheels_dir)
-    except subprocess.CalledProcessError as exc:
+    result = _do_ensure(dep_specs, online, wheels_dir, prog)
+    if result:
         # Well, that's not good.
-        msg, bad = diagnose(dep_specs[0], online, wheels_dir, prog)
-        if bad:
-            raise Ouch(msg) from exc
-        raise SystemExit(f"\n{msg}\n\n") from exc
+        if result[1]:
+            raise Ouch(result[0])
+        raise SystemExit(f"\n{result[0]}\n\n")
 
 
 def post_venv_setup() -> None:
-- 
2.40.1

Re: [PATCH] mkvenv: pass first missing package to diagnose()

[John Snow]

On Fri, May 19, 2023 at 4:25 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> If sphinx is present but the theme is not, mkvenv will print an
> inaccurate diagnostic:
>
> ERROR: Could not find a version that satisfies the requirement sphinx-rtd-theme>=0.5.0 (from versions: none)
> ERROR: No matching distribution found for sphinx-rtd-theme>=0.5.0
>
> 'sphinx>=1.6.0' not found:
>  • Python package 'sphinx' version '5.3.0' was found, but isn't suitable.
>  • mkvenv was configured to operate offline and did not check PyPI.
>
> Instead, ignore the packages that were found to be present, and report
> an error based on the first absent package.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Late to the party, but ACK.