Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17375 closed defect (fixed)

Maxima should not need a working C compiler to run

Reported by: pbruin Owned by:
Priority: blocker Milestone: sage-6.5
Component: packages: standard Keywords: maxima
Cc: nbruin, vbraun Merged in:
Authors: Peter Bruin Reviewers: Karl-Dieter Crisman
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 9f12141 (Commits) Commit:
Dependencies: Stopgaps:

Description

It turns out that after the upgrade to Maxima 5.34.1 (#16908), a working C compiler is needed to run Maxima on ECL, which is of course inacceptable. The problem is caused by the compilation of four regular expressions on startup.

As a workaround until this is fixed upstream, we revert the Maxima commit introducing this compilation.

Initial report and diagnosis: https://groups.google.com/forum/#!topic/sage-support/wj4ObDhv_xE

Upstream report: http://sourceforge.net/p/maxima/bugs/2845/

Change History (12)

comment:1 Changed 5 years ago by pbruin

  • Branch set to u/pbruin/17375-maxima_compile
  • Commit set to 203767dd8a46873d41df73693739338cc61d85d6
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by kcrisman

  • Cc vbraun added

Thanks for the quick diagnosis. I'm trying this right now. For full testing, someone should test this on a system which does NOT have the compiler - though I don't know how such a person would get the branch! (Maybe move gcc after compiling?)

I would almost say this earns a "bug fix only release", at least for the purpose of making binaries, though unfortunately this branch is based on beta0. After all, it breaks a lot of calculus! Volker, what do you think?

comment:3 follow-up: Changed 5 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

I'm happy with this, it solves the problem of the extra messages and hence presumably the compiler, passes tests, and I have tested that moving the Sage gcc out of the way when this is not applied causes the problem with GAZONK when running sage -maxima, but doing so after this branch is applied causes no problems.

So I think we are cool here. Anything I might have missed? Again, I think it is quite reasonable to have a bug fix with just this commit cherry-picked as Sage 6.4.1 (after all, no new spkg needed) while also including this in the normal Sage 6.5.beta1 so as not to worry about history.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 5 years ago by pbruin

Replying to kcrisman:

I'm happy with this, it solves the problem of the extra messages and hence presumably the compiler, passes tests, and I have tested that moving the Sage gcc out of the way when this is not applied causes the problem with GAZONK when running sage -maxima, but doing so after this branch is applied causes no problems.

Thanks for the quick review!

So I think we are cool here. Anything I might have missed? Again, I think it is quite reasonable to have a bug fix with just this commit cherry-picked as Sage 6.4.1 (after all, no new spkg needed) while also including this in the normal Sage 6.5.beta1 so as not to worry about history.

This also sounds good to me.

comment:5 in reply to: ↑ 4 Changed 5 years ago by kcrisman

I'm happy with this, it solves the problem of the extra messages and hence presumably the compiler, passes tests, and I have tested that moving the Sage gcc out of the way when this is not applied causes the problem with GAZONK when running sage -maxima, but doing so after this branch is applied causes no problems.

Thanks for the quick review!

Well, having calculus work for non-developers is a fairly high priority for me ;-)

So I think we are cool here. Anything I might have missed? Again, I think it is quite reasonable to have a bug fix with just this commit cherry-picked as Sage 6.4.1 (after all, no new spkg needed) while also including this in the normal Sage 6.5.beta1 so as not to worry about history.

This also sounds good to me.

Hopefully our release manager will concur, though it will probably be a little extra annoying.

comment:6 Changed 5 years ago by vbraun

Anything that you want in a hotfix must be based on 6.4, otherwise it'll drag in the whole beta. Cherry-picking would just lead to merge conflicts later, its akin to rebasing.

comment:7 Changed 5 years ago by pbruin

  • Branch changed from u/pbruin/17375-maxima_compile to u/pbruin/17375-maxima_compile-6.4
  • Commit changed from 203767dd8a46873d41df73693739338cc61d85d6 to 9f12141c59c6b0c9a6d69928996b8cea00f382e2

Here is a (one-commit) branch with the same content but now based on 6.4.

comment:8 Changed 5 years ago by kcrisman

Thanks for doing that!

Volker, you can still add this commit in the next beta as well, correct - the conflicts only would happen the other direction, I guess?

comment:9 Changed 5 years ago by vbraun

Conflicts happen if you put the same change to the source under more than one commit, I don't understand what you mean by direction. There is only one direction in git, and that is forward ;-)

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/pbruin/17375-maxima_compile-6.4 to 9f12141c59c6b0c9a6d69928996b8cea00f382e2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 follow-up: Changed 5 years ago by kcrisman

  • Commit 9f12141c59c6b0c9a6d69928996b8cea00f382e2 deleted
  • Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

Just FYI this has been fixed upstream, so when we upgrade again, we can remove this patch.

comment:12 in reply to: ↑ 11 Changed 5 years ago by pbruin

Replying to kcrisman:

Just FYI this has been fixed upstream, so when we upgrade again, we can remove this patch.

An improved fix by Nils has now been committed upstream: http://sourceforge.net/p/maxima/bugs/2848/

Note: See TracTickets for help on using tickets.