Clean up top-level Makefile

[jdemeyer]

Using && inside commands for pipestatus does not work. Therefore, in the top-level Makefile, we should move $(TESTPRELIMS) out of $(PIPE).

For example, if sage-starts fails, one would get

$ make ptestlong
[...]
spkg/pipestatus ". local/bin/sage-env && sage-starts && ./sage -tp 0  -sagenb -long devel/sage/doc/common devel/sage/doc/de devel/sage/doc/en devel/sage/doc/fr devel/sage/doc/ru devel/sage/sage 2>&1" "tee -a ptestlong.log"

Testing that Sage starts...
Traceback (most recent call last):
  File "/usr/local/src/sage-4.7.2.alpha4/local/bin/sage-eval", line 4, in <module>
    from sage.all import *
  File "/usr/local/src/sage-4.7.2.alpha4/local/lib/python2.6/site-packages/sage/all.py", line 68, in <module>
    from sage.misc.all       import *         # takes a while
  File "/usr/local/src/sage-4.7.2.alpha4/local/lib/python2.6/site-packages/sage/misc/all.py", line 81, in <module>
    from functional import (additive_order,
  File "/usr/local/src/sage-4.7.2.alpha4/local/lib/python2.6/site-packages/sage/misc/functional.py", line 36, in <module>
    from sage.rings.complex_double import CDF
  File "integer.pxd", line 9, in init sage.rings.complex_double (sage/rings/complex_double.c:14716)
ImportError: libgmp.so.8: cannot open shared object file: No such file or directory
Sage failed to start up.
Please email sage-devel (http://groups.google.com/group/sage-devel)
explaining the problem and send the log file
  /usr/local/src/sage-4.7.2.alpha4/start.log
Describe your computer, operating system, etc.
spkg/pipestatus: line 44: [: -ne: unary operator expected
make: *** [ptestlong] Error 1

Note the penultimate line.

The attached patch also:

1. removes sage-starts from sage -tp, because running make ptest or make ptestlong already runs sage-starts.
2. runs ./sage -b in the make build rule. This ensures that the Sage library is up-to-date when testing or building documentation.
3. adds make testalllong and make ptestalllong for completeness.
4. generally cleans up the Makefile.

Apply:

1. attachment: 11969_testprelims.patch to SAGE_ROOT
2. attachment: 11969_scripts.patch to scripts

Depends on #11926
Depends on #11972
Depends on #11959

Component: build

Keywords: Makefile pipestatus

Author: Jeroen Demeyer

Reviewer: John Palmieri, Leif Leonhardy

Merged: sage-4.8.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/11969

[jdemeyer]

Changed keywords from none to Makefile pipestatus

[nexttime]

comment:4

Looks reasonable, although you could just use parentheses in the parameter to pipestatus. (That way, the output of sage-starts would still get logged, too.)

[jhpalmieri]

comment:5

Looks okay to me, too. I think it also makes sense to move sage-env out of TESTPRELIMS and only run it explicitly where needed. I think that logging the output from sage-starts might be nice, but it's pretty minor.

Does sage -t accept --optional as meaning the same as -optional? It accepts --sagenb and --long. Do you want to change those while you're editing those lines?

[jdemeyer]

comment:6

I noticed that make ptestlong actually runs sage-starts twice: once from the Makefile and once from sage-sage.

I propose not to run sage-starts when the user explicitly runs

$ ./sage -tp [...]

but to run sage-starts only when the user does

$ make ptestlong

The rationale is that sage -tp is often done with one or few files and then the overhead of sage-starts might be significant. However, for a make ptestlong run, running sage-starts once does not really impact the testing time.

[jdemeyer]

comment:7

Attachment: 11969_scripts.patch.gz

[jdemeyer]

comment:9

I made some additional changes to the top-level Makefile, needs_review

[jhpalmieri]

comment:11

I'm wary of not running sage-starts as part of "sage -tp". It was introduced in #7103 to fix a problem when someone ran "sage -tp" before actually running Sage. I can see someone doing "make build" and then "./sage -tp ..." in order to test whether a new Sage build fixes problems in a particular directory.

[jdemeyer]

comment:12

Replying to @jhpalmieri:

I can see someone doing "make build" and then "./sage -tp ..." in order to test whether a new Sage build fixes problems in a particular directory.

Then all those tests would fail. Is that a problem?

In all seriousness: I can see some arguments supporting sage-starts before ./sage -tp [...]. But then we should also do that for ./sage -t [...]. I value consistency and predictability very high, I prefer either "always" or "never" to anything in between.

[jhpalmieri]

comment:13

Replying to @jdemeyer:

Replying to @jhpalmieri:

I can see someone doing "make build" and then "./sage -tp ..." in order to test whether a new Sage build fixes problems in a particular directory.

Then all those tests would fail. Is that a problem?

No, the problem is that you might get this:

A mysterious error (perhaps a memory error?) occurred, which may have crashed doctest.

And you get it from a race condition, where multiple processes are simultaneously trying to create the same subdirectories of .sage. So it doesn't occur when you do "sage -t", only "sage -tp".

[nexttime]

comment:14

Replying to @jdemeyer:

I noticed that make ptestlong actually runs sage-starts twice: once from the Makefile and once from sage-sage.

I propose not to run sage-starts when the user explicitly runs

$ ./sage -tp [...]

but to run sage-starts only when the user does

$ make ptestlong

The rationale is that sage -tp is often done with one or few files and then the overhead of sage-starts might be significant. However, for a make ptestlong run, running sage-starts once does not really impact the testing time.

Yes, definitely, i.e. remove it from sage-sage.

[jdemeyer]

Dependencies: #11926

[jdemeyer]

comment:15

Rebased to #11926.

[jdemeyer]

comment:16

Replying to @jhpalmieri:

And you get it from a race condition, where multiple processes are simultaneously trying to create the same subdirectories of .sage.

How is this solved by running sage-starts? I would assume that a user doing ./sage -tp has already ran Sage at least once. And it also seems that #11926 should solve precisely this problem (except in the rare case where the changed code has to do with creating directories in the Sage startup).

Also, such a race condition is a bug in Sage. If you have more information about when it occurs and for which directories, we can look into this. (I find it very annoying that Python doesn't seem to have an equivalent of mkdir -p, making a directory but not complaining if it exists).

[nexttime]

comment:17

Sorry, hadn't read John's comments before my last post.

But IMHO, if there are race conditions in creating directories (in $DOT_SAGE/), the race conditions should get fixed instead of using dumb work-arounds that are hardly ever necessary.

Note that e.g. only sage-ptest supports SAGE_TEST_ITER.

[nexttime]

comment:18

Replying to @jdemeyer:

Replying to @jhpalmieri:

And you get it from a race condition, where multiple processes are simultaneously trying to create the same subdirectories of .sage.

I would assume that a user doing ./sage -tp has already ran Sage at least once. And it also seems that #11926 should solve precisely this problem (except in the rare case where the changed code has to do with creating directories in the Sage startup).

This doesn't help if $DOT_SAGE differs, e.g. because the user changed the variable's setting, or it's just a different user that runs Sage [for the first time maybe, or deleted his previous .sage, or there are new directories to create].

[jhpalmieri]

comment:19

If you have more information about when it occurs and for which directories, we can look into this.

You could look at #7103, as I mentioned above, and to #7079, to which #7103 refers.

I find it very annoying that Python doesn't seem to have an equivalent of mkdir -p, making a directory but not complaining if it exists

See #11972. I think the patch there should also take care of the race conditions when creating DOT_SAGE and its various subdirectories, which seemed to be the problem with #7103. One way to test: remove the sage-starts code from the doctesting routines and set DOT_SAGE to a non-existing directory and run "sage -tp ...". Repeat many times and see if you ever get "mysterious errors". Then apply the patch at #11972 and see if that fixes it.

If this fixes it, then I'm willing to make #11972 a prerequisite for this ticket and remove the execution of sage-starts from sage-sage. But I'm not willing to remove sage-starts unless we're pretty sure that we haven't reintroduced a possible bug in doctesting.

[jdemeyer]

Changed dependencies from #11926 to #11926, #11972

[jdemeyer]

Attachment: 11969_testprelims.patch.gz

[jdemeyer]

Changed dependencies from #11926, #11972 to #11926, #11972, #11959

[jdemeyer]

Milestone sage-4.7.3 deleted

[jhpalmieri]

Reviewer: John Palmieri, Leif Leonhardy

[jhpalmieri]

comment:25

This all looks good to me, and I'm happy with the patch to the scripts repo now that #11972 is merged.

[jdemeyer]

comment:26

Replying to @jhpalmieri:

This all looks good to me, and I'm happy with the patch to the scripts repo now that #11972 is merged.

Thanks for the review!

[jdemeyer]

Merged: sage-4.8.alpha2