make doesn't work properly for targets 'test' and 'micro_release'

[Itai Bar-Natan]

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​

[Itai Bar-Natan]

A patch which fixes the bug

[Itai Bar-Natan]

I should have done this a while ago since I already have a patch.

[Jean-Pierre Flori]

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.

[Jean-Pierre Flori]

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...

[Jean-Pierre Flori]

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

[Jean-Pierre Flori]

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.

[Jean-Pierre Flori]

My bad once more. sage-env is only meant to be sourced (and only once) and should not be executable: #10469

[Jean-Pierre Flori]

Ok here is a slightly reworked patch making the test target like the other ones.

[Jeroen Demeyer]

1. Obviously, /usr/bin/env bash should be simply bash.

2. Then, if you don't use sage-maketest anymore (which is probably a good thing), remove the sage-maketest script.

[Jean-Pierre Flori]

I think the sage-maketest script is used when you call "sage -testall" which is called by some make targets so is still necessary.

[Jean-Pierre Flori]

Reviewer patch; rebase on 5.0 series

[Jean-Pierre Flori]

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.

[Jeroen Demeyer]

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.