Opened 2 years ago
Closed 2 years ago
#29287 closed enhancement (fixed)
SPKG type: Make "normal/script/pip" orthogonal to "base/standard/optional/experimental"
Reported by:  mkoeppe  Owned by:  

Priority:  critical  Milestone:  sage9.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: 
Description (last modified by )
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 achecksum.ini
filepip
packages have arequirements.txt
file instead (which is used forpip install
). (This allows us to change thepyopenssl
package fromscript
topip
.)script
packages have neither of the two
The ticket also makes script
packages more similar to normal
packages: They can now optionally have a packageversion.txt
file; and their installation status is recorded.
Finally, this ticket adds documentation to the developer manual.
Change History (48)
comment:1 Changed 2 years ago by
 Branch set to u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_
comment:2 Changed 2 years ago by
 Commit set to 90054f65bfbba569d1d95d1de08931f2738ee35f
comment:3 Changed 2 years ago by
 Commit changed from 90054f65bfbba569d1d95d1de08931f2738ee35f to f7a661c00eb396629a445b3395056b531f1afad3
Branch pushed to git repo; I updated commit sha1. New commits:
f7a661c  sage.misc.package.list_packages: New argument pkg_sources, orthogonal to pkg_types

comment:4 Changed 2 years ago by
 Cc jhpalmieri added
 Status changed from new to needs_review
comment:5 Changed 2 years ago by
 Status changed from needs_review to needs_work
This needs documentation.
comment:6 Changed 2 years ago by
Indeed, as noted already in #21033.
comment:7 Changed 2 years ago by
 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 2 years ago by
Yes, I'll work on it
comment:9 Changed 2 years ago by
 Dependencies changed from #21033 to #29096
comment:10 Changed 2 years ago by
 Commit changed from f7a661c00eb396629a445b3395056b531f1afad3 to f819b51bbccb7b13fb9bf12c4f6e6aefb37bc3b6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
09cc8ba  build/pkgs: Change all 'type=pip' packages to 'type=optional' and add requirements.txt

8a8100a  build/pkgs: Change all 'type=script' packages to 'type=optional'

ca47dc4  m4/sage_spkg_collect.m4: Make SPKG_SOURCE={normalpipscript} orthogonal to SPKG_TYPE={base,standard,optional,experimental }

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

041c76d  Rename spkgbuild, spkginstall etc. templates as spkgbuild.in, spkginstall.in etc.

4524b04  Update developer manual

