#30563 closed enhancement (fixed)
Use configuration variable MAXIMA instead of hardcoding "maxima"
Reported by:  Matthias Köppe  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  build: configure  Keywords:  
Cc:  Tobias Hansen, Michael Orlitzky, François Bissey  Merged in:  
Authors:  François Bissey  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  3da851f (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Continuation of #29038.
Upstreaming a generalized version of https://salsa.debian.org/scienceteam/sagemath//blob/master/debian/patches/d0maxima.patch
Change History (25)
comment:1 Changed 2 years ago by
Description:  modified (diff) 

comment:2 Changed 2 years ago by
Cc:  Michael Orlitzky François Bissey added 

comment:3 followup: 4 Changed 2 years ago by
comment:4 Changed 2 years ago by
Replying to fbissey:
I am guessing that ideally we want to use the variable
MAXIMA
Yes, that's the idea
comment:5 followup: 6 Changed 2 years ago by
maxima.py
is already using MAXIMA
that leaves us with touching maxima_abstract.py
and a couple of things to help sageondebian pass its doctests out of the box.
Not sure how to proceed with bin/sage
as it is not reading that configuration file, nor should it.
comment:6 followup: 7 Changed 2 years ago by
Replying to fbissey:
Not sure how to proceed with
bin/sage
as it is not reading that configuration file, nor should it.
sageconfig MAXIMA
works if sage_conf
is installed, can fall back to maxima
comment:7 followup: 8 Changed 2 years ago by
Replying to mkoeppe:
Replying to fbissey:
Not sure how to proceed with
bin/sage
as it is not reading that configuration file, nor should it.
sageconfig MAXIMA
works ifsage_conf
is installed, can fall back tomaxima
OK, that's another issue, I don't like how sageconfig is installed as a separate package but this is kind of orthogonal.
comment:8 Changed 2 years ago by
Replying to fbissey:
I don't like how sageconfig is installed as a separate package but this is kind of orthogonal.
Yes, that's orthogonal.
comment:9 Changed 2 years ago by
Branch:  → u/fbissey/ticket_30563 

Commit:  → 38010138ed9f9c8d405b4e521526a749fc399f37 
I shouldn't just leave things sitting on my hard drive.
There is a couple more things to do before calling it done.
New commits:
3801013  use the MAXIMA variable in maxima_abstract

comment:10 Changed 2 years ago by
Commit:  38010138ed9f9c8d405b4e521526a749fc399f37 → 1ce58a777cb26f6f8e0a92a351904d51832da886 

Branch pushed to git repo; I updated commit sha1. New commits:
1ce58a7  Relax some doctesting of string outputs for sageondebian

comment:11 Changed 2 years ago by
Commit:  1ce58a777cb26f6f8e0a92a351904d51832da886 → 10bafa6f07531cabfb56adb1868da44c1cb0874b 

Branch pushed to git repo; I updated commit sha1. New commits:
10bafa6  use sageconfig to figure maxima in bin/sage

comment:12 Changed 2 years ago by
@mkoeppe was the last commit the kind of things you had in mind for src/bin/sage
?
comment:13 Changed 2 years ago by
Yes, something like this. Probably needs stderr redirection though.
comment:14 Changed 2 years ago by
Most definitely, but after a night sleep I think I need to change the design slightly.
comment:15 Changed 2 years ago by
Commit:  10bafa6f07531cabfb56adb1868da44c1cb0874b → 5cedc360a97378e27fe1e5eff6fe34205a1fbb72 

Branch pushed to git repo; I updated commit sha1. New commits:
5cedc36  redirect error message so it doesn't look scary when things are missing (in distros for example)

comment:16 Changed 2 years ago by
Authors:  → François Bissey 

Status:  new → needs_review 
I was overthinking things. The current branch takes care of most of the stuff in the debian patch and should work nicely in most distros with minimal effort.
comment:17 Changed 2 years ago by
+ exec "$maxima_cmd" "$@"
I think $maxima_cmd
shouldn't be quoted here
comment:18 Changed 2 years ago by
Funny, I thought it should be quoted, because it could be a string with spaces in it. maxima l ecl
specifically. But I initially had put "$maxima_cmd $@"
which failed miserably when calling ./sage maxima
without any argument because of the space. So if it is safe for the case above, I am OK with removing the quotes.
comment:20 Changed 2 years ago by
Indeed it does. And even if I remove the quotes there is an extra bit that needs to be dealt with. The l
is getting interpreted in the []
on line 609 so I need to do something more careful.
comment:21 Changed 2 years ago by
Commit:  5cedc360a97378e27fe1e5eff6fe34205a1fbb72 → 3da851ff141e4eba5035ebcb994907b4902ec7da 

Branch pushed to git repo; I updated commit sha1. New commits:
3da851f  Simplify the maxima_cmd setting logic and make it more robust. Also get rid of problematic quotes.

comment:22 Changed 2 years ago by
Turns out my overnight thought of redesign are not wasted. It is much more elegant and less problematic. Any objection to the default being maxima l ecl
? That means it works in sageongentoo with the right interpreter even if we don't ship sageconfig
. It should work with any maxima used by sage.
comment:23 Changed 2 years ago by
Reviewers:  → Matthias Koeppe 

Status:  needs_review → positive_review 
Looks good to me.
comment:24 Changed 2 years ago by
Branch:  u/fbissey/ticket_30563 → 3da851ff141e4eba5035ebcb994907b4902ec7da 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:25 Changed 2 years ago by
Commit:  3da851ff141e4eba5035ebcb994907b4902ec7da 

I missed a case in my testing of maxima l ecl
. Follow up at #30676.
I am guessing that ideally we want to use the variable
MAXIMA
rather than thosemaxima
andmaximasage
from the patch. It could be interesting as I could replacewith a variable assignment in sageongentoo.