Opened 2 years ago

Closed 2 years ago

#29813 closed enhancement (fixed)

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

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.2
Component: build Keywords:
Cc: Dima Pasechnik, Vincent Delecroix, François Bissey, John Palmieri, Michael Orlitzky, Erik Bray Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: bedc7ae (Commits, GitHub, GitLab) Commit: bedc7ae9cd5a134df1aa2a07170724bba18ee02f
Dependencies: #29345 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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).

Change History (21)

comment:1 Changed 2 years ago by Matthias Köppe

Cc: Vincent Delecroix added
Description: modified (diff)

comment:2 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/add_pytest_as_a_type_optional__source_pip_package

comment:3 Changed 2 years ago by git

Commit: 55babfaf2fdfe2c5d96f1fbcc29ce987737c29ee

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

comment:4 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe
Status: newneeds_review

comment:5 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:6 Changed 2 years ago by Matthias Köppe

Cc: François Bissey John Palmieri added

comment:7 Changed 2 years ago by Matthias Köppe

Status: needs_reviewneeds_work

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 Changed 2 years ago by Matthias Köppe

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

comment:9 Changed 2 years ago by Matthias Köppe

Cc: Michael Orlitzky Erik Bray added
Dependencies: #29345

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 in reply to:  9 Changed 2 years ago by Matthias Köppe

Replying to mkoeppe:

(That's exactly what's failing.)

Well, not exactly

comment:11 Changed 2 years ago by git

Commit: 55babfaf2fdfe2c5d96f1fbcc29ce987737c29eebedc7ae9cd5a134df1aa2a07170724bba18ee02f

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

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

comment:12 Changed 2 years ago by Matthias Köppe

Status: needs_workneeds_review

comment:13 Changed 2 years ago by Dima Pasechnik

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 Changed 2 years ago by Matthias Köppe

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

comment:15 Changed 2 years ago by Matthias Köppe

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 Changed 2 years ago by Dima Pasechnik

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

comment:17 Changed 2 years ago by Matthias Köppe

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 Changed 2 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik

lgtm

comment:19 Changed 2 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

comment:20 Changed 2 years ago by Matthias Köppe

Thanks!

comment:21 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/add_pytest_as_a_type_optional__source_pip_packagebedc7ae9cd5a134df1aa2a07170724bba18ee02f
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.