2724083  Undo rename of build/pkgs/*/spkginstall.py

ec540dd  Fix up sagespkg

f819b51  Merge 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 2 years ago by
Rebased on 9.1.beta7, merged #29096 to avoid conflicts in documentation
comment:12 Changed 2 years ago by
 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 2 years ago by
 Commit changed from f819b51bbccb7b13fb9bf12c4f6e6aefb37bc3b6 to 678f4cd2b66a30481f1160af45a70fb8b36350ac
Branch pushed to git repo; I updated commit sha1. New commits:
678f4cd  src/doc/en/developer/packaging.rst: Update doc

comment:14 Changed 2 years ago by
 Commit changed from 678f4cd2b66a30481f1160af45a70fb8b36350ac to b80d88f851cc3ab3119ac5da25d4c5636e415ba8
comment:15 Changed 2 years ago by
 Commit changed from b80d88f851cc3ab3119ac5da25d4c5636e415ba8 to 4793466dba3f397ee0e4d60084cc8e7e2b5fed67
Branch pushed to git repo; I updated commit sha1. New commits:
4793466  build/pkgs/sage_conf: Change type to standard

comment:16 Changed 2 years ago by
Added documentation describing normal, pip, and script packages.
comment:17 Changed 2 years ago by
 Commit changed from 4793466dba3f397ee0e4d60084cc8e7e2b5fed67 to a6cea8e03928741fc0830e83dd85430d313ed2f9
Branch pushed to git repo; I updated commit sha1. New commits:
72e7719  m4/sage_spkg_collect.m4: Remove handling of type=script

dcaaa85  m4/sage_spkg_collect.m4: Remove specialcasing of unversioned (script/pip) packages

52902eb  m4/sage_spkg_collect.m4: For unversioned packages, set version to 'none' instead of name of spkg

ff871b7  build/make/Makefile.in: Version and record script packages like normal packages

aa8cdc5  Update doc regarding versioning/recording of script packages

a6cea8e  build/make/Makefile.in: Mark the targets SPKG and SPKGclean as phony

comment:18 Changed 2 years ago by
 Commit changed from a6cea8e03928741fc0830e83dd85430d313ed2f9 to d6005a301cd99a4e549a84fe59ecbd6dfddb08d7
Branch pushed to git repo; I updated commit sha1. New commits:
d6005a3  src/bin/sagelistpackages: Remove use of type=pip

comment:19 Changed 2 years ago by
 Commit changed from d6005a301cd99a4e549a84fe59ecbd6dfddb08d7 to d4b4d81f47f538acf6eef1468634502a7bb00b98
Branch pushed to git repo; I updated commit sha1. New commits:
d4b4d81  sage.misc.package.list_packages: For unversioned packages, use 'none' as version

comment:20 Changed 2 years ago by
 Commit changed from d4b4d81f47f538acf6eef1468634502a7bb00b98 to cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae
comment:21 Changed 2 years ago by
 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 followup: ↓ 25 Changed 2 years ago by
Why do you had all these .in
? Such a change should be agreed on sagedevel.
comment:23 Changed 2 years ago by
Unless I misunderstood something, in the section Install scripts of script packages
, ``spkginstall``
should be ``spkginstall.in``
(twice).
comment:24 Changed 2 years ago by
Changing to .in happened on #29096.
No, script packages don't to templating.
comment:25 in reply to: ↑ 22 Changed 2 years ago by
Replying to vdelecroix:
Why do you had all these
.in
? Such a change should be agreed on sagedevel.
we had an argument about it, and that change should have been done when the actual conversion of most of sageinstall scripts into templates happened, a couple of years back. So this change merely corrects an old fault of the naming scheme.
comment:26 Changed 2 years ago by
 Commit changed from cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae to dbb9860cb41006fbe4108447f4f01e26ed8f0bdd
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
ad9e524  m4/sage_spkg_collect.m4: Remove handling of type=script

6c977be  m4/sage_spkg_collect.m4: Remove specialcasing of unversioned (script/pip) packages

d4e8cd9  m4/sage_spkg_collect.m4: For unversioned packages, set version to 'none' instead of name of spkg

8370225  build/make/Makefile.in: Version and record script packages like normal packages

495d499  Update doc regarding versioning/recording of script packages

bc4ed34  build/make/Makefile.in: Mark the targets SPKG and SPKGclean as phony

d1fd7a7  src/bin/sagelistpackages: Remove use of type=pip

75175db  sage.misc.package.list_packages: For unversioned packages, use 'none' as version

d3d78f4  m4/sage_spkg_collect.m4: Fix up collection of dependencies

dbb9860  build/pkgs/texlive/spkginstall: Fix up path

comment:27 Changed 2 years ago by
 Dependencies #29096 deleted
Rebased on 9.1.beta8  so the diff is easier to read now. Needs review
comment:28 followup: ↓ 37 Changed 2 years ago by
I get a doctest failure:
sage t long warnlong 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/sage9.1.beta8 ********************************************************************** 1 item had failures: 1 of 5 in sage.env [45 tests, 1 failure, 1.41 s]  sage t long warnlong 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 2 years ago by
I don't know m4 syntax. Is the indentation remaining after you delete if test "$SPKG_NAME" != "$SPKG_VERSION"; then
meaningful?
comment:30 Changed 2 years ago by
No, it is not meaningful, I was just keeping the diff small.
comment:31 Changed 2 years ago by
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: 87 87  is obtained directly from https://pypi.org/; 88 88 89 89  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/#requirementsfileformat); 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/#requirementsfiles); 92 93 93 94  Sage installs the package using the ``pip`` package manager; 94 95
Note: different link, in addition to the changed text.
comment:32 Changed 2 years ago by
Excellent, please push it to the ticket
comment:33 Changed 2 years ago by
 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 2 years ago by
 Commit changed from dbb9860cb41006fbe4108447f4f01e26ed8f0bdd to 8b1d5ca6074561315be14ea4f072149b88fe2fd0
Not necessarily for this ticket, but should pyopenssl
be a pip package?
New commits:
8b1d5ca  trac 29287: rewording documentation

comment:35 Changed 2 years ago by
Good idea. I'll make this change.
comment:36 Changed 2 years ago by
 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 2 years ago by
 Commit changed from 8b1d5ca6074561315be14ea4f072149b88fe2fd0 to 057a66bdd05ad75539f2bdbd27561af3f0d57d65
Replying to jhpalmieri:
I get a doctest failure:
sage t long warnlong 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/sage9.1.beta8
Ha, this doctest is funny. I'll fix it.
New commits:
057a66b  Make pyopenssl a pip package

comment:38 Changed 2 years ago by
 Commit changed from 057a66bdd05ad75539f2bdbd27561af3f0d57d65 to 8f6323ed8af19d0c1301c65e41d3b219a8fa7a43
Branch pushed to git repo; I updated commit sha1. New commits:
8f6323e  src/sage/env.py: Fix up doctest on starting sage without SAGE_* variables

comment:39 Changed 2 years ago by
 Description modified (diff)
comment:40 Changed 2 years ago by
Can you explain the doctest failure? For some reason, there was no output before, but now there is output?
comment:41 Changed 2 years ago by
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 sageenv.
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 followup: ↓ 43 Changed 2 years ago by
Why do we need to give a longer path in the spkginstall
file for texlive
?
comment:43 in reply to: ↑ 42 Changed 2 years ago by
Replying to jhpalmieri:
Why do we need to give a longer path in the
spkginstall
file fortexlive
?
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 2 years ago by
 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 2 years ago by
 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:
0919086  trac 29287: doc fixes

comment:46 Changed 2 years ago by
 Status changed from needs_review to positive_review
Great, thanks very much.
comment:47 Changed 2 years ago by
 Priority changed from major to critical
comment:48 Changed 2 years ago by
 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
New commits:
build/pkgs: Change all 'type=pip' packages to 'type=optional' and add requirements.txt
build/pkgs: Change all 'type=script' packages to 'type=optional'
m4/sage_spkg_collect.m4: Make SPKG_SOURCE={normalpipscript} orthogonal to SPKG_TYPE={base,standard,optional,experimental
}