Opened 8 years ago

Closed 8 years ago

#17306 closed enhancement (fixed)

Let mathjax spkg work with sagenb

Reported by: Thierry Monteil Owned by:
Priority: major Milestone: sage-6.6
Component: notebook Keywords: mathjax notebook
Cc: John Palmieri, Karl-Dieter Crisman Merged in:
Authors: Thierry Monteil Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 9c20f45 (Commits, GitHub, GitLab) Commit: 9c20f45cadbfc291b26ac5a21a5c3a324d594a8b
Dependencies: #17288 Stopgaps:

Status badges

Description

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

Change History (22)

comment:1 Changed 8 years ago by Thierry Monteil

Branch: u/tmonteil/let_mathjax_spkg_work_with_sagenb

comment:2 Changed 8 years ago by Thierry Monteil

Authors: Thierry Monteil
Commit: 8ec7d5c3a4fbf47b551df2b288b74816eca74b7a
Status: newneeds_review

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

comment:3 Changed 8 years ago by Andrey Novoseltsev

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.

comment:4 Changed 8 years ago by Karl-Dieter Crisman

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.

comment:5 Changed 8 years ago by Karl-Dieter Crisman

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)?

comment:6 in reply to:  4 Changed 8 years ago by Thierry Monteil

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.

comment:7 Changed 8 years ago by git

Commit: 8ec7d5c3a4fbf47b551df2b288b74816eca74b7a7c574c7f4161443c317b3805ba2325f1bfff4623

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

comment:8 Changed 8 years ago by Karl-Dieter Crisman

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.

comment:9 Changed 8 years ago by Thierry Monteil

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.

comment:10 Changed 8 years ago by Karl-Dieter Crisman

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.

comment:11 in reply to:  10 Changed 8 years ago by Thierry Monteil

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.

comment:12 Changed 8 years ago by Karl-Dieter Crisman

Milestone: sage-6.4sage-pending

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

comment:13 Changed 8 years ago by Volker Braun

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.

comment:14 Changed 8 years ago by Volker Braun

Milestone: sage-pendingsage-6.6
Status: needs_reviewneeds_work

comment:15 Changed 8 years ago by git

Commit: 7c574c7f4161443c317b3805ba2325f1bfff462310a88282babac28633778bab94f8aa2b5fdc3a28

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

10a8828#17306 remove mathjax as a build dependency.

comment:16 Changed 8 years ago by Volker Braun

Reviewers: Volker Braun
Status: needs_workpositive_review

lgtm

comment:17 Changed 8 years ago by Volker Braun

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

comment:18 Changed 8 years ago by Volker Braun

Status: positive_reviewneeds_work

comment:19 Changed 8 years ago by git

Commit: 10a88282babac28633778bab94f8aa2b5fdc3a289c20f45cadbfc291b26ac5a21a5c3a324d594a8b

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.

comment:20 Changed 8 years ago by Thierry Monteil

Status: needs_workneeds_review

I hope the merge is correct.

comment:21 Changed 8 years ago by Volker Braun

Status: needs_reviewpositive_review

comment:22 Changed 8 years ago by Volker Braun

Branch: u/tmonteil/let_mathjax_spkg_work_with_sagenb9c20f45cadbfc291b26ac5a21a5c3a324d594a8b
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.