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

Let mathjax spkg work with sagenb #17306

Closed
sagetrac-tmonteil mannequin opened this issue Nov 7, 2014 · 25 comments
Closed

Let mathjax spkg work with sagenb #17306

sagetrac-tmonteil mannequin opened this issue Nov 7, 2014 · 25 comments

Comments

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Nov 7, 2014

The aim of this ticket is to put mathjax (see #17288) as a dependency of sagenb spkg and let them work together.

Depends on #17288

CC: @jhpalmieri @kcrisman

Component: notebook

Keywords: mathjax notebook

Author: Thierry Monteil

Branch/Commit: 9c20f45

Reviewer: Volker Braun

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

@sagetrac-tmonteil sagetrac-tmonteil mannequin added this to the sage-6.4 milestone Nov 7, 2014
@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Nov 7, 2014

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Nov 7, 2014

comment:2

I put a "need review" because the commit is ready, but there is no hurry if you need to discuss on the sagenb side.


New commits:

e8ef55d#17288 : mathjax spkg
8f18d36#17288 strip mathjax down from 175M to <10M.
0443d99#17288 make mathjax standard and a dependency of ipython
8ec7d5c#17288 make mathjax a dependency of sagenb

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Nov 7, 2014

Commit: 8ec7d5c

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Nov 7, 2014

Author: Thierry Monteil

@novoselt
Copy link
Member

novoselt commented Nov 7, 2014

comment:3

Since this change works even without modifying the sagenb, I don't see any point in delaying the switch. For the users the difference should be using a more up-to-date version of MathJax.

@kcrisman
Copy link
Member

kcrisman commented Nov 8, 2014

comment:4

Well, it's kind of a hack removing things from the egg, isn't it? Anyway, I won't be trying it in the near future, but if you have time to test it and there aren't any subtle things, obviously in the long run this is a very good idea.

@kcrisman
Copy link
Member

kcrisman commented Dec 7, 2014

comment:5

Shouldn't the sagenb egg thing use the version number of the sagenb package (this should be available as some kind of env var in spkg-install)?

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Dec 8, 2014

comment:6

Replying to @kcrisman:

Well, it's kind of a hack removing things from the egg, isn't it?

If i understand the motivation in having this ticket separate from #17288, sagenb development has not much energy. So, if sagenb developpers agree with this ticket, then it can be accepted without the need to release a new version of sagenb (and the removing part of spkg-install can be removed later once mathjax is removed from sagenb tarball during a following release). I just wanted to avoid this ticket to impose some additional work on the sagenb side.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2014

Changed commit from 8ec7d5c to 7c574c7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2014

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

1e5305b#17306 removal of sagenb's mathjax repository can be removed once sagenb does not ship mathjax anymore.
7c574c7#17306 use the version number of sagenb and python packages in egg path

@kcrisman
Copy link
Member

kcrisman commented Dec 8, 2014

comment:8

Okay, in that case though I am not sure I will be able to review this - Andrey, I'm happy if you test this, though, I just don't want to mix things up too badly.

Incidentally, I'm surprised you have to go to those lengths to find the version number, at least for sagenb; shouldn't there be a variable defining the current pkg version available inside the current pkg install process? It surprises me if that's not the case. But I admit I don't have independent proof of this.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Dec 8, 2014

comment:9

I just read the sage-spkg script to see if i can steal something from there. There are indeed some PKG_VER, LOCAL_PKG_VER and PKG_BASE_VER variables that could be good candidates. However, those variables are not exported and the spkg-install script is called (not sourced) so the spkg-install will not inherit those variables.

By the way, LOCAL_PKG_VER is also built by looking at package-version.txt.

@kcrisman
Copy link
Member

kcrisman commented Dec 8, 2014

comment:10

Ah, interesting. Do you think that it should be sourced? As long as those variables then change (and after a build are vanquished) that could be useful.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Dec 8, 2014

comment:11

Replying to @kcrisman:

Ah, interesting. Do you think that it should be sourced? As long as those variables then change (and after a build are vanquished) that could be useful.

No, if we want some variable to be available in the spkg-install script i would use export USEFUL_VARIABLE before calling the spkg-install script (by the way, note that some spkg-install scripts are written in Python and can not be sourced within a bash script). I do not know how much those variables are needed though.

@kcrisman
Copy link
Member

kcrisman commented Feb 3, 2015

comment:12

Waiting on #17288 at this time before bothering to review.

@kcrisman kcrisman removed this from the sage-6.4 milestone Feb 3, 2015
@vbraun
Copy link
Member

vbraun commented Mar 9, 2015

comment:13

Mathjax is just a runtime dependency but doesn't block installing SageNB, right?. You can also make symlinks whose destination is not yet installed. So no need to list it in deps.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 13, 2015

Changed commit from 7c574c7 to 10a8828

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 13, 2015

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

10a8828#17306 remove mathjax as a build dependency.

@vbraun
Copy link
Member

vbraun commented Mar 13, 2015

comment:16

lgtm

@vbraun
Copy link
Member

vbraun commented Mar 13, 2015

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Mar 14, 2015

comment:17

Merge conflict with sage-6.6.beta5 (ipython 3.0 update)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2015

Changed commit from 10a8828 to 9c20f45

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2015

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

9770edf#17306 : move ipython dependencies on a single line.
9c20f45#17306 : merge with develop.6.6.beta5.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Mar 14, 2015

comment:20

I hope the merge is correct.

@vbraun
Copy link
Member

vbraun commented Mar 17, 2015

Changed branch from u/tmonteil/let_mathjax_spkg_work_with_sagenb to 9c20f45

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