Error checking in sage-spkg

[vbraun]

• Remove duplicate checksumming
• Do not silently ignore errors in sage-spkg.

[vbraun]

New commits:

​7c65f3d	Try our own tarball host last
​d5bbfc0	Merge #18417 into #18428
​e280943	remove checksumming from sage-spkg
​aa24229	Do not ignor errors in sage-spkg

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​49aa3c5

Only download mirror list when needed

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​fd0b5ec

sage-spkg has to ignore some errors when building from scratch

[jdemeyer]

Surely, this is wrong:

sage-download-file "$PKG_URL" "$PKG_TMP"
if [ $? -ne 0 ]; then
    exit 1
fi
if [ $? -ne 0 ]; then
    # Delete failed download
    rm -f "$PKG_TMP"

[jdemeyer]

Why this?

export PYTHONUNBUFFERED=yes

Why is it a problem if output is buffered?

(I am a bit worried about performance implications, since this is passed to every Python script run during the installation of Sage).

[jdemeyer]

And I also don't understand why you remove some of the

if [ $INFO -eq 0 ]; then

branches.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​c8c2601

remove extraneous error check

[vbraun]

The problem with buffering is that network connections might take a while, and the last message on the screen should be flushed to indicate what we are doing. PYTHONUNBUFFERED only makes Python stdin/out unbuffered, and we don't print() that much. So I don't see where there could be a performance impact. Even in the worst case (passing the entire build log through Python unbuffered, which we don't do) its unlikely to take a long time compared to actually producing the build log.

The first INFO branch is about checking whether the tarball is already downloaded, which is now done by sage-download file. The second INFO branch is just printing logs, and I moved that to the relevant place.

[jdemeyer]

Replying to vbraun:

The problem with buffering is that network connections might take a while, and the last message on the screen should be flushed to indicate what we are doing.

Of course, that can be done with sys.stdout.flush() also...

Regarding this, I also don't think that sage-spkg is the best place to put this. I would rather like to put it in build/deps, so it also applies to building the Sage library.

[vbraun]

Not sure I follow, build/deps is a Makefile but just setting a make variable is not good enough. You mean use the "export" gnu make extension to change the environment?

Probably the best solution would be to subclass the stdout stream to a version that always flushes. But sage-download-file is already too many loc. I'm thinking to refactor it into a python package (say, src/sage_bootstrap) that deals with downloading and potentially other pre-installation stuff. That then could also be tested nicely with tox to see that it runs on different Python versions. But we should probably fix more urgent issues first.

[jdemeyer]

Can we just move the buffering stuff to a new ticket and focus on the "more urgent issues first" then?

[vbraun]

Sure, it just simplifies analyzing problems if the last log message is actually on the screen.

[jdemeyer]

Replying to vbraun:

You mean use the "export" gnu make extension to change the environment?

Well, I guess we are already assuming GNU make. Alternatively, put it in build/install.

[vbraun]

But that is only used during the initial build, and not when running sage -i <package>, right?

[jdemeyer]

Replying to vbraun:

But that is only used during the initial build, and not when running sage -i <package>, right?

Right indeed.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​a5e492d

Remove buffering fix

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​616ee13

Flush printed output

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​69a501d

Minor fixes to sage-spkg

[jdemeyer]

needs review...