Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sage.doctest.control: Replace use of sage.misc.package.list_packages #30746

Open
mkoeppe opened this issue Oct 8, 2020 · 28 comments
Open

sage.doctest.control: Replace use of sage.misc.package.list_packages #30746

mkoeppe opened this issue Oct 8, 2020 · 28 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 8, 2020

... by features.

This is so that users can make use of doctests conditionalized with "# optional" in their Sage programs even on distribution-packaged Sage installations, or for optional packages with spkg-configure.m4 (as observed in #30887 comment:28)

After #30887, #32614, #32649 a Feature has a name attribute that can be used as the # optional tag for doctesting.

#32174 implements global Feature discovery and hooks it into sage.doctest.control.

It remains to define Features for all remaining SPKGs that appear in # optional tags somewhere in the Sage library (which can be found using git grep -l '# *optional' | xargs sed -E -n 's/^.*# *optional[ -:]*([a-zA-Z_0-9.]*).*/\1/gp' | sort -u). Then we can remove the use of sage.misc.package.

Related:

CC: @antonio-rojas @kiwifb @simon-king-jena @dimpase @jhpalmieri @kliem @tscrim @kwankyu @orlitzky

Component: doctest framework

Keywords: sd111

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Oct 8, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 8, 2020

comment:1

How do distributions handle this currently?

@mkoeppe

This comment has been minimized.

@antonio-rojas
Copy link
Contributor

comment:3

I'm patching it out completely

https://github.com/archlinux/svntogit-community/blob/packages/sagemath/trunk/test-optional.patch

so tests depending on optional packages are skipped unless explicitely specified in the sage -t command

@kiwifb
Copy link
Member

kiwifb commented Oct 8, 2020

comment:4

Replying to @mkoeppe:

How do distributions handle this currently?

I don't know about others but I do it badly. The current stuff allow some autodetection of what is installed by the sage package management system. That was not always the case. I remove the autodetection bits and add anything I want to test manually when running sage -t.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2020

comment:7

(wrong ticket)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

Changed keywords from none to sd111

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

comment:9

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2021

comment:10

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 8, 2021

Dependencies: #20879

@kiwifb
Copy link
Member

kiwifb commented Jul 9, 2021

comment:14

Why the dependency on #20879? I am assuming this is a typo or the wrong ticket.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 9, 2021

Changed dependencies from #20879 to #30887

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 9, 2021

comment:15

Yes, wrong ticket, I meant #30887.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Changed dependencies from #30887 to #30887, #32614

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 15, 2021

Changed dependencies from #30887, #32614 to #30887, #32614, #32649

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@orlitzky
Copy link
Contributor

comment:22

I'm just seeing this now after wondering why sage keeps searching my system for random broken programs to try to use. Runtime detection of these things will not be a good long-term approach:

  1. You have to reimplement every spkg-configure.m4 in python for the feature check and keep them synchronized forever.
  2. Runtime checks are run over and over again. And many are going to be slow, once they are made to return the right answers.
  3. Distros track dependencies and usually don't want them to (dis)appear at runtime.
  4. If I build sage with e.g. --disable-lrslib, I expect it to not search my system for something that looks like lrslib and use it anyway. There has to be a way to reliably disable things.

As an alternative, you can imagine a shared location where "features" record their existence (think pkg-config). The simplest implementation might be to create a file named after the feature to indicate that the feature is present.

Optional packages would touch $FEATURESDIR/$PACKAGE upon being detected or installed. That centralizes all of the detection code in spkg-configure.m4, and would eventually make --disable-foo work again. Distros could install a "feature file" for every feature that their distro package provides. Or if they don't mind the automagic dependencies, you could have (say) the "ffmpeg" package install the ffmpeg feature file to tell sage that ffmpeg is available. Of course this would only be done if the distro's ffmpeg package actually works with its sage. But it does allow the user to add features on a binary distro on-the-fly without the developers having to jump through too many hoops. (On Gentoo we might just make you rebuild with USE=feature to enable it and encode the dependency.)

Depending on the scenario, this essentially runs the "feature check" either once or never, as [ -f $feature ] is free. Thus we avoid wasting time on every invocation of sage -t.

The file-based implementation here is not well-thought-out and probably has flaws, but it's also not what's important. What is is that the features should respect the user/distro configuration, not waste too many resources, not cause surprises, and not require any unnecessary duplication (i.e. work). Moving towards runtime-only detection is IMHO moving backwards.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 15, 2022

comment:23

+1 on a mechanism for statically declaring features present/not present, for use by downstream distributions. They could be provided through sage_conf.

-1 on defining file system stuff, config files, etc.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 15, 2022

comment:24

Replying to @orlitzky:

  1. You have to reimplement every spkg-configure.m4 in python for the feature check and keep them synchronized forever.

A more positive perspective on that would be that eventually, when we can assume that there is an at least mildly usable python3 available at ./configure time, we can replace many spkg-configure.m4 tests by Feature tests, written in Python.

@orlitzky
Copy link
Contributor

comment:25

Replying to @mkoeppe:

+1 on a mechanism for statically declaring features present/not present, for use by downstream distributions. They could be provided through sage_conf.

-1 on defining file system stuff, config files, etc.

With sleep comes clarity. Instead of something like pkg-config, why don't we literally use pkg-config for this? To each feature, associate sage-$feature.pc. This meets all the criteria above without any hacky wheel reinventing. Distros are already familiar with the concept. PKG_CONFIG_PATH tricks should let us avoid version mismatch issues when, say, a git checkout is used on a system where a distro sage is also installed.

A more positive perspective on that would be that eventually, when we can assume that there is an at least mildly usable python3 available at ./configure time, we can replace many spkg-configure.m4 tests by Feature tests, written in Python.

Maybe in some cases, but in general we aren't going to be able to easily replace well-tested autoconf macros with hand-written python. Consequently we'll wind up with the same machinery implemented in two different places in two different languages.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 15, 2022

comment:26

Replying to @orlitzky:

A more positive perspective on that would be that eventually, when we can assume that there is an at least mildly usable python3 available at ./configure time, we can replace many spkg-configure.m4 tests by Feature tests, written in Python.

Maybe in some cases, but in general we aren't going to be able to easily replace well-tested autoconf macros with hand-written python. Consequently we'll wind up with the same machinery implemented in two different places in two different languages.

The trickiest spkg-configure.m4 tests are for compile-time stuff - existence of libraries and such - and they do not need to be implemented in sage.features.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 15, 2022

comment:27

A possible solution could be to go through #31277.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
@mkoeppe mkoeppe added this to the sage-10.5 milestone Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants