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

Install Jupyter kernel spec and nbextensions in $SAGE_LOCAL #19371

Closed
jdemeyer opened this issue Oct 8, 2015 · 39 comments
Closed

Install Jupyter kernel spec and nbextensions in $SAGE_LOCAL #19371

jdemeyer opened this issue Oct 8, 2015 · 39 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Oct 8, 2015

Instead of installing the Jupyter files in the user's home directory, we should install it in $SAGE_LOCAL (a "system" install). This has two main advantages:

  1. it avoids conflicts in case multiple installations of Sage exist.
  2. the Sage Jupyter notebook can be used more easily outside of Sage (e.g. using Jupyterhub)

We should not use versioned identifiers. For example, all these actually refer to the same Sage installation:

$ ls -l ~/.local/share/jupyter/kernels/
total 12
drwx------ 2 jdemeyer jdemeyer 4096 Sep 24 18:05 sage_6_9_beta7
drwx------ 2 jdemeyer jdemeyer 4096 Oct  7 22:41 sage_6_9_rc0
drwx------ 2 jdemeyer jdemeyer 4096 Oct  7 22:04 sage_6_9_rc3

I'm also replacing IPython -> Jupyter and Sage -> SageMath

Depends on #19373

Component: interfaces

Author: Jeroen Demeyer

Branch/Commit: 4f06d3d

Reviewer: Emmanuel Charpentier

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

@jdemeyer jdemeyer added this to the sage-6.9 milestone Oct 8, 2015
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 8, 2015

@jdemeyer
Copy link
Author

jdemeyer commented Oct 8, 2015

New commits:

7c916efInstall Jupyter kernel spec in $SAGE_LOCAL

@jdemeyer
Copy link
Author

jdemeyer commented Oct 8, 2015

Commit: 7c916ef

@egourgoulhon
Copy link
Member

comment:4

Looks good to me.
But it would be nice if someone more expert on Jupyter gives a look too.

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@kiwifb
Copy link
Member

kiwifb commented Oct 8, 2015

comment:6

You are concerned with multiple SageMath installs I am more concerned with global install for multiple users (SMC and sage-on-gentoo) and I am not sure about that change. It gives me problem on install at the moment because it violates sandbox rules of packaging. It's more the fault of jupyter_core for not giving me a way of over-ridding ENV_JUPYTER_PATH than yours.

But I am more worried about the fact that on a global install people won't be able to touch that directory. If you don't need to put anything else in it at "user runtime" great. If not we are in a big no situation as far as I am concerned (and William will almost certainly not be happy either).

As for running multiple SageMath install side by side without bad interactions from the content of ~/.sage you can always define DOTSAGE. A bit inconvenient (unless you version it inside of sage) but it works.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 9, 2015

comment:7

Replying to @kiwifb:

It gives me problem on install at the moment because it violates sandbox rules of packaging.

I think you need to explain this more. The only possible issue I can see is that maybe the directory ENV_JUPYTER_PATH[0] is the "wrong" place in your Sage-on-Gentoo setup.

If ENV_JUPYTER_PATH[0] is some directory inside $SAGE_LOCAL (which it should be), I don't see why installing this one particular file in $SAGE_LOCAL is any different from installing the huge number of other files in $SAGE_LOCAL...

But I am more worried about the fact that on a global install people won't be able to touch that directory. If you don't need to put anything else in it at "user runtime" great.

We do not need to put anything else in that directory at runtime. Why do you think we do?

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2015

comment:8

Replying to @jdemeyer:

Replying to @kiwifb:

It gives me problem on install at the moment because it violates sandbox rules of packaging.

I think you need to explain this more. The only possible issue I can see is that maybe the directory ENV_JUPYTER_PATH[0] is the "wrong" place in your Sage-on-Gentoo setup.

ENV_JUPYTER_PATH is defined as os.path.join(sys.prefix, 'share', 'jupyter') which translates as /usr/share/jupyter (or $SAGE_LOCAL/share/jupyter) in concrete terms. The problem is the sage installation procedure is now trying to create and write files out of the sandbox.

Proper packaging in the sense of rpm, deb and most other doesn't allow you to write in the "live" system. Things are installed and written in a sandbox space and then copied (merged) to the system. Because of the way Andrew wrote env.py (and I was grateful to him for that) I can set most SAGE_* variables so sage will write things in my sandbox. If jupyter_core was allowing me to over-ride ENV_JUPYTER_PATH I could set in the appropriate place in the sandbox and it would be merged in the right place.

If ENV_JUPYTER_PATH[0] is some directory inside $SAGE_LOCAL (which it should be), I don't see why installing this one particular file in $SAGE_LOCAL is any different from installing the huge number of other files in $SAGE_LOCAL...

sys.prefix is SAGE_LOCAL, actually even in sage-on-gentoo, the difference is that you don't protect your live prefix during build.

But I am more worried about the fact that on a global install people won't be able to touch that directory. If you don't need to put anything else in it at "user runtime" great.

We do not need to put anything else in that directory at runtime. Why do you think we do?

Because I am uncertain of how it works. You are obviously creating something there as place holder but it was possibly silly of me.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 9, 2015

comment:9

Replying to @kiwifb:

sys.prefix is SAGE_LOCAL, actually even in sage-on-gentoo

So, if sys.prefix is $SAGE_LOCAL and you can set $SAGE_LOCAL to whatever you want, then I don't see the problem.

the difference is that you don't protect your live prefix during build.

I have absolutely no idea what this sentence means.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 9, 2015

comment:10

Replying to @kiwifb:

Because I am uncertain of how it works.

It's really just one configuration file. That's the whole thing I don't understand: what makes this one particular file so special? You apparently manage to install all of Sage, but not this one particular file?

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2015

comment:11

Two things:

  • the fact that sys.prefix is SAGE_LOCAL is incidental, it doesn't have to be.
  • Once jupyter_core is in place it doesn't matter what I set SAGE_LOCAL to, it doesn't change the value of sys.prefix jupyter_core or rather python is reporting.

The install procedure for sage is now trying to create /usr/share/jupyter/kernels/sagemath and any missing directory. rpm, deb and ebuild do not allow you to do that directly. You install $SOMESANDBOX/usr/share/jupyter/kernels/sagemath and later you merge into the live system. That's what DESTDIR is for in autotools, you set DESTDIR=$SOMESANDBOX and things magically install in the sandbox.

Now my main concern was the potential for user to do something to {/usr,$SAGE_LOCAL}/share/jupyter/kernels/sagemath which would be problematic for multi-user install. It is unfounded. The rest is an upstream problem and we can stop debating packaging tech in this ticket.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 9, 2015

comment:12

Replying to @kiwifb:

You install $SOMESANDBOX/usr/share/jupyter/kernels/sagemath and later you merge into the live system. That's what DESTDIR is for in autotools, you set DESTDIR=$SOMESANDBOX and things magically install in the sandbox.

Right, and how does this work with distutils? I assume (but I might be wrong) that distutils also uses sys.prefix to determine where to install stuff.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 9, 2015

comment:13

Replying to @kiwifb:

The rest is an upstream problem and we can stop debating packaging tech in this ticket.

It happens that I am debating packaging tech with upstream, so I'd like to understand your issue.

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2015

comment:14

Replying to @jdemeyer:

Replying to @kiwifb:

The rest is an upstream problem and we can stop debating packaging tech in this ticket.

It happens that I am debating packaging tech with upstream, so I'd like to understand your issue.

I have just read it ( before seeing your comment here). It is a bit messy but thinking more about it there must a bug of some kind. distutils as used from within portage should set a sandboxed prefix, therefore sys.prefix should point to a sandboxed location, so something is wrong there.

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2015

comment:15

I know what the problem is. We have three phases in order: build, test (optional) and install. Only the install phase is run with a prefix, this is the only time when it should be necessary. But the installation of the jupyter kernel is attempted during the build phase. So from my point of view that bit has to be restricted to install time.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2015

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

2cc3a74Fix dependencies; install kernel spec with custom install class

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2015

Changed commit from 7c916ef to 2cc3a74

@jdemeyer jdemeyer modified the milestones: sage-6.9, sage-6.10 Oct 9, 2015
@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2015

comment:21

Thank you for taking my concern on board.

@jdemeyer
Copy link
Author

Dependencies: #19371

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2015

Changed commit from 2cc3a74 to 6c7261c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2015

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

52a717aUse relative links to local help
6c7261cInstall Jupyter kernel spec in $SAGE_LOCAL

@jdemeyer
Copy link
Author

Changed dependencies from #19371 to #19373

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Install Jupyter kernel spec in $SAGE_LOCAL Install Jupyter kernel spec and nbextensions in $SAGE_LOCAL Oct 11, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2015

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

4f06d3dInstall Jupyter kernel and nbextensions in $SAGE_LOCAL

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2015

Changed commit from 6c7261c to 4f06d3d

@jdemeyer

This comment has been minimized.

@egourgoulhon
Copy link
Member

Changed reviewer from Eric Gourgoulhon to none

@egourgoulhon
Copy link
Member

comment:28

Sorry I'm afraid I cannot review this one (don't feel competent about it).

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 15, 2015

comment:29

I had a problem on an Ubuntu machine wher I could get MathJax in sheets created in my home directory but nowhere else.

I checked the present patch and recompiled :

  1. It seems to solve my Ubuntu-specific problem
  2. It passes ptestlong.

Is that enough for a positive review ?

@jdemeyer
Copy link
Author

comment:30

Replying to @EmmanuelCharpentier:

Is that enough for a positive review ?

Who are you asking? Essentially, that's up to you to decide.

@jdemeyer
Copy link
Author

comment:31

Note that this depends on #19373. So, it would be nice if you could also review that.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 15, 2015

comment:32

Replying to @jdemeyer:

Note that this depends on #19373. So, it would be nice if you could also review that.

Is this dependance somehow managed by git ?

In other terms, when I checked #19371 out, did that also applied #19373 ?

If so, that means that my tests show that (#19371+#19373) passes ptestlong. I cannot speak for a Jupyter hub, since I don't have such a beast in my zoo...

Otherwise, I'll have to check out #19373 and merge both branches, then recompile and ptestlong (about 4 hours on my Ubuntu netbook...).

Or would it be enough to positively review #19371, and leave #19373 to someone equipped to do so ?

@jdemeyer
Copy link
Author

comment:33

Replying to @EmmanuelCharpentier:

In other terms, when I checked #19371 out, did that also applied #19373 ?

Yes, this branch here is based on #19373.

If so, that means that my tests show that (#19371+#19373) passes ptestlong.

Indeed.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 15, 2015

Reviewer: Emmanuel Charpentier

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 15, 2015

comment:34

Replying to @jdemeyer:

Replying to @EmmanuelCharpentier:

In other terms, when I checked #19371 out, did that also applied #19373 ?

Yes, this branch here is based on #19373.

If so, that means that my tests show that (#19371+#19373) passes ptestlong.

Indeed.

OK. I give a positive review to #19371. I'll leave to someone having the possibility to really test #19373 the responsibility of validating it.

@vbraun
Copy link
Member

vbraun commented Oct 18, 2015

Changed branch from u/jdemeyer/install_jupyter_kernel_spec_in__sage_local to 4f06d3d

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