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

Priority:  critical  Milestone:  sage9.1 
Component:  build  Keywords:  
Cc:  Erik Bray, Dima Pasechnik, John Palmieri, Thierry Monteil, Volker Braun  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 3 years ago by
Branch:  → u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ 

comment:2 Changed 3 years ago by
Authors:  → Matthias Koeppe 

Commit:  → 90054f65bfbba569d1d95d1de08931f2738ee35f 
comment:3 Changed 3 years ago by
Commit:  90054f65bfbba569d1d95d1de08931f2738ee35f → 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 3 years ago by
Cc:  John Palmieri added 

Status:  new → needs_review 
comment:7 Changed 3 years ago by
Dependencies:  → #21033 

Just putting the list in the description somewhere in the docs would help. What is a "script" package, anyway?
comment:9 Changed 3 years ago by
Dependencies:  #21033 → #29096 

comment:10 Changed 3 years ago by
Commit:  f7a661c00eb396629a445b3395056b531f1afad3 → 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 3 years ago by
Rebased on 9.1.beta7, merged #29096 to avoid conflicts in documentation
comment:12 Changed 3 years ago by
Description:  modified (diff) 

Summary:  SPKG type: Make "regular/script/pip" orthogonal to "standard/optional/experimental" → SPKG type: Make "normal/script/pip" orthogonal to "standard/optional/experimental" 
comment:13 Changed 3 years ago by
Commit:  f819b51bbccb7b13fb9bf12c4f6e6aefb37bc3b6 → 678f4cd2b66a30481f1160af45a70fb8b36350ac 

Branch pushed to git repo; I updated commit sha1. New commits:
678f4cd  src/doc/en/developer/packaging.rst: Update doc

comment:14 Changed 3 years ago by
Commit:  678f4cd2b66a30481f1160af45a70fb8b36350ac → b80d88f851cc3ab3119ac5da25d4c5636e415ba8 

comment:15 Changed 3 years ago by
Commit:  b80d88f851cc3ab3119ac5da25d4c5636e415ba8 → 4793466dba3f397ee0e4d60084cc8e7e2b5fed67 

Branch pushed to git repo; I updated commit sha1. New commits:
4793466  build/pkgs/sage_conf: Change type to standard

comment:17 Changed 3 years ago by
Commit:  4793466dba3f397ee0e4d60084cc8e7e2b5fed67 → 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 3 years ago by
Commit:  a6cea8e03928741fc0830e83dd85430d313ed2f9 → d6005a301cd99a4e549a84fe59ecbd6dfddb08d7 

Branch pushed to git repo; I updated commit sha1. New commits:
d6005a3  src/bin/sagelistpackages: Remove use of type=pip

comment:19 Changed 3 years ago by
Commit:  d6005a301cd99a4e549a84fe59ecbd6dfddb08d7 → 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 3 years ago by
Commit:  d4b4d81f47f538acf6eef1468634502a7bb00b98 → cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae 

comment:21 Changed 3 years ago by
Cc:  Thierry Monteil Volker Braun added 

Description:  modified (diff) 
Status:  needs_work → needs_review 
Summary:  SPKG type: Make "normal/script/pip" orthogonal to "standard/optional/experimental" → SPKG type: Make "normal/script/pip" orthogonal to "base/standard/optional/experimental" 
comment:22 followup: 25 Changed 3 years ago by
Why do you had all these .in
? Such a change should be agreed on sagedevel.
comment:23 Changed 3 years ago by
Unless I misunderstood something, in the section Install scripts of script packages
, ``spkginstall``
should be ``spkginstall.in``
(twice).
comment:24 Changed 3 years ago by
Changing to .in happened on #29096.
No, script packages don't to templating.
comment:25 Changed 3 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 3 years ago by
Commit:  cb3c9137ba9a6561ebd2d6cf8312e8f3401fe7ae → 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 3 years ago by
Dependencies:  #29096 

Rebased on 9.1.beta8  so the diff is easier to read now. Needs review
comment:28 followup: 37 Changed 3 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 3 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:31 Changed 3 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:33 Changed 3 years ago by
Branch:  u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ → u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ 

comment:34 Changed 3 years ago by
Commit:  dbb9860cb41006fbe4108447f4f01e26ed8f0bdd → 8b1d5ca6074561315be14ea4f072149b88fe2fd0 

Not necessarily for this ticket, but should pyopenssl
be a pip package?
New commits:
8b1d5ca  trac 29287: rewording documentation

comment:36 Changed 3 years ago by
Branch:  u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ → u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ 

comment:37 Changed 3 years ago by
Commit:  8b1d5ca6074561315be14ea4f072149b88fe2fd0 → 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 3 years ago by
Commit:  057a66bdd05ad75539f2bdbd27561af3f0d57d65 → 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 3 years ago by
Description:  modified (diff) 

comment:40 Changed 3 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 3 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 3 years ago by
Why do we need to give a longer path in the spkginstall
file for texlive
?
comment:43 Changed 3 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 3 years ago by
Branch:  u/mkoeppe/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ → u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ 

comment:45 Changed 3 years ago by
Commit:  8f6323ed8af19d0c1301c65e41d3b219a8fa7a43 → 091908665ae681b6f9cc1f491d4801acbdfb89ae 

Reviewers:  → John Palmieri 
A few small changes. If you're happy with these, positive review from me.
New commits:
0919086  trac 29287: doc fixes

comment:47 Changed 3 years ago by
Priority:  major → critical 

comment:48 Changed 3 years ago by
Branch:  u/jhpalmieri/spkg_type__make__regular_script_pip__orthogonal_to__standard_optional_experimental_ → 091908665ae681b6f9cc1f491d4801acbdfb89ae 

Resolution:  → fixed 
Status:  positive_review → 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
}