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

SPKG type: Make "normal/script/pip" orthogonal to "base/standard/optional/experimental" #29287

Closed
mkoeppe opened this issue Mar 6, 2020 · 65 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 6, 2020

This ticket, inspired by a proposal by embray in #24586, removes the misuse of "package type" for the undocumented "script package" category (introduced in #19427). Likewise, it removes the same misuse of "package type" for the undocumented (as noted in #21033) "pip package" category (introduced in #19187).

With this ticket, type has to be one of base / standard / optional / experimental.

All type=pip packages are changed to optional.

All type=script packages are changed to optional; with the exception of sage_conf, which is a dependency of sagelib and is changed to standard.

Orthogonal to type is the new notion of a package "source", which is defined as follows:

  • normal packages have a checksum.ini file
  • pip packages have a requirements.txt file instead (which is used for pip install). (This allows us to change the pyopenssl package from script to pip.)
  • script packages have neither of the two

The ticket also makes script packages more similar to normal packages: They can now optionally have a package-version.txt file; and their installation status is recorded.

Finally, this ticket adds documentation to the developer manual.

CC: @embray @dimpase @jhpalmieri @sagetrac-tmonteil @vbraun

Component: build

Author: Matthias Koeppe

Branch/Commit: 0919086

Reviewer: John Palmieri

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

@mkoeppe mkoeppe added this to the sage-9.1 milestone Mar 6, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2020

Commit: 90054f6

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2020

New commits:

c5dcfd6build/pkgs: Change all 'type=pip' packages to 'type=optional' and add requirements.txt
9a9b3c3build/pkgs: Change all 'type=script' packages to 'type=optional'
90054f6m4/sage_spkg_collect.m4: Make SPKG_SOURCE={normal|pip|script} orthogonal to SPKG_TYPE={base,standard,optional,experimental}

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2020

Changed commit from 90054f6 to f7a661c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2020

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

f7a661csage.misc.package.list_packages: New argument pkg_sources, orthogonal to pkg_types

@jhpalmieri
Copy link
Member

comment:5

This needs documentation.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2020

comment:6

Indeed, as noted already in #21033.

@jhpalmieri
Copy link
Member

comment:7

Just putting the list in the description somewhere in the docs would help. What is a "script" package, anyway?

@jhpalmieri
Copy link
Member

Dependencies: #21033

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2020

comment:8

Yes, I'll work on it

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 14, 2020

Changed dependencies from #21033 to #29096

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

09cc8babuild/pkgs: Change all 'type=pip' packages to 'type=optional' and add requirements.txt
8a8100abuild/pkgs: Change all 'type=script' packages to 'type=optional'
ca47dc4m4/sage_spkg_collect.m4: Make SPKG_SOURCE={normal|pip|script} orthogonal to SPKG_TYPE={base,standard,optional,experimental}
ff4db4bsage.misc.package.list_packages: New argument pkg_sources, orthogonal to pkg_types
041c76dRename spkg-build, spkg-install etc. templates as spkg-build.in, spkg-install.in etc.
4524b04Update developer manual
2724083Undo rename of build/pkgs/*/spkg-install.py
ec540ddFix up sage-spkg
f819b51Merge branch 't/29096/rename_spkg_build__spkg_install_etc__templates_as_spkg_build_in__spkg_install_in_etc_' into t/29287/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

Changed commit from f7a661c to f819b51

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 14, 2020

comment:11

Rebased on 9.1.beta7, merged #29096 to avoid conflicts in documentation

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title SPKG type: Make "regular/script/pip" orthogonal to "standard/optional/experimental" SPKG type: Make "normal/script/pip" orthogonal to "standard/optional/experimental" Mar 14, 2020
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

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

678f4cdsrc/doc/en/developer/packaging.rst: Update doc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

Changed commit from f819b51 to 678f4cd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

Changed commit from 678f4cd to b80d88f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2020

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

28d5c92build/make/Makefile.in: Install pip packages using 'pip install -r requirements.txt'
b80d88fUpdate doc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

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

4793466build/pkgs/sage_conf: Change type to standard

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

Changed commit from b80d88f to 4793466

@jhpalmieri
Copy link
Member

comment:29

I don't know m4 syntax. Is the indentation remaining after you delete if test "$SPKG_NAME" != "$SPKG_VERSION"; then meaningful?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:30

No, it is not meaningful, I was just keeping the diff small.

@jhpalmieri
Copy link
Member

comment:31

What do you think about this proposed change:

diff --git a/src/doc/en/developer/packaging.rst b/src/doc/en/developer/packaging.rst
index 6934f4ebbf..34f5651bc1 100644
--- a/src/doc/en/developer/packaging.rst
+++ b/src/doc/en/developer/packaging.rst
@@ -87,8 +87,9 @@ the following source types:
    - is obtained directly from https://pypi.org/;
 
    - the version to be installed is determined using the required file
-     ``requirements.txt`` (see
-     https://pip.pypa.io/en/stable/reference/pip_install/#requirements-file-format);
+     ``requirements.txt`` -- in its simplest form, this file just
+     contains the name of the package (more details at
+     https://pip.pypa.io/en/stable/user_guide/#requirements-files);
 
    - Sage installs the package using the ``pip`` package manager;
 

Note: different link, in addition to the changed text.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:32

Excellent, please push it to the ticket

@jhpalmieri
Copy link
Member

comment:34

Not necessarily for this ticket, but should pyopenssl be a pip package?


New commits:

8b1d5catrac 29287: rewording documentation

@jhpalmieri
Copy link
Member

Changed commit from dbb9860 to 8b1d5ca

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:35

Good idea. I'll make this change.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:37

Replying to @jhpalmieri:

I get a doctest failure:

sage -t --long --warn-long 65.3 src/sage/env.py
**********************************************************************
File "src/sage/env.py", line 14, in sage.env
Failed example:
    res = subprocess.call([sys.executable, "-c", cmd], env=env)  # long time
Expected:
    None
Got:
    /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/29090/29287/sage-9.1.beta8

Ha, this doctest is funny. I'll fix it.


New commits:

057a66bMake pyopenssl a pip package

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

Changed commit from 8b1d5ca to 057a66b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

Changed commit from 057a66b to 8f6323e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

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

8f6323esrc/sage/env.py: Fix up doctest on starting sage without SAGE_* variables

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:40

Can you explain the doctest failure? For some reason, there was no output before, but now there is output?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:41

The doctest was actually "broken" by #29038 in the situation when the sage_conf package that it added is installed.
See ticket description of #29038 -- in particular, it provides SAGE_ROOT in the situation that Python imports sage.all outside of a sage-env.
That doctest was testing that SAGE_ROOT is not provided in that situation -- when it should simply have tested that the sage.all module works.

The present ticket makes sure that sage_conf is always installed (fixes #29365, for example), and so you now saw the doctest failure.

@jhpalmieri
Copy link
Member

comment:42

Why do we need to give a longer path in the spkg-install file for texlive?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:43

Replying to @jhpalmieri:

Why do we need to give a longer path in the spkg-install file for texlive?

This is an incidental bugfix for something that was broken by some earlier ticket. Seems nobody has tried sage -i texlive in a while.

@jhpalmieri
Copy link
Member

Changed commit from 8f6323e to 0919086

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:45

A few small changes. If you're happy with these, positive review from me.


New commits:

0919086trac 29287: doc fixes

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 19, 2020

comment:46

Great, thanks very much.

@vbraun
Copy link
Member

vbraun commented Mar 25, 2020

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

5 participants