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

Meta-ticket: Downstream patch upstreaming #31016

Open
mkoeppe opened this issue Dec 6, 2020 · 33 comments
Open

Meta-ticket: Downstream patch upstreaming #31016

mkoeppe opened this issue Dec 6, 2020 · 33 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Dec 6, 2020

https://wiki.sagemath.org/days111

See also: https://wiki.sagemath.org/Distribution

Issues:

cp -r sage/tests/books "$pkgdir"/$_pythonpath/sage/tests

CC: @tobihan @antonio-rojas @kiwifb @orlitzky @isuruf @jamesjer @slel @tornaria @culler

Component: porting

Keywords: sd111

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Dec 6, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kiwifb

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kiwifb

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Dec 9, 2020

comment:8

Should I add my language selection options for building the documentation?

@tobihan
Copy link

tobihan commented Dec 9, 2020

comment:9

Some of the reasons we still run configure and make to build the Debian package:

  • In order to run the tests during package build we need a working sage (without installing it into the system).
  • We are still installing var/libs/sage/installed since it is (or at least used to be) needed to detect which optional packages are installed.
  • It is hard to keep track how to build the parts other than sagelib separately (doc, jupyter kernel...) and to set everything up for running the tests before system installation.

Will #29013 and ./configure --with-python=no help with these problems? Will there also be a make install for the non-python parts of sage?

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2020

comment:10

Replying to @tobihan:

Some of the reasons we still run configure and make to build the Debian package:

  • In order to run the tests during package build we need a working sage (without installing it into the system).
  • We are still installing var/libs/sage/installed since it is (or at least used to be) needed to detect which optional packages are installed.
  • It is hard to keep track how to build the parts other than sagelib separately (doc, jupyter kernel...) and to set everything up for running the tests before system installation.

These are of course all valid points. The second point will hopefully be resolved soon in the course of the modularization, as the build will no longer depend on the presence of optional libraries.

I hope we can resolve the third point soon. For example, I hope that we can put the docbuild into a separate Python package - that's #29868.

Will #29013 and ./configure --with-python=no help with these problems?

