Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#26641 closed defect (fixed)

Repackage sagenb

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-8.5
Component: packages: standard Keywords:
Cc: fbissey, jhpalmieri, dimpase Merged in:
Authors: Jeroen Demeyer Reviewers: John Palmieri, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 8824d93 (Commits) Commit:
Dependencies: #26642 Stopgaps:

Description (last modified by jdemeyer)

Since a while, sagenb is not supposed to contain a bundled mathjax. In particular, the github repo of sagenb does not contain mathjax.

Mysteriously, the sagenb packaged in #26499 does contain mathjax, but it's not at all clear where this comes from.

So this ticket fixes the packaging procedure of SageNB and makes a new tarball from essentially the same sources.

Tarball: https://github.com/sagemath/sagenb/releases/download/1.1.1/sagenb-1.1.1.tar.bz2

Change History (25)

comment:1 follow-up: Changed 4 months ago by dimpase

Please fix this in sagenb repo.

comment:2 in reply to: ↑ 1 Changed 4 months ago by jdemeyer

Replying to dimpase:

Please fix this in sagenb repo.

The problem is not in the sagenb repo, it's in the packaging of the sagenb repo. So in order to make sure that this doesn't happen again, it would be good to know how you created the sagenb package on #26499.

I will release sagenb-1.1.1 from essentially the same sources as 1.1.0, packaged from a clean checkout of the sagenb repo, using the dist.sh script.

comment:3 follow-up: Changed 4 months ago by dimpase

Sagenb's GitHub repo contains a script to build the release. please fix it so that it doesn't do what you find wrong.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 4 months ago by jdemeyer

Replying to dimpase:

Sagenb's GitHub repo contains a script to build the release. please fix it so that it doesn't do what you find wrong.

That's hard to say since I don't know the exact steps to reproduce the bad sources at #26499. But I'll try: https://github.com/sagemath/sagenb/pull/462

comment:5 in reply to: ↑ 4 Changed 4 months ago by jdemeyer

Replying to jdemeyer:

That's hard to say since I don't know the exact steps to reproduce the bad sources at #26499.

Do you still have the sagenb checkout that you used for #26499? If so, could you run git status and git clean -n -x -d there and show the output?

Last edited 4 months ago by jdemeyer (previous) (diff)

comment:6 Changed 4 months ago by jdemeyer

  • Branch set to u/jdemeyer/repackage_sagenb

comment:7 follow-up: Changed 4 months ago by dimpase

  • Commit set to d78718449183c27cef07c98b06b512218e060bcf

No, I don't have a directory I packaged 1.1.0 in, anymore.

Can't one add an appropriate call to git clean to ./dist.sh to avoid such a mess in the future?


Where is the new tarball?


New commits:

2c2df8dProperly check errors when copying files
d787184Upgrade to sagenb version 1.1.1

comment:8 in reply to: ↑ 7 Changed 4 months ago by jdemeyer

Replying to dimpase:

No, I don't have a directory I packaged 1.1.0 in, anymore.

Can't one add an appropriate call to git clean to ./dist.sh to avoid such a mess in the future?

Yes, that is what I did in https://github.com/sagemath/sagenb/pull/462

comment:9 Changed 4 months ago by git

  • Commit changed from d78718449183c27cef07c98b06b512218e060bcf to 8a0e01741fe7b263e9c8360a33d55175ac1e6490

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

8a0e017Upgrade to sagenb version 1.1.1

comment:10 Changed 4 months ago by jdemeyer

This seems to be working now, but I'll wait on feedback about ​https://github.com/sagemath/sagenb/pull/462 before creating the final tarball.

comment:11 Changed 4 months ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Description modified (diff)

comment:12 Changed 4 months ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 4 months ago by git

  • Commit changed from 8a0e01741fe7b263e9c8360a33d55175ac1e6490 to 8824d9344a32107ba56d718eca6b930164149807

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

8824d93Upgrade to sagenb version 1.1.1

comment:14 Changed 4 months ago by jdemeyer

  • Status changed from new to needs_review

comment:15 follow-up: Changed 4 months ago by dimpase

How does one deal with mathjax after this? Have you tested it works with sagenb and Jupiter (after installing something?)

I can't test it now myself, sitting in a ready to depart plane...

Last edited 4 months ago by dimpase (previous) (diff)

comment:16 in reply to: ↑ 15 Changed 4 months ago by jdemeyer

Replying to dimpase:

How does one deal with mathjax after this?

The same way as we dealt with mathjax before #26499. In other words, we don't need to do anything special.

comment:17 follow-ups: Changed 4 months ago by chapoton

NOTE: the section of spkg-install for sagenb that builds sagenb documentation is very fragile, and is breaking the build of sage from scratch (not an incremental build).

I would rather remove that part.

comment:18 in reply to: ↑ 17 Changed 4 months ago by jhpalmieri

Replying to chapoton:

NOTE: the section of spkg-install for sagenb that builds sagenb documentation is very fragile, and is breaking the build of sage from scratch (not an incremental build).

I would rather remove that part.

That should probably be a separate ticket. I would like to see more details.

comment:19 Changed 4 months ago by jhpalmieri

This works for me: I get the correct behavior with sage -n, sage -n sagenb, and sage -n jupyter. Both notebooks work for simple calculations – I didn't try anything very complicated.

comment:20 Changed 4 months ago by jhpalmieri

  • Reviewers set to John Palmieri

I'm willing to give this a positive review. Any objections?

comment:21 Changed 4 months ago by dimpase

  • Reviewers changed from John Palmieri to John Palmieri, Dima Pasechnik
  • Status changed from needs_review to positive_review

Lgtm

comment:22 Changed 4 months ago by jdemeyer

  • Dependencies set to #26642

comment:23 Changed 4 months ago by vbraun

  • Branch changed from u/jdemeyer/repackage_sagenb to 8824d9344a32107ba56d718eca6b930164149807
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 in reply to: ↑ 17 Changed 4 months ago by jhpalmieri

  • Commit 8824d9344a32107ba56d718eca6b930164149807 deleted

Replying to chapoton:

NOTE: the section of spkg-install for sagenb that builds sagenb documentation is very fragile, and is breaking the build of sage from scratch (not an incremental build).

I would rather remove that part.

There is now #26686. Do you think that is the same problem you are having?

comment:25 Changed 4 months ago by dimpase

It should be the same, just missing dependencies in sagenb. Mea culpa - I think #26686 fixes this.

Note: See TracTickets for help on using tickets.