Opened 10 years ago
Closed 10 years ago
#12568 closed defect (fixed)
make doesn't work properly for targets 'test' and 'micro_release'
Reported by: | itaibn | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | build | Keywords: | make test micro_release |
Cc: | Merged in: | sage-5.0.beta14 | |
Authors: | Itai Bar-Natan, Jean-Pierre Flori | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I am using sage 4.8 with GNU make 3.81. When I am in SAGE_ROOT
, if I type the commands make test
or make micro_release
, the shell returns an error. For make micro_release
, the it returns the following output:
. local/bin/sage-env && local/bin/sage-micro_release local/bin/sage-env: 289: Bad substitution mkdir: cannot create directory `//matplotlib-1.0.1': Permission denied local/bin/sage-env: 475: Syntax error: "(" unexpected make: *** [micro_release] Error 2
For make test
, it returns a longer output which ends with:
. local/bin/sage-env && sage-maketest local/bin/sage-env: 289: Bad substitution mkdir: cannot create directory `//matplotlib-1.0.1': Permission denied local/bin/sage-env: 475: Syntax error: "(" unexpected make: *** [test] Error 2
The problem appears to be because make runs each command as a sh script, but that sage-env is a bash script. A similar error message appears if I type . local/bin/sage-env
in interactive sh.
Apply only trac_12568-reviewer.patch
Attachments (2)
Change History (18)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
I should have done this a while ago since I already have a patch.
comment:2 Changed 10 years ago by
- Reviewers set to Jean-Pierre Flori
- Status changed from needs_review to positive_review
comment:3 Changed 10 years ago by
- Status changed from positive_review to needs_work
I was a little too fast on this. Are we sure that bash will be installed on every system sage supports (and is it a sage dependency) ? I fear that explicitely calling bash might fail even if a compatible shell is installed.
I think a better solution would be modify the sage-env header to something like "#! /usr/bin/env bash". By the way, this file should be executable.
comment:4 Changed 10 years ago by
Moreover the sage-env script is in the spkg/bin directory and not in the local/bin one (edit: this is a recent change #11073).
Finally the micro_release target segfault on my machine...
comment:5 Changed 10 years ago by
I propose to let the test target behave like the other ones, i.e. call "sage -t ..." (in the end it calls sage-maketest anyway, but at least sage-env is properly called by the sage script).
For the micro_release one, I'm not sure we can do much better than the solution Itai proposed without calling sage-micro_release from the sage script to properly set SAGE_ROOT (i.e. through an option or something like that).
comment:6 Changed 10 years ago by
Not sure why make micro_release now and not before... It does so while trying to strip libpython which is expected: http://trac.sagemath.org/sage_trac/ticket/11743#comment:3
What's more mysterious is why it tries to strip libpython now. It was updated to 2.7 in the 5.0 series but is still readonly.
In fact it did already segfault with 4.8. It's just that because I used system atlas libs with my 4.8 install, it did fail before.
comment:7 Changed 10 years ago by
My bad once more. sage-env is only meant to be sourced (and only once) and should not be executable: #10469
comment:8 Changed 10 years ago by
Ok here is a slightly reworked patch making the test target like the other ones.
comment:9 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:10 Changed 10 years ago by
- Obviously,
/usr/bin/env bash
should be simplybash
.
- Then, if you don't use
sage-maketest
anymore (which is probably a good thing), remove thesage-maketest
script.
comment:11 Changed 10 years ago by
- Status changed from needs_review to needs_work
comment:12 Changed 10 years ago by
I think the sage-maketest script is used when you call "sage -testall" which is called by some make targets so is still necessary.
comment:13 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:14 Changed 10 years ago by
In fact the testall option is not present in the make targets, but is defined and documented by spkg/bin/sage. Anyway, I don't think here is the place to remove it.
comment:15 Changed 10 years ago by
- Reviewers changed from Jean-Pierre Flori to Jeroen Demeyer
- Status changed from needs_review to positive_review
make micro_release
is indeed broken. It's not surprising, as it changes the python library while python
is running:
strip "/tmp/local/sage/local/lib/libpython2.7.so" bash: line 1: 5817 Segmentation fault local/bin/sage-micro_release
but that's not relevant for this ticket.
Everything else seems to work, so positive_review.
comment:16 Changed 10 years ago by
- Merged in set to sage-5.0.beta14
- Resolution set to fixed
- Status changed from positive_review to closed
A patch which fixes the bug