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

Move sage-dist-helpers from src/bin to build/bin #25130

Closed
mkoeppe opened this issue Apr 9, 2018 · 25 comments
Closed

Move sage-dist-helpers from src/bin to build/bin #25130

mkoeppe opened this issue Apr 9, 2018 · 25 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Apr 9, 2018

This seems to be a script for use within build/pkgs scripts, so it should go to build/bin.

CC: @embray @kiwifb @dimpase

Component: build

Author: Matthias Koeppe

Branch/Commit: abc9fa9

Reviewer: Erik Bray

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

@mkoeppe mkoeppe added this to the sage-8.2 milestone Apr 9, 2018
@embray
Copy link
Contributor

embray commented Apr 9, 2018

comment:1

I go back-and-forth on that. I agree with your logic completely. But I think we need to talk about where installation-related files go for use "at runtime". Because currently all build-related files live in build/ but don't get "installed" into $SAGE_LOCAL. And yet they are in a sense needed at "runtime" since a feature of Sage is the ability to install new packages--particularly optional packages.

So I think what we really need is overall better organization of what build-related files are needed at runtime and how to "install" those files in such a way that installing additional Sage packages doesn't necessarily depend on $SAGE_ROOT.

Under that rubrik I'm not sure the existence of build/bin even makes sense since most, if not all of those scripts are needed for optional package installation.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 9, 2018

comment:2

build is for sage-the-distribution, and therefore for everything related to installing packages.
src is for sagelib.

@embray
Copy link
Contributor

embray commented Apr 10, 2018

comment:3

I agree, and yet that ignores everything else I wrote...

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 10, 2018

comment:4

Replying to @embray:

I agree, and yet that ignores everything else I wrote...

Sorry, I had to catch a flight.

The longer answer: I disagree with the notion of installation of optional packages being done "at runtime". There is no ticket that describes your goal of being able to install tickets even if $SAGE_ROOT is not present. Right now installation of optional packages does not even reliably work in binary distributions.

@embray
Copy link
Contributor

embray commented Apr 10, 2018

comment:5

I think it should work--at least so long as a compiler is available. As it stands, in binary distributions the feature is not disabled, and it is built into the command-line sage interface.

Alternatively, I have also mentioned that I don't like it in the first place that build-related commands (e.g. sage -b and the like) are built into the same runtime interface that users use to start and run Sage and associated commands.

So I think there needs to be a clearer vision of what an arbitrary user should expect to be able to do from the sage command regardless of the context in which they're running it.

@embray
Copy link
Contributor

embray commented Apr 10, 2018

comment:6

Put more succinctly, if someone wants to move this file around because it's "build-related" I have no problem with that. My point is just that I see it as a distinction without a difference since Sage doesn't clearly separate build-time from run-time in all cases.

@jdemeyer
Copy link

comment:7

I agree with @mkoeppe here. Installing Sage packages should not considered to be run-time, but build-time.

@embray
Copy link
Contributor

embray commented Apr 11, 2018

comment:8

And yet sage -i <optional-package> is considered standard functionality of Sage that is expected to work for all users...?

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 11, 2018

comment:9

It wouldn't be expected, for example, in a Linux distribution's sage binary; sage's optional packages would probably be supplied by the distribution's package manager instead.

I see the "sage -i" as a mere convenience which "calls out to the SAGE_ROOT" to install a package.

@embray
Copy link
Contributor

embray commented Apr 11, 2018

comment:10

I'd be curious about the sage-on-gentoo approach to this. If that is indeed the case (and I'm not necessarily disagreeing) then we should still clearly delineate what features of the command-line sage interface should and should not work outside of a Sage source tree, and clearly disable if not remove those features in cases where Sage is "installed" in another filesystem hierarchy. This should also be made clear in the documentation both for users, and for developers interesting in packaging Sage for different platforms: That they need to think about what optional packages to include, if any, and if and whether users should have means to obtain optional packages. There's also a question of whether or not "pip" should work...

I'm thinking about this because currently installing optional packages is broken in Sage for Windows. Users really have no way to install them. If sage -i shouldn't work, then there has to be some mechanism whereby users can install packages. For Windows I have a few different ideas which I'd be fine with, but I would still want to be able to either explicitly disable sage -i, or have it print some message as to how users should actually install optional packages (or maybe it could install the package, but would have to go through an alternate system-specific backend to do so). For example in the case of Windows my preferred approach is to provide binary packages that are downloaded and unpacked.

