#29287 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.1
Component: build Keywords:
Cc: embray, dimpase, jhpalmieri, tmonteil, vbraun Merged in:
Authors: Matthias Koeppe Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 0919086 (Commits, GitHub, GitLab) Commit: 091908665ae681b6f9cc1f491d4801acbdfb89ae
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

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.

Change History (48)

comment:1 Changed 19 months ago by mkoeppe

  • Branch set to u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_

comment:2 Changed 19 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 90054f65bfbba569d1d95d1de08931f2738ee35f

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}

comment:3 Changed 19 months ago by git

  • Commit changed from 90054f65bfbba569d1d95d1de08931f2738ee35f to f7a661c00eb396629a445b3395056b531f1afad3

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

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

comment:4 Changed 19 months ago by mkoeppe

  • Cc jhpalmieri added
  • Status changed from new to needs_review

comment:5 Changed 19 months ago by jhpalmieri

  • Status changed from needs_review to needs_work

This needs documentation.

comment:6 Changed 19 months ago by mkoeppe

Indeed, as noted already in #21033.

comment:7 Changed 19 months ago by jhpalmieri

  • Dependencies set to #21033

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

comment:8 Changed 19 months ago by mkoeppe

Yes, I'll work on it

comment:9 Changed 18 months ago by mkoeppe

  • Dependencies changed from #21033 to #29096

comment:10 Changed 18 months ago by git

  • Commit changed from f7a661c00eb396629a445b3395056b531f1afad3 to f819b51bbccb7b13fb9bf12c4f6e6aefb37bc3b6

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_

comment:11 Changed 18 months ago by mkoeppe

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

comment:12 Changed 18 months ago by mkoeppe

  • Description modified (diff)
  • Summary changed from SPKG type: Make "regular/script/pip" orthogonal to "standard/optional/experimental" to SPKG type: Make "normal/script/pip" orthogonal to "standard/optional/experimental"

comment:13 Changed 18 months ago by git

  • Commit changed from f819b51bbccb7b13fb9bf12c4f6e6aefb37bc3b6 to 678f4cd2b66a30481f1160af45a70fb8b36350ac

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

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

comment:14 Changed 18 months ago by git

  • Commit changed from 678f4cd2b66a30481f1160af45a70fb8b36350ac to b80d88f851cc3ab3119ac5da25d4c5636e415ba8

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

comment:15 Changed 18 months ago by git

  • Commit changed from b80d88f851cc3ab3119ac5da25d4c5636e415ba8 to 4793466dba3f397ee0e4d60084cc8e7e2b5fed67

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

4793466build/pkgs/sage_conf: Change type to standard

comment:16 Changed 18 months ago by mkoeppe

Added documentation describing normal, pip, and script packages.

comment:17 Changed 18 months ago by git

  • Commit changed from 4793466dba3f397ee0e4d60084cc8e7e2b5fed67 to a6cea8e03928741fc0830e83dd85430d313ed2f9

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

72e7719m4/sage_spkg_collect.m4: Remove handling of type=script
dcaaa85m4/sage_spkg_collect.m4: Remove special-casing of unversioned (script/pip) packages
52902ebm4/sage_spkg_collect.m4: For unversioned packages, set version to 'none' instead of name of spkg
ff871b7build/make/Makefile.in: Version and record script packages like normal packages
aa8cdc5Update doc regarding versioning/recording of script packages
a6cea8ebuild/make/Makefile.in: Mark the targets SPKG and SPKG-clean as phony

comment:18 Changed 18 months ago by git

  • Commit changed from a6cea8e03928741fc0830e83dd85430d313ed2f9 to d6005a301cd99a4e549a84fe59ecbd6dfddb08d7

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

d6005a3src/bin/sage-list-packages: Remove use of type=pip

comment:19 Changed 18 months ago by git

  • Commit changed from d6005a301cd99a4e549a84fe59ecbd6dfddb08d7 to d4b4d81f47f538acf6eef1468634502a7bb00b98

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

d4b4d81sage.misc.package.list_packages: For unversioned packages, use 'none' as version

comment:20 Changed 18 months ago by git

  • Commit changed from d4b4d81f47f538acf6eef1468634502a7bb00b98 to cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae

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

ec2805em4/sage_spkg_collect.m4: Fix up collection of dependencies
cb3c913build/pkgs/texlive/spkg-install: Fix up path

comment:21 Changed 18 months ago by mkoeppe

  • Cc tmonteil vbraun added
  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from SPKG type: Make "normal/script/pip" orthogonal to "standard/optional/experimental" to SPKG type: Make "normal/script/pip" orthogonal to "base/standard/optional/experimental"

comment:22 follow-up: Changed 18 months ago by vdelecroix

Why do you had all these .in? Such a change should be agreed on sage-devel.

comment:23 Changed 18 months ago by vdelecroix

Unless I misunderstood something, in the section Install scripts of script packages, ``spkg-install`` should be ``spkg-install.in`` (twice).

comment:24 Changed 18 months ago by mkoeppe

Changing to .in happened on #29096.

No, script packages don't to templating.

comment:25 in reply to: ↑ 22 Changed 18 months ago by dimpase

