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

Various python 3 issues #18508

Closed
ohanar opened this issue May 26, 2015 · 36 comments
Closed

Various python 3 issues #18508

ohanar opened this issue May 26, 2015 · 36 comments

Comments

@ohanar
Copy link
Member

ohanar commented May 26, 2015

There are a number of really small issues when trying to build with python 3:

  1. sagenb is a marked as dependency of jmol, even though this is no longer the case
  2. sagenb is marked as a runtime dependency of sage, again this is no longer the case
  3. scons and sagenb don't work with python 3, so they should only be a standard package for python 2
  4. atlas needs a small change to make its build script work with python 3

CC: @vbraun @nathanncohen

Component: build

Author: R. Andrew Ohana

Branch/Commit: 83e849e

Reviewer: Jeroen Demeyer

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

@ohanar ohanar added this to the sage-6.8 milestone May 26, 2015
@ohanar
Copy link
Member Author

ohanar commented May 26, 2015

Dependencies: #17607

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:3

The notebook() command uses sagenb, so its a runtime dependency. See also #18512

I'm fine with EOL'ing SageNB with Python2, but you should probably post to sage-devel to see if anybody is interested in maintaining it.

@jdemeyer
Copy link

comment:4

Replying to @vbraun:

The notebook() command uses sagenb, so its a runtime dependency.

No, it's not a run-time dependency, since Sage works without the Sage notebook. Of course, the notebook() command doesn't work, but that doesn't matter.

@jdemeyer
Copy link

comment:5

Unless you add support for the python2standard package type, this branch isn't going to work.

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:6

There are other packages that could be removed yet Sage would still starts up; of course some functionality is then missing. I don't see how sagenb is different. Define "works". If you mean "works enough to create the conway polynomial pickles" then I guess so.

The python2standard type is from #17607

@jdemeyer
Copy link

comment:7

Replying to @vbraun:

Define "works".

This is actually documented in build/deps:

# Everything needed to start up Sage using "./sage".  Of course, not
# every part of Sage will work.  It does not include Maxima for example.

@jdemeyer
Copy link

comment:8

I guess a formal definition of "works" is that

./sage -c ""

has exit status 0.

@jdemeyer
Copy link

comment:10

I don't like this from #17607:

    if [ "$1" = all ]; then
        PKG_VAR=PYTHON
        if [ "$SAGE_PYTHON3" = yes ]; then
            PKG_NAME=python3
        else
            PKG_NAME=python2
        fi
        PKG_VERSION=$(newest_version $PKG_NAME)
        echo "$PKG_NAME $PKG_VERSION $PKG_VAR"
    fi

Just define PYTHON as alias of PYTHON2 or PYTHON3, analogous how build/install deals with GMP/MPIR and the toolchain. See the SAGE_MP_LIBRARY and TOOLCHAIN variables.

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:11

We will have to remove python2 in the not so distant future so I don't see the value in winning any beauty contests in the meantime.

@jdemeyer
Copy link

comment:12

Replying to @vbraun:

We will have to remove python2 in the not so distant future

I think that future might be more distant than you think... but that is besides the point anyway.

I don't see the value in winning any beauty contests in the meantime.

This isn't just about beauty. It's about writing code which is easy to understand by doing things in a consistent way.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 26, 2015

comment:13

This isn't just about beauty. It's about writing code which is easy to understand by doing things in a consistent way.

+1

Plus we might also want to work on the build/ files to make them more readable, and that task is not made simpler if we have to clean "workarounds" on the way.

Nathann

P.S.: this does not exactly give the impression of a 'clean job'.

(GRRR|…)~/.Sage/build/pkgs$ head python3/checksums.ini -n 1
tarball=Python-VERSION.tar.gz
(GRRR|…)~/.Sage/build/pkgs$ head python2/checksums.ini -n 1
tarball=python-VERSION.tar.gz

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:14

Replying to @nathanncohen:

Plus we might also want to work on the build/ files to make them more readable

Like strip out the Python 2.x stuff in a little bit over 4 years?

@jdemeyer
Copy link

comment:15

Replying to @vbraun:

The python2standard type is from #17607

...and I will remove it in #18517 since it's a very ugly hack to add to the build system.

@jdemeyer
Copy link

Changed dependencies from #17607 to #17607, #18517

@jdemeyer
Copy link

comment:16

What exactly are you trying to achieve with making scons build on Python 2 only? How do you solve the problem that c_lib still requires scons to build?

@jdemeyer
Copy link

comment:17

For sagenb, you could instead edit build/pkgs/sagenb/spkg-install to skip the package on Python 3.

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:18

scons doesn't support Python 3 at this point.

@jdemeyer
Copy link

comment:19

Replying to @vbraun:

scons doesn't support Python 3 at this point.

I know, but read my question: How do you solve the problem that c_lib still requires scons to build? In other words: what's the gain from removing SCONS as standard package if it's still a dependency of csage?

@vbraun
Copy link
Member

vbraun commented May 26, 2015

comment:20

I don't claim to have solved that, but we might want to get rid of scons. I think Andrew did some work towards autotoolizing polybori. We might want to do the same for c_lib, though its not the only option.

@jdemeyer
Copy link

comment:21

Replying to @vbraun:

I don't claim to have solved that, but we might want to get rid of scons.

If it helps you, I propose to make scons an optional package. It will still be built by default since it's needed as dependency of polybori and c_lib.

We might want to do the same for c_lib, though its not the only option.

I assume you are aware of #17854?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2015

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

1628a11fix scons and sagenb to work with #18517

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2015

Changed commit from 9d2af12 to 1628a11

@kcrisman
Copy link
Member

comment:25

Related: sagemath/sagenb#343

@jdemeyer
Copy link

comment:26

Replying to @ohanar:

I've been basically assuming that #17854 will be done in the not too distant future

It's only ntl_wrap which remains (#18367). Moving ntl_wrap to the Sage library is relatively easy I think but a lot of boring work.

@jdemeyer
Copy link

comment:27
The content of "/usr/local/src/sage-config/build/pkgs/sagenb/type" must be 'base', 'standard', 'optional' or 'experimental'

(just set back the type to standard, see #18517)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2015

Changed commit from 1628a11 to 6fe3896

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2015

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

6fe3896make sagenb a standard package again

@jdemeyer
Copy link

comment:30

This is probably good, but I'd rather wait for Sage 6.8.beta1 to formally review this.

@jdemeyer
Copy link

Changed branch from u/ohanar/python3smallfixes to u/jdemeyer/python3smallfixes

@jdemeyer
Copy link

Changed commit from 6fe3896 to 83e849e

@jdemeyer
Copy link

New commits:

83e849eMerge tag '6.8.beta2' into t/18508/python3smallfixes

@jdemeyer
Copy link

Changed dependencies from #17607, #18517 to none

@vbraun
Copy link
Member

vbraun commented May 30, 2015

comment:34

Reviewer name is missing

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Jun 2, 2015

Changed branch from u/jdemeyer/python3smallfixes to 83e849e

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

4 participants