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: | 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: |
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 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 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={normal|pip|script} orthogonal to SPKG_TYPE={base,standard,optional,experimental }
|
ff4db4b | sage.misc.package.list_packages: New argument pkg_sources, orthogonal to pkg_types
|
041c76d | Rename spkg-build, spkg-install etc. templates as spkg-build.in, spkg-install.in etc.
|
4524b04 | Update developer manual
|
2724083 | Undo rename of build/pkgs/*/spkg-install.py
|
ec540dd | Fix up sage-spkg
|
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 special-casing 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 SPKG-clean 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/sage-list-packages: 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 follow-up: ↓ 25 Changed 2 years ago by
Why do you had all these .in
? Such a change should be agreed on sage-devel.
comment:23 Changed 2 years ago by
Unless I misunderstood something, in the section Install scripts of script packages
, ``spkg-install``
should be ``spkg-install.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 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 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 special-casing 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 SPKG-clean as phony
|
d1fd7a7 | src/bin/sage-list-packages: 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/spkg-install: 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 follow-up: ↓ 37 Changed 2 years ago by
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 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/#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); 92 93 93 94 - Sage installs the package using the ``pip`` package manager; 94 95
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 --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:
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 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: ↓ 43 Changed 2 years ago by
Why do we need to give a longer path in the spkg-install
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
spkg-install
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={normal|pip|script} orthogonal to SPKG_TYPE={base,standard,optional,experimental
}