Replying to vdelecroix:

Why do you had all these .in? Such a change should be agreed on sage-devel.

we had an argument about it, and that change should have been done when the actual conversion of most of sage-install scripts into templates happened, a couple of years back. So this change merely corrects an old fault of the naming scheme.

comment:26 Changed 18 months ago by git

  • Commit changed from cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae to dbb9860cb41006fbe4108447f4f01e26ed8f0bdd

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

ad9e524m4/sage_spkg_collect.m4: Remove handling of type=script
6c977bem4/sage_spkg_collect.m4: Remove special-casing of unversioned (script/pip) packages
d4e8cd9m4/sage_spkg_collect.m4: For unversioned packages, set version to 'none' instead of name of spkg
8370225build/make/Makefile.in: Version and record script packages like normal packages
495d499Update doc regarding versioning/recording of script packages
bc4ed34build/make/Makefile.in: Mark the targets SPKG and SPKG-clean as phony
d1fd7a7src/bin/sage-list-packages: Remove use of type=pip
75175dbsage.misc.package.list_packages: For unversioned packages, use 'none' as version
d3d78f4m4/sage_spkg_collect.m4: Fix up collection of dependencies
dbb9860build/pkgs/texlive/spkg-install: Fix up path

comment:27 Changed 18 months ago by mkoeppe

  • Dependencies #29096 deleted

Rebased on 9.1.beta8 -- so the diff is easier to read now. Needs review

comment:28 follow-up: Changed 18 months ago by 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
**********************************************************************
1 item had failures:
   1 of   5 in sage.env
    [45 tests, 1 failure, 1.41 s]
----------------------------------------------------------------------
sage -t --long --warn-long 65.3 src/sage/env.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 1.4 seconds
    cpu time: 0.1 seconds
    cumulative wall time: 1.4 seconds

comment:29 Changed 18 months ago by jhpalmieri

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

comment:30 Changed 18 months ago by mkoeppe

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

comment:31 Changed 18 months ago by jhpalmieri

What do you think about this proposed change:

  • src/doc/en/developer/packaging.rst

    diff --git a/src/doc/en/developer/packaging.rst b/src/doc/en/developer/packaging.rst
    index 6934f4ebbf..34f5651bc1 100644
    a b the following source types: 
    8787   - is obtained directly from https://pypi.org/;
    8888
    8989   - the version to be installed is determined using the required file
    90      ``requirements.txt`` (see
    91      https://pip.pypa.io/en/stable/reference/pip_install/#requirements-file-format);
     90     ``requirements.txt`` -- in its simplest form, this file just
     91     contains the name of the package (more details at
     92     https://pip.pypa.io/en/stable/user_guide/#requirements-files);
    9293
    9394   - Sage installs the package using the ``pip`` package manager;
    9495
Version 0, edited 18 months ago by jhpalmieri (next)

comment:32 Changed 18 months ago by mkoeppe

Excellent, please push it to the ticket

comment:33 Changed 18 months ago by jhpalmieri

  • Branch changed from u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ to u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_

comment:34 Changed 18 months ago by jhpalmieri

  • Commit changed from dbb9860cb41006fbe4108447f4f01e26ed8f0bdd to 8b1d5ca6074561315be14ea4f072149b88fe2fd0

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


New commits:

8b1d5catrac 29287: rewording documentation

comment:35 Changed 18 months ago by mkoeppe

Good idea. I'll make this change.

comment:36 Changed 18 months ago by mkoeppe

  • Branch changed from u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ to u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_

comment:37 in reply to: ↑ 28 Changed 18 months ago by mkoeppe

  • Commit changed from 8b1d5ca6074561315be14ea4f072149b88fe2fd0 to 057a66bdd05ad75539f2bdbd27561af3f0d57d65

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

comment:38 Changed 18 months ago by git

  • Commit changed from 057a66bdd05ad75539f2bdbd27561af3f0d57d65 to 8f6323ed8af19d0c1301c65e41d3b219a8fa7a43

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

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

comment:39 Changed 18 months ago by mkoeppe

  • Description modified (diff)

comment:40 Changed 18 months ago by jhpalmieri

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

comment:41 Changed 18 months ago by mkoeppe

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.

comment:42 follow-up: Changed 18 months ago by jhpalmieri

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

comment:43 in reply to: ↑ 42 Changed 18 months ago by mkoeppe

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.

comment:44 Changed 18 months ago by jhpalmieri

  • Branch changed from u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ to u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_

comment:45 Changed 18 months ago by jhpalmieri

  • Commit changed from 8f6323ed8af19d0c1301c65e41d3b219a8fa7a43 to 091908665ae681b6f9cc1f491d4801acbdfb89ae
  • Reviewers set to John Palmieri

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


New commits:

0919086trac 29287: doc fixes

comment:46 Changed 18 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Great, thanks very much.

comment:47 Changed 18 months ago by mkoeppe

  • Priority changed from major to critical

comment:48 Changed 18 months ago by vbraun

  • Branch changed from u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ to 091908665ae681b6f9cc1f491d4801acbdfb89ae
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.