sagemath/sage

Add pytest as a type=optional, source=pip package

mkoeppe opened this issue · 22 comments

Following a discussion in #19680, #28998, because pytest is only needed if SAGE_CHECK=yes, standard packages are allowed to depend on this optional package -- if they make the dependency conditional on SAGE_CHECK (see #29766 for an example).

Depends on #29345

CC: @dimpase @videlec @kiwifb @jhpalmieri @orlitzky @embray

Component: build

Author: Matthias Koeppe

Branch/Commit: bedc7ae

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29813

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
-(Following a discussion in #19680, #28998.)
+Following a discussion in #19680, #28998, because `pytest` is only needed if `SAGE_CHECK=yes`, standard packages are allowed to depend on this optional package -- if they make the dependency conditional on `SAGE_CHECK`.
 
 

Branch pushed to git repo; I updated commit sha1. New commits:

55babfasrc/bin/sage [-i]: Set SAGE_CHECK here so that Makefile dependencies can depend on it

Author: Matthias Koeppe

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
-Following a discussion in #19680, #28998, because `pytest` is only needed if `SAGE_CHECK=yes`, standard packages are allowed to depend on this optional package -- if they make the dependency conditional on `SAGE_CHECK`.
+Following a discussion in #19680, #28998, because `pytest` is only needed if `SAGE_CHECK=yes`, standard packages are allowed to depend on this optional package -- if they make the dependency conditional on `SAGE_CHECK` (see #29766 for an example).
 
 
comment:7

In #29766 at https://github.com/mkoeppe/sage/runs/744591025:

 make[1]: *** No rule to make target '/sage/local/var/lib/sage/installed/pytest-none', needed by '/sage/local/var/lib/sage/installed/networkx-2.4'.
comment:8

Doesn't work because dependencies on pip packages is broken.

Dependencies: #29345

comment:9

From build/make/Makefile.in:

# Note: In these rules the $(INST)/<pkgname>-<pkgvers> target is used
# explicitly, rather than expanding the $(inst_<pkgname>) variable, since
# it may expand to $(INST)/.dummy for packages that were not configured
# for installation by default.

(That's exactly what's failing.)

comment:10

Replying to @mkoeppe:

(That's exactly what's failing.)

Well, not exactly

Branch pushed to git repo; I updated commit sha1. New commits:

bedc7aebuild/make/Makefile.in: Allow pip packages as dependencies

Changed commit from 55babfa to bedc7ae

comment:13

is this ready? I don't see how the current branch is going to affect installing this package if needed.

How about just force tox etc configs to install this package? (using a configure option).

comment:14

Yes, it's ready. Try it with the networkx ticket where it is merged.

comment:15

Of course the problem with this approach is that it depends on SSL...
for example on ubuntu-focal-standard (https://github.com/mkoeppe/sage/runs/748390636):

  [pytest]   Could not fetch URL https://pypi.org/simple/pytest/: There was a problem confirming the ssl certificate: HTTPSConnectionPool(host='pypi.org', port=443): Max retries exceeded with url: /simple/pytest/ (Caused by SSLError("Can't connect to HTTPS URL because the SSL module is not available.")) - skipping
  [pytest]   ERROR: Could not find a version that satisfies the requirement pytest (from -r /sage/build/pkgs/pytest/requirements.txt (line 1)) (from versions: none)
  [pytest]   ERROR: No matching distribution found for pytest (from -r /sage/build/pkgs/pytest/requirements.txt (line 1))
  [pytest]   WARNING: pip is configured with locations that require TLS/SSL, however the ssl module in Python is not available.
  [pytest]   Could not fetch URL https://pypi.org/simple/pip/: There was a problem confirming the ssl certificate: HTTPSConnectionPool(host='pypi.org', port=443): Max retries exceeded with url: /simple/pip/ (Caused by SSLError("Can't connect to HTTPS URL because the SSL module is not available.")) - skipping
  [pytest] Full log file: /sage/logs/pkgs/pytest.log
comment:16

Does this mean that SSL needs to be installed on the hosts to use pytest?
Why is this a problem?

comment:17

Yes. Not a big problem, but:

  • we need system package info for build/pkgs/openssl/distros (see also #29555)
  • it does not really fit well into the standard/optional/experimental categories (and hence for example the -standard build configurations in tox does not include it at the moment)
  • it is another potential source for user confusion
comment:18

lgtm

Reviewer: Dima Pasechnik

comment:20

Thanks!