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

Deprecate deprecated sage-cython script harder, remove use of it in the main sage script #29923

Closed
jhpalmieri opened this issue Jun 20, 2020 · 14 comments

Comments

@jhpalmieri
Copy link
Member

The script sage-cython has been deprecated for about 18 months, so let's remove its use within sage.

Depends on #29111

Component: scripts

Author: John Palmieri

Branch/Commit: 648e327

Reviewer: Matthias Koeppe

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

@jhpalmieri jhpalmieri added this to the sage-9.2 milestone Jun 20, 2020
@jhpalmieri
Copy link
Member Author

Branch: u/jhpalmieri/remove-sage-cython

@jhpalmieri
Copy link
Member Author

Last 10 new commits:

55c5680src/bin/sage: Move parts of the help message to sage-site
885092esrc/doc/bootstrap: Generate src/doc/en/reference/repl/options.txt
05ba86aMerge branch 't/29884/src_doc_bootstrap__generate_src_doc_en_reference_repl_options_rst' into t/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution
72c7e23src/doc/en/reference/repl/options.rst: Replace copypasta by include of generated file options.txt
e9a883dtrac 29111: revising "sage --advanced" message.
5dca421trac 29111: more reorganization of "sage --advanced" message
8f0ee05trac 29111: delete sage-fix-pkg-checksums
21fc231trac 29111: re "sage --advanced" message:
875940dtrac 29111: more tinkering. Fix doctests in tests/cmdline.py.
8c9b79btrac 29923: remove the deprecated sage-cython script

@jhpalmieri
Copy link
Member Author

Commit: 8c9b79b

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 21, 2020

comment:3

This script has a good chance of being used in user package install scripts. So I would suggest to do the removal a little later...

@jhpalmieri
Copy link
Member Author

comment:4

What about this part, at least:

diff --git a/build/bin/sage-site b/build/bin/sage-site
index 6daa094..8de96ce 100755
--- a/build/bin/sage-site
+++ b/build/bin/sage-site
@@ -200,7 +200,7 @@ fi
 
 if [ "$1" = "-cython" -o "$1" = '--cython' -o "$1" = '-pyrex' -o "$1" = "--pyrex" ]; then
     shift
-    exec sage-cython "$@"
+    exec cython "$@"
 fi
 
 if [ "$1" = '-gap' -o "$1" = '--gap' ]; then

plus maybe a deprecation warning whenever sage-cython is called?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 21, 2020

comment:5

Sure, sounds good

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 21, 2020

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

4a3d36eMove 'sage -app' back to src/bin/sage
0f51881trac 29923: remove use of sage-cython in main sage script,

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 21, 2020

Changed commit from 8c9b79b to 0f51881

@jhpalmieri
Copy link
Member Author

comment:7

Okay, done.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 21, 2020

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

648e327trac 29923: remove use of sage-cython in main sage script,

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 21, 2020

Changed commit from 0f51881 to 648e327

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 21, 2020

Reviewer: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Remove deprecated sage-cython script Deprecate deprecated sage-cython script harder, remove use of it in the main sage script Jun 21, 2020
@vbraun
Copy link
Member

vbraun commented Jul 10, 2020

Changed branch from u/jhpalmieri/remove-sage-cython to 648e327

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

3 participants