Yes, I think it would offer a good route of transition for Debian packaging of Sage. With ./configure --with-python=no, which would imply ./configure --with-sage-venv=no (#30896), plain make would just install the non-Python parts of sage.

You could then install the Python library of sage following the standard way how you package Python packages in Debian.

@mkoeppe

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2020

comment:12

After double checking we already fixed the installation of text files. It looks like you may have been looking at an old ebuild. If not, that's something that should be updated to be current on my side.

@kiwifb

This comment has been minimized.

@antonio-rojas
Copy link
Contributor

comment:13

Replying to @kiwifb:

After double checking we already fixed the installation of text files. It looks like you may have been looking at an old ebuild. If not, that's something that should be updated to be current on my side.

Those lines came from the Arch PKGBUILD. Indeed it looks like doctest/tests is already being installed by setup.py, but tests/books is not.

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2020

comment:14

Indeed I don't have them installed! It looks like they are python files so they having an __init__.py file in the affected directories should fix the issue I think.

@kiwifb

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2020

comment:16

Replying to @kiwifb:

Indeed I don't have them installed! It looks like they are python files so they having an __init__.py file in the affected directories should fix the issue I think.

Putting __init__.py solves 99% of the problem. There is just a text file which we should decide whether or not to ship (it is not tested). This is a README
https://github.com/sagemath/sage-prod/blob/develop/src/sage/tests/books/computational-mathematics-with-sagemath/README

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2020

comment:17

I should add that using __init__.py is my favored solution because it also means that pycache will be autogenerated when installing. If I ship non precompiled python files, Gentoo automated QA will be on my back to do it manually.

@kiwifb

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 7, 2021

comment:21

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Apr 7, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 10, 2021
@mkoeppe

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:25

bump to 9.6

@fchapoton fchapoton modified the milestones: sage-9.5, sage-9.6 Jan 29, 2022
@tornaria

This comment has been minimized.

@tornaria
Copy link
Contributor

tornaria commented Feb 4, 2022

comment:26

As of yesterday sagemath-9.5 is available on official void repos, working for x86_64 (glibc and musl libc) and i686 (32 bit, glibc only).

The build template is here:

https://github.com/void-linux/void-packages/tree/master/srcpkgs/sagemath

Notes:

  • after a few months of playing with installing sage-as-distro, I finally switched to installing sage-as-lib as it was suggested, and it's really much easier. However, starting with sage-as-distro was better while we were working on the other package dependencies. Once all packages were available switching to sage-as-lib was really easy.
  • we are applying 15 patches; of those:
    • the 11 starting with numbers in patches should IMO be straightforward to upstream (as-is, except maybe 08-dont_run_pytest--see_31924.patch). All of them are very short except the last two which are just improving on # long tagging (thus non-essential but nice to have). I'll try to open individual tickets here for these.
    • two patches trac-* are taken from tickets with positive review Make tests pass with arb 2.22 #33189 and Support giac-1.7.0.45 in doctests #33226.
    • two patches zzz-* are void specific, although the one about loadable_module_extension() I can think of a way to fix it in a generic way. The one about threejs... well, I just need to ship threejs but whatever, that's a stupid test since nothing else requires threejs, I might as well ship an empty directory to pass the test!
  • The template itself:
    • Removes sage/libs/ratpoints.{pyx,pxd} since these are not used anywhere; maybe ratpoints spkg can be made optional and compile these files only if ratpoints is available?

    • Builds from pkgs/sagemath-standard, however bootstrapping this requires some non-obvious moves. Can we have a bootstrap script in this directory that does "the right thing"? Here is my snippet:

      pushd $wrksrc/build/pkgs/sagelib
      PATH=../../bin:$PATH BOOTSTRAP_QUIET=no ./bootstrap
      popd
      
    • Patches setup.cfg so that sage-venv-config is not installed; that should not be necessary, besides it's actively harmful since having it set means sage-env will mess up with PATH in an undesirable way. In fact it's very hard to understand what is going on with PATH in sage-env (I guess it's just that there are a zillion different cases to be supported, but it's quite opaque) and it's missing at least the case I end up requiring (see next point).

    • Creates a custom sage-env-config script with a single line PATH=$(dirname $SELF):$PATH; we need this because scripts will be installed in a custom location not in system path (currently /usr/lib/sagemath/bin) with a symlink to the sage script placed in /usr/bin.

    • Configures some paths by appending to sage/env.py, that file is kind of weird, mixing settings with code and it keeps configuration variables in two different places: the module dict itself, and the SAGE_ENV dict. There's like 3 users of SAGE_ENV, wouldn't it make sense to kill it for consistency? The only reason I had to touch variables is because I decided to place databases in /usr/share/sagemath instead of directly in /usr/share, my fault I guess.

  • Other notes
    • Running setup.py needs PYTHONPATH=../sage-setup, I took that from arch. Should this (and the previous bootstrap step) be documented in README.rst?
    • Building with multiple jobs works fine when setting SAGE_NUM_THREADS to the number of threads (also took that hint from arch, also nice to document).
    • The jupyter kernel install is bad: it installs symlinks for the logo which point to the build directory, which won't exist at runtime. I replace the symlinks by hard copies. I also change the spec from running /usr/bin/sage --python to /usr/bin/python because that works regardless of where the sage script is installed (I hope I'm not missing anything with that.. it seems to work ok)
    • Doctesting the built module (without install) works fine provided:
    • We run the sage script in build/scripts-*/sage and either add build/scripts-* to PATH or do the PATH=$(dirname $SELF):$PATH trick I mentioned above.
    • We set PYTHONPATH to build/lib*.
    • We change directory to some other place (I create an empty temp dir). It turns out that if we set PYTHONPATH, then python will prepend $PWD to that (wtf?) so running from pkgs/sagemath-standard will make it pick the sage module from the wrong location (the source one, which doesn't contain the compiled extension modules). I don't know a way to avoid this $PWD misfeature.

What could be upstreamed from the above that might be useful

  • patches, as I mentioned above, this I will do in separate tickets
  • make ratpoints optional
  • have a bootstrap script in pkgs/sagemath-standard
  • do not install sage-venv-config ??? Or another way that results in SAGE_VENV unset.
  • do PATH=$(dirname $SELF):$PATH in sage-env (perhaps when dirname $SELF is not already in PATH, when SAGE_LOCAL, SAGE_ROOT and SAGE_VENV are unset, or something).
  • change jupyter install so that it makes hard copies of the logos instead of symlinks to build dir.

@tobihan

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 5, 2022

comment:28

Replying to @tornaria:

  • Builds from pkgs/sagemath-standard, however bootstrapping this requires some non-obvious moves. Can we have a bootstrap script in this directory that does "the right thing"? Here is my snippet:

      pushd $wrksrc/build/pkgs/sagelib
      PATH=../../bin:$PATH BOOTSTRAP_QUIET=no ./bootstrap
      popd
    

The bootstrap scripts do need access to build/pkgs/ for package version information. The idea is that you bootstrap the whole repo using the top-level bootstrap script, not just one directory in pkgs/

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 5, 2022

comment:29

Replying to @tornaria:

  • make ratpoints optional

From build/pkgs/ratpoints/SPKG.rst:

NOTE: the ratpoints package has been assimilated by PARI/GP. Therefore,
this package (as Sage package) is deprecated. In the future, it will be
removed from Sage.

I think we should just do that

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 5, 2022

comment:30

Replying to @tornaria:

  • Running setup.py needs PYTHONPATH=../sage-setup, I took that from arch. Should this (and the previous bootstrap step) be documented in README.rst?

No, sage-setup is declared as a build-system requires in pkgs/sagemath-standard/pyproject.toml.m4

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 5, 2022

comment:31

Replying to @tornaria:

  • The jupyter kernel install is bad

See #30306

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 5, 2022

comment:32

Replying to @tornaria:

  • Configures some paths by appending to sage/env.py, that file is kind of weird, mixing settings with code and it keeps configuration variables in two different places: the module dict itself, and the SAGE_ENV dict.

Don't do that, put settings in sage_conf.py instead (see also #33295)

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

6 participants