@kiwifb
Copy link
Member

kiwifb commented Apr 11, 2018

comment:11

On sage-on-gentoo sage -i is not offered. I am pushing slowly but surely for is_package_installed to be removed at runtime and in the more distant future at build time. doctesting and is_package_installed are also causing me some headaches.

For information this is sage-on-gentoo misc/package.py file [ https://github.com/cschwan/sage-on-gentoo/blob/master/sci-mathematics/sage/files/sage-7.3-package.py].

And these are the commands that I ship if you request debugging and doctesting enabled

/usr/bin/math-readline
/usr/bin/sage
/usr/bin/sage-cachegrind
/usr/bin/sage-callgrind
/usr/bin/sage-cleaner
/usr/bin/sage-cython
/usr/bin/sage-eval
/usr/bin/sage-gdb-commands
/usr/bin/sage-ipython
/usr/bin/sage-massif
/usr/bin/sage-native-execute
/usr/bin/sage-notebook
/usr/bin/sage-num-threads.py
/usr/bin/sage-omega
/usr/bin/sage-preparse
/usr/bin/sage-python
/usr/bin/sage-rst2sws
/usr/bin/sage-rst2txt
/usr/bin/sage-run
/usr/bin/sage-run-cython
/usr/bin/sage-runtests
/usr/bin/sage-startuptime.py
/usr/bin/sage-sws2rst
/usr/bin/sage-valgrind
/usr/bin/sage-version.sh

As for dealing with optional packages. In sage-on-distro we of course expect them to be installed by the distro's package manager. Not all optional packages may be on offer and filling that gap is hard. Debian has a massive amount of packages so they have a better starting point than me on this.

Next there is the problem that there are two categories of optional packages:

  • runtime packages, the functionality will be available if the package is installed and it can be installed after sage. They are easy to deal with - you can just install them whenever, in a standard path, and they should work.
  • build time packages. In the current system if those packages are found when sage is built, specific binding will be build for those packages. Those are the hard ones. If you want to support them they have to be part of your dependency tree.

Gentoo has useflags, I offer some built time optional packages through useflags. If you want any of bliss, modular_decomposition, libbraiding or libhomfly they can be enabled. cbc is hard and I failed to enable it once.

Missing packages in that category are mcqd, tdlib, coxeter3, fes, sirocco, meataxe (not going to happen anytime soon, with all due respect, meataxe packaging sucks), gurobi (in its own category, it checks for presence differently from anyone else), cplex and cbc (the only extension to require lapack).

@embray
Copy link
Contributor

embray commented Apr 13, 2018

comment:12

Thanks for details. It's definitely clear to me we need to do some more rethinking of how optional packages are handled. Anyways, sorry to derail this ticket. Like I said in the first place I don't mind moving the current location of sage-dist-helpers. It just got me thinking about why I placed it where I did in the first place. There was certainly motivated reasoning behind it, though in practice it still didn't make sense...

@mkoeppe
Copy link
Member Author

mkoeppe commented May 18, 2019

@mkoeppe
Copy link
Member Author

mkoeppe commented May 18, 2019

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented May 18, 2019

New commits:

7405dd5Move sage-dist-helpers from src/bin to build/bin

@mkoeppe
Copy link
Member Author

mkoeppe commented May 18, 2019

Commit: 7405dd5

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-8.2, sage-8.8 May 18, 2019
@embray
Copy link
Contributor

embray commented May 23, 2019

comment:16

I'm concerned that this will be broken if $SAGE_ROOT contains spaces. It probably shouldn't and I'm not sure that there aren't other things that break if that's the case.

I think it would be fine to just put each path in double-quotes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 23, 2019

Changed commit from 7405dd5 to abc9fa9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 23, 2019

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

abc9fa9Add quotes

@mkoeppe
Copy link
Member Author

mkoeppe commented May 30, 2019

comment:18

Needs review.

@embray
Copy link
Contributor

embray commented Jul 1, 2019

comment:19

Thank you. Sorry this took so long.

@embray
Copy link
Contributor

embray commented Jul 1, 2019

Reviewer: Erik Bray

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 1, 2019

comment:20

Thank you!

@vbraun
Copy link
Member

vbraun commented Jul 4, 2019

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

5 participants