Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#11959 closed enhancement (fixed)

Remove the necessity to set SAGE_PARALLEL_SPKG_BUILD

Reported by: jhpalmieri Owned by: GeorgSWeber
Priority: minor Milestone: sage-4.8
Component: build Keywords: SAGE_PARALLEL_SPKG_BUILD MAKE -j --jobs
Cc: leif, kcrisman Merged in: sage-4.8.alpha0
Authors: John Palmieri Reviewers: Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11926 Stopgaps:

Description (last modified by jdemeyer)

The patches on this ticket remove necessity to set the environment variable SAGE_PARALLEL_SPKG_BUILD to turn parallel spkg building on. Thus it will be on by default, but note that "parallel spkg building" only has an effect if MAKE is set to, for example, "make -j16".

In the current implementation, parallel building can be disabled by either setting the variable to "no" or by running "make build-serial".

This change was discussed in sage-devel.

Apply:

Attachments (8)

trac_11959-root.patch (8.2 KB) - added by jhpalmieri 8 years ago.
root repo
trac_11959-sage.patch (5.1 KB) - added by jhpalmieri 8 years ago.
Sage library
trac_11959-sage-delta1to2.patch (1.1 KB) - added by jhpalmieri 8 years ago.
for review only
trac_11959-root-delta1to2.patch (4.3 KB) - added by jhpalmieri 8 years ago.
for review only
trac_11959-sage.v2.patch (5.2 KB) - added by jhpalmieri 8 years ago.
Sage library
trac_11959-root.v2.patch (8.7 KB) - added by jhpalmieri 8 years ago.
root repo
trac_11959-root.v2-rebased.patch (8.4 KB) - added by jdemeyer 8 years ago.
Rebased on top of #11926
trac_11959-sage.v2-rebased.patch (5.0 KB) - added by jdemeyer 8 years ago.
Rebased on top of #11926

Download all attachments as: .zip

Change History (50)

comment:1 Changed 8 years ago by jhpalmieri

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by leif

  • Keywords SAGE_PARALLEL_SPKG_BUILD MAKE -j --jobs added

Objections to renaming the ticket to "Remove the necessity to set SAGE_PARALLEL_SPKG_BUILD"? (It's yours, so I didn't change its title. ;-) )

comment:3 Changed 8 years ago by leif

John, you confused the names of the patches. :)

comment:4 Changed 8 years ago by leif

My alternate patch to $SAGE_ROOT/spkg/install has become a bit longer:

  • spkg/install

    diff --git a/spkg/install b/spkg/install
    a b  
    430430
    431431
    432432# Skip the rest if nothing to do (i.e., to [re]build):
    433 # Note that we should use $MAKE here, but we currently don't use
    434 # it further below either unless SAGE_PARALLEL_SPKG_BUILD=yes, which
    435 # I consider a "bug". -- Leif Leonhardy
    436 # If "make" doesn't understand the -q option, it should exit with a
    437 # non-zero status which is not a problem.
    438 if make -q -f standard/deps $1; then
     433# If "make" doesn't understand the -q option (although we require
     434# GNU make, which supports it), it should exit with a non-zero status
     435# which is not a problem.
     436if ${MAKE:-make} -q -f standard/deps $1; then
    439437    echo "Nothing to (re)build / all up-to-date."
    440438    exit 0
    441439fi
     
    443441
    444442# Dump environment for debugging purposes:
    445443echo "*** ALL ENVIRONMENT VARIABLES BEFORE BUILD: ***"
    446 env
     444env | sort
    447445echo "***********************************************"
    448446
    449447
    450448###############################################################################
    451449# NOW do the actual build:
    452450###############################################################################
    453 if [ "$SAGE_PARALLEL_SPKG_BUILD" = "yes" ] && [ -n "$MAKE" ]; then
    454     time $MAKE -f standard/deps $1
    455 else
    456     time make -f standard/deps $1
    457 fi
     451time ${MAKE:-make} -f standard/deps $1
    458452
    459 # added by Burcin Erocal, see trac #6295.
     453# Added by Burcin Erocal, see trac #6295:
    460454if [ $? -ne 0 ]; then
    461     echo "Error building Sage."
     455    echo >&2 "Error building Sage."
    462456    exit 1
    463457fi
    464458
     459# Rename makefile to Makefile (see #10156):
    465460cd "$SAGE_ROOT"
    466 
    467 # Rename makefile to Makefile (see #10156)
    468461if [ -f makefile ]; then
    469     mv makefile Makefile
    470     if [ $? -ne 0 ]; then
    471         echo "Error renaming 'makefile' to 'Makefile'."
    472         exit 1
     462    if [ ! -f Makefile ]; then
     463        if ! mv makefile Makefile; then
     464            echo >&2 "Error renaming 'makefile' to 'Makefile'."
     465            exit 1
     466        fi
     467    else
     468        # Both 'makefile' and 'Makefile' exist.
     469        echo >&2 "Warning: Deleting old 'makefile'; 'Makefile' remains."
     470        if ! rm -f makefile; then
     471            echo >&2 "Error deleting old 'makefile'."
     472            exit 1
     473        fi
    473474    fi
    474475fi
    475476
    476 # build succeeded
    477 if [ "$1" = "all" ]; then
    478     echo "To install gap, gp, singular, etc., scripts"
    479     echo "in a standard bin directory, start sage and"
    480     echo "type something like"
    481     echo "   sage: install_scripts('/usr/local/bin')"
    482     echo "at the Sage command prompt."
    483     echo ""
    484     echo "To build the documentation, run"
    485     echo "   make doc"
    486     echo ""
    487 fi
    488 
     477# Build succeeded.
    489478echo "Sage build/upgrade complete!"
    490479
     480if [ "$1" = "all" ]; then
     481    echo
     482    echo "To install small scripts to directly run Sage's versions of GAP,"
     483    echo "the PARI/GP interpreter, Maxima, or Singular etc. (by typing e.g."
     484    echo "just 'gap' or 'gp') into a standard 'bin' directory, start Sage"
     485    echo "by typing 'sage' (or './sage') and enter something like"
     486    echo
     487    echo "    install_scripts('/usr/local/bin')"
     488    echo
     489    echo "at the Sage command prompt ('sage:')."
     490    echo
     491    echo "To (re)build the HTML documentation, run 'make doc' (if you haven't"
     492    echo "issued 'make' but just 'make build' or e.g. 'sage --upgrade'; other-"
     493    echo "wise it will be built automatically right now)."
     494    echo
     495    echo "To build the PDF version of the documentation, run 'make doc-pdf'."
     496    echo
     497fi

comment:5 Changed 8 years ago by leif

W.r.t. README.txt:

I'd quote make ('make'), and directly refer to the GNU make (online) manual, i.e., its Options summary section (which IMHO explains -j and -l etc. quite good):

http://www.gnu.org/software/make/manual/make.html#Options-Summary

s/so for example if you set/so for example if you do/ (or ... type)

Perhaps also add that it's usually ok (or advantageous) to use more jobs than the machine has CPU cores (minus already running processes, or the current system load), especially if disk access isn't very fast [provided the machine isn't low of available memory or actually RAM].

comment:6 Changed 8 years ago by leif

W.r.t. the Sage Installation Guide:

export MAKE='make -jNUM' is not "a common setting of MAKE", but a shell command to set MAKE; 'make -jNUM' is the setting. (This has been in before.)

s/won't need to use/won't need to set/ (dito)

s/can speed up the process/can significantly speed up the process/

I'd omit "(Not all packages support this, but the ones for which this could be problematic should automatically disable it.)" since this is pretty irrelevant (or confusing); or rephrase it to say that a few packages do not support building them with multiple jobs and hence are built sequentially, although in parallel with other packages.

In the warning, substitute one-processor by single-core CPU or single-core machine. I'd also mention the platform (or operating system) to which this apparently applies; in my experience it is a non-issue on Linux. Attempting to build Sage with probably too many jobs should in general not hurt; one can simply reduce the number of jobs upon build errors and restart, or more precisely continue, the build afterwards by again typing make.

Similar what I've said w.r.t. README.txt applies to the Installation Guide.

Also we should somehow mention potential problems with ATLAS there, and perhaps what I said about self-tuning packages in general (on sage-devel). (Again, if ATLAS failed to build -- often just due to too high system load -- one can simply type make again, perhaps multiple times, since there are usually less packages left to build when make finally exits. If we haven't already, we should also mention that CPU throttling aka power-saving mode should be turned off; "on-demand" in contrast should be ok, i.e., provided it works properly. [I don't know whether that's in the README.txt; I think it is important enough that it should be there, too.])

comment:7 Changed 8 years ago by leif

P.S.:

I think (e.g.) MAKE="make -j3 -l10.0" or MAKE="make -j 3 -l 10.0" (or MAKE="make --jobs=3 --load-average=10.0") is more natural.

(Although AFAIK older versions of GNU make interpreted make -j 3 as "build target(!) '3' with infinitely many(!) jobs", so the first and last version may be safer.)

comment:8 follow-up: Changed 8 years ago by was

I'm against this as is, but I think something very close is OK.

I think SAGE_PARALLEL_SPKG_BUILD="no" should be supported, but otherwise everything should be like in this proposal. The reason is that it can be confusing trying to deal with build issues when the build log mixes up output from multiple packages being built at once. Also there can be weird build issues resulting from building multiple packages at once. However, using "make -j16" (say) is very useful in all cases on a machine with lots of processors so you don't have to wait so long for individual packages to build.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 8 years ago by leif

Replying to was:

I'm against this as is, but I think something very close is OK.

I think SAGE_PARALLEL_SPKG_BUILD="no" should be supported, but otherwise everything should be like in this proposal. The reason is that it can be confusing trying to deal with build issues when the build log mixes up output from multiple packages being built at once.

Therefore we have (in addition) individual logs for each spkg in $SAGE_ROOT/spkg/logs.


Also there can be weird build issues resulting from building multiple packages at once. However, using "make -j16" (say) is very useful in all cases on a machine with lots of processors so you don't have to wait so long for individual packages to build.

Well, building multiple spkgs in parallel is IMHO quite mature.

If you only need an equivalent of SAGE_PARALLEL_SPKG_BUILD=no (still building each spkg in parallel if it supports that, and of course MAKE is set to make -jN) for debugging,

$ echo ".NOTPARALLEL" >> $SAGE_ROOT/spkg/standard/deps

does the job.

We could of course document that, perhaps better in the Developer's Guide.

(Or provide some [other] way to enable and disable that feature -- by an environment variable or an easier command; the only odd thing is that deps is under revision control and hence you get uncommitted changes in the Sage root repository :-) whenever that file is modified. As an alternative, we could include some other "temporary" or volatile [make]file which is in .hgignore, and is either empty or contains just .NOTPARALLEL.)

comment:10 in reply to: ↑ 9 Changed 8 years ago by leif

Replying to leif:

(Or provide some [other] way to enable and disable that feature -- by an environment variable or an easier command; the only odd thing is that deps is under revision control and hence you get uncommitted changes in the Sage root repository :-) whenever that file is modified. As an alternative, we could include some other "temporary" or volatile [make]file which is in .hgignore, and is either empty or contains just .NOTPARALLEL.)

We could easily achieve that by changing spkg/install to

...

if [[ ${SAGE_PARALLEL_SPKG_BUILD:-yes} = no ]]; then
    # Disable just building multiple packages at the same time; individual
    # spkgs still get built in parallel if they support it and '--jobs'
    # in $MAKE is greater than one (and at all specified):
    echo ".NOTPARALLEL:" > "$SAGE_ROOT"/spkg/standard/parallel_make.cfg
else
    # Create an (almost) empty file, s.t. including it from spkg/standard/deps
    # doesn't raise an error, thereby also invalidating any previous setting:
    echo > "$SAGE_ROOT"/spkg/standard/parallel_make.cfg
fi

time ${MAKE:-make} -f standard/deps $1

...

adding spkg/standard/parallel_make.cfg to $SAGE_ROOT/.hgignore, and adding the line

include parallel_make.cfg

to spkg/standard/deps.

comment:11 follow-up: Changed 8 years ago by jhpalmieri

How about a "make serial" target instead of the environment variable?

comment:12 in reply to: ↑ 11 Changed 8 years ago by leif

Replying to jhpalmieri:

How about a "make serial" target instead of the environment variable?

I'd say make build-serial, which just sets SAGE_PARALLEL_SPKG_BUILD=no and then does the usual thing, namely build.

Just like make rebuild sets SAGE_UPGRADING=yes and then "calls" build. ;-)

You can of course define some aliases for convenience, e.g. serial-build or build-macosx... XD

comment:13 follow-up: Changed 8 years ago by jhpalmieri

Building in parallel on Mac OS X works just fine in my experience. :p

Anyway, here are new versions of the patches.

comment:14 Changed 8 years ago by jhpalmieri

  • Summary changed from build spkgs in parallel by default to Remove the necessity to set SAGE_PARALLEL_SPKG_BUILD

comment:15 follow-up: Changed 8 years ago by jhpalmieri

Some comments about the new versions:

  • I've added a "build-serial" target in the Makefile.
  • spkg/install writes "parallel-make.cfg" in the directory "spkg", since that seems to be the current directory when the makefile "standard/deps" is executed.
  • Leif's proposed change to spkg/install regarding moving "makefile" to "Makefile" didn't seem to work on OS X, probably because of the stupid way OS X deals with upper and lower case in their standard file system. Or maybe I did something else wrong. Anyway, I didn't change that part of the script.
  • I haven't put everything possible into the top-level README file: the installation guide is supposed to have all of the details. I did add a comment about the spkg/logs/ files. See also #11926, which adds a message about spkg/logs/PKG.log to the error message printed by sage-spkg.
  • in the installation guide, I added a few comments about problems building ATLAS.

By the way, is there anything wrong with this kind of change:

  • spkg/install

    diff --git a/spkg/install b/spkg/install
    a b fi 
    483483echo "Sage build/upgrade complete!"
    484484
    485485if [ "$1" = "all" ]; then
    486     echo
    487     echo "To install small scripts to directly run Sage's versions of GAP,"
    488     echo "the PARI/GP interpreter, Maxima, or Singular etc. (by typing e.g."
    489     echo "just 'gap' or 'gp') into a standard 'bin' directory, start Sage"
    490     echo "by typing 'sage' (or './sage') and enter something like"
    491     echo
    492     echo "    install_scripts('/usr/local/bin')"
    493     echo
    494     echo "at the Sage command prompt ('sage:')."
    495     echo
    496     echo "If you issued 'make', 'make all', or a similar command, then the"
    497     echo "HTML version of the documentation will be built right now."
    498     echo "Otherwise, if you want to (re)build the HTML documentation,"
    499     echo "run 'make doc'.  To build the PDF version, run 'make doc-pdf'."
    500     echo
     486    echo "
     487To install small scripts to directly run Sage's versions of GAP,
     488the PARI/GP interpreter, Maxima, or Singular etc. (by typing e.g.
     489just 'gap' or 'gp') into a standard 'bin' directory, start Sage
     490by typing 'sage' (or './sage') and enter something like
     491
     492    install_scripts('/usr/local/bin')
     493
     494at the Sage command prompt ('sage:').
     495
     496If you issued 'make', 'make all', or a similar command, then the
     497HTML version of the documentation will be built right now.
     498Otherwise, if you want to (re)build the HTML documentation,
     499run 'make doc'.  To build the PDF version, run 'make doc-pdf'.
     500"
    501501fi

Are multiline strings not a good idea for "echo"? If this sort of thing is okay, it makes it much easier to edit the message.

Changed 8 years ago by jhpalmieri

root repo

comment:16 Changed 8 years ago by jhpalmieri

  • Description modified (diff)

Changed 8 years ago by jhpalmieri

Sage library

comment:17 in reply to: ↑ 13 Changed 8 years ago by leif

Replying to jhpalmieri:

Building in parallel on Mac OS X works just fine in my experience. :p

Incidentally I ran into "random" (non-reproducible) parallel build errors on bsd.math just yesterday, when it was btw. otherwise idling; with MAKE="make -j8".

comment:18 in reply to: ↑ 15 Changed 8 years ago by leif

Replying to jhpalmieri:

Are multiline strings not a good idea for "echo"? If this sort of thing is okay, it makes it much easier to edit the message.

echo has no problem with that, nor any reasonable shell. For larger texts, you might get problems with the size of the environment on some systems though. Note that

$ echo "
one line
"

outputs three lines, the first and last being empty.

I'd prefer cat and "here" documents though (if at all). If you want to focus on the code rather than the text, it's better to use a chain of (properly indented) echo commands.

bash also supports indentation of "here" documents (that doesn't appear in the output);

$ cat <<-EOF
...
EOF

strips leading tabs from the text.

comment:19 Changed 8 years ago by leif

  • Reviewers set to Leif Leonhardy
  • Status changed from needs_review to needs_work

make doesn't export "its" variables by default. So unless SAGE_PARALLEL_SPKG_BUILD was already defined (and exported) when make was invoked, setting it in the Makefile has no effect on programs (including shells) invoked by make. Without using further GNU make specifics, you'd have to do e.g.

build: $(PIPE) 
        cd spkg && \
        "../$(PIPE)" \
                 "env SAGE_PARALLEL_SPKG_BUILD='$(SAGE_PARALLEL_SPKG_BUILD)' ./install all 2>&1" \
                 "tee -a ../install.log" 

I <3 lengthy env var names...


It would be better (i.e., safer) to use an absolute filename in the include directive, e.g. $(SAGE_ROOT)/spkg/parallel_make.cfg.

I'd perhaps add to the comment there that .NOTPARALLEL has no effect on sub-makes.


While you're at it, the commit messages are no longer current.

comment:20 Changed 8 years ago by leif

P.S.: Using a maximal (avg) sysload of 2.5 to me seems a bit funny [even] as an example; I'd use at least the number of cores, which today is usually 4 or more.


In README.txt, grepping for "An error" is sufficient and best used with -l (which only prints the filenames of matches), or perhaps -c, which prints the number of matches in each file (unfortunately also including zero, i.e., one should postprocess that output further), since we always append to the logs and hence previous errors never vanish unless one deletes (or edits) the log.

Since the error message ("An error ...") already contains the package's name, using

$ grep -h "An error" spkg/logs/*

would also give a more readable output, as an alternative to -l, with the advantage that one immediately sees whether a package failed to install multiple times.

To get the actual error messages, (e)grepping for "Error" and "Failed" at the beginning of the line (egrep "^(Error|Failed)" spkg/logs/...1), optionally with a (non-POSIX) before-context (-B), is usually more informative. When case-insensitively ( -i) grepping for any "error", to suppress frequent errors ignored by make, one can pipe the whole through grep -iv ignored. -n also prepends the line numbers to the output, which is useful if you're going to open the log in an editor.


Do we mention LC_ALL=C (or e.g. LC_MESSAGES=en_US.utf8) for submitting error logs?


1 "^Failed" usually also gives non-fatal errors for the Python spkg ("Failed to find the necessary bits..."). In the long run, all of Sage's error messages should really start with "Error" rather than "Failed". (The new ATLAS spkg with the Python install script in contrast raises arbitrary exceptions.)

We should really in addition create a fresh temporary file at the start of each build to keep track of which packages failed to build, to be dumped at the end (in case of errors).

comment:21 Changed 8 years ago by leif

Slightly off the ticket's topic:

Wouldn't it be better to put a lot of stuff into SAGE_ROOT/TROUBLESHOOTING.txt?

People tend to not read READMEs... (presumably because they're so ubiquitous and many of them are hardly informative)

comment:22 follow-ups: Changed 8 years ago by jhpalmieri

The 2.5 number for -l came straight from the GNU make man page.

Regarding variables in the Makefile, I saw this sentence in the man page: "When make runs a recipe, variables defined in the makefile are placed into the environment of each shell." This is a bit misleading, because it only exports variables that already came from the environment, so I saw that my setting of SAGE_PARALLEL_SPKG_BUILD=yes was being overridden when I ran "make build-serial". I see now that if this variable is unset, then "make build-serial" doesn't do what it should. Why can't I just do

build-serial: export SAGE_PARALLEL_SPKG_BUILD = no
build-serial: build

Or is this too specific to GNU make? (Will our makefiles work without GNU make anyway?)

For grep, I like the output from grep -l ..., so I'll use that.

Do we mention LC_ALL=C (or e.g. LC_MESSAGES=en_US.utf8) for submitting error logs?

I don't think so, and I'm not planning to add anything about it.

Re TROUBLESHOOTING.txt: we could instead have a structure more like many GNU projects: a boilerplate README and then a file INSTALL with specific instructions.

While we're off-topic: Is the top-level install.log any use as it is right now? It would be better if it had just summary information:

All environment variables ...

and then lines like the following (probably intermixed for the different spkgs):

[time] Started building python-...spkg; keeping a log in spkg/logs/python-...log
[time] Successfully installed python-....

or

[time] Started building python-...spkg; keeping a log in spkg/logs/python-...log
[time] Error building python-...; see spkg/logs/python-... for a full log

or

[time] Started building python-...spkg; keeping a log in spkg/logs/python-...log
[time] Error testing python...; see spkg/logs/python-... for a full log

And then at the end

real	106m26.143s
user	147m9.046s
sys	21m31.998s

Sage build/upgrade complete!
...

How hard would that be?

comment:23 Changed 8 years ago by jhpalmieri

  • Status changed from needs_work to needs_review

Here are new patches.

For changing the format of install.log etc., we could write to the individual log files as part of sage-spkg, instead of when sage-spkg is called by sage-sage or spkg/install. Then spkg/install could have minimal output (or we could have a flag for setting its verbosity), which it would write directly to install.log.

sage-spkg could take an optional argument, the name of the log file, or at least whether or not to produce a log file; the default would be to append to one.

comment:24 Changed 8 years ago by jhpalmieri

  • Description modified (diff)

comment:25 in reply to: ↑ 22 Changed 8 years ago by leif

Replying to jhpalmieri:

Why can't I just do

build-serial: export SAGE_PARALLEL_SPKG_BUILD = no
build-serial: build

Well, you could. You could also add

export SAGE_PARALLEL_SPKG_BUILD

somewhere in the Makefile (preferably near the top), or use

EXPORTS = SAGE_PARALLEL_SPKG_BUILD="$(SAGE_PARALLEL_SPKG_BUILD)" \
          SAGE_FOO="$(SAGE_FOO)"

...

bar:
        env $(EXPORTS) ./baz

Or is this too specific to GNU make?

I don't care much, although I prefer not using GNUisms if there's a simple alternative. AFAIK older GNU makes don't support export either, but I haven't checked since when they do.


Do we mention LC_ALL=C (or e.g. LC_MESSAGES=en_US.utf8) for submitting error logs?

I don't think so, and I'm not planning to add anything about it.

Hmmmm. :(


Re TROUBLESHOOTING.txt: we could instead have a structure more like many GNU projects: a boilerplate README and then a file INSTALL with specific instructions.

TROUBLESHOOTING is much more explicit. Most INSTALL files shipped are just templates, so it's quite likely that people wouldn't read them either. We also have the (longer) Sage Installation Guide; I was aiming at a short plain text file purely on topic, i.e. common issues (which might get updated more frequently and easily), and how and where to report build failures / ask for build-related support.


While we're off-topic: Is the top-level install.log any use as it is right now?

I'd keep at least the file; whether it makes sense to dump it to the screen -- especially in parallel builds -- is another question.

For changing the format of install.log etc., we could write to the individual log files as part of sage-spkg, instead of when sage-spkg is called by sage-sage or spkg/install.

That's what I suggested more than a year ago. :-) Also, a large amount of the output are distutils' "copying ..." messages, and ECL (also when building Maxima) is far too verbose, too.

We don't lose much if we move the logging from deps to sage-spkg. The latter should take a couple of logging-related parameters like --logfile=, --logfd=, --loglevel=, with the possibility to specify more than one logfile or file descriptor, with the same or different log levels.

Unfortunately sage-spkg is a hot spot; it needs work in all places.

comment:26 in reply to: ↑ 22 Changed 8 years ago by leif

Replying to jhpalmieri:

For grep, I like the output from grep -l ..., so I'll use that.

grep -li ... would be better, since sage-build also has

	  echo "ERROR: There was an error building c_lib."
         echo "sage: There was an error installing modified $1 library code."

(Or just change that file, too... :P Although "Error: ..." would be better anyway.)

Changed 8 years ago by jhpalmieri

for review only

Changed 8 years ago by jhpalmieri

for review only

Changed 8 years ago by jhpalmieri

Sage library

comment:27 Changed 8 years ago by jhpalmieri

(New versions of the v2 patches, just changing "grep -l" to "grep -li".)

comment:28 Changed 8 years ago by leif

I'm happy with the v2 patches as they are, so in principle positive review. Just have to take a closer look at the whole and make sure the patches apply... ;-)

Feel free to add or change further things if you like.

[IRC distracted me; that was a while ago, but I hadn't yet grabbed the patches... :-) ]

comment:29 follow-up: Changed 8 years ago by leif

"If you have a machine with 4 processors..."

Ask Intel what the differences between a uni-processor, a dual-processor and a multi-processor machine are... ;-) (These are separate Xeon series; AMD has Athlon/Phenom? vs. Opteron. The Sage cluster machines boxen, geom, mod, redhawk and sage are all 4-processor machines, each processor a hexacore.)

(The cheap desktop and notebook ones are all single-processor multi-core machines, and I think the notion of cores is meanwhile quite common to everyone.)


If you happen to edit README.txt again, perhaps also mention sage-release, which is IMHO better suited for build errors. The lengthy standard error message of sage-spkg already mentions sage-devel (and currently only that).

comment:30 Changed 8 years ago by leif

Small typo in deps:

"# only determines whether more than one spkg may be built at a time."

Or I should just try what it builds when it builds more than spkgs... :-)

comment:31 in reply to: ↑ 29 Changed 8 years ago by leif

Replying to leif:

If you happen to edit README.txt again [...]

You could also increase the number of developers in "Over 200 people have contributed code to Sage." to 300+.

comment:32 Changed 8 years ago by kcrisman

Just in case this isn't written in stone, it would be nice to still have the full thing in install.log. One could add something about referring to the other logs, though, if it doesn't already do so, maybe especially in the case of an error in the build.

comment:33 follow-up: Changed 8 years ago by jhpalmieri

I'm curious as to how the full install.log is useful. Having it essentially doubles the amount of log space, since almost everything is in both the install.log and the individual log files. The individual log files aren't jumbled together the way they are in install.log (for parallel builds). If the whole log file just said:

  • building X
  • building Y
  • ...
  • done building Y (or "error building Y")
  • done building X
  • ...

with instructions about how to find the individual log files, doesn't that convey all of the necessary information?

comment:34 Changed 8 years ago by leif

  • Status changed from needs_review to positive_review

It's amazing how many typos are in the Installation Guide. But the documentation has really improved compared to how it was e.g. last year.

One could also add a link to the Environment Variables section (i.e., MAKE) when it comes to "make", since the steps for building from source there currently do not mention building in parallel (or make -j) at all.


As said, I'm happy with the patches as they are, and everything works as expected, so positive review.

comment:35 Changed 8 years ago by jhpalmieri

FYI: I'm updating the root patch to fix the typo in deps.

Changed 8 years ago by jhpalmieri

root repo

comment:36 in reply to: ↑ 33 ; follow-up: Changed 8 years ago by kcrisman

Replying to jhpalmieri:

I'm curious as to how the full install.log is useful. Having it essentially doubles the amount of log space, since almost

Because I'm lazy and don't want to have to dive into spkg/ to open them. Won't revert the positive review on this, though, since it will be in 4.7.3, certainly makes sense in many ways, and if anyone else wants it they'll have plenty of alphas to complain :)

comment:37 in reply to: ↑ 36 Changed 8 years ago by jhpalmieri

Replying to kcrisman:

Replying to jhpalmieri:

I'm curious as to how the full install.log is useful. Having it essentially doubles the amount of log space, since almost

Because I'm lazy and don't want to have to dive into spkg/ to open them. Won't revert the positive review on this

This ticket doesn't actually affect the contents of install.log, it was just a side discussion. The patches here do put a little more emphasis on spkg/logs/*, because I think they still aren't well known enough, but no log files were harmed by the patches for this ticket.

Changed 8 years ago by jdemeyer

Rebased on top of #11926

comment:38 Changed 8 years ago by jdemeyer

  • Dependencies set to #11926
  • Description modified (diff)

comment:39 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.7.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:40 Changed 8 years ago by jdemeyer

  • Description modified (diff)

Changed 8 years ago by jdemeyer

Rebased on top of #11926

comment:41 Changed 8 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:42 Changed 8 years ago by jdemeyer

  • Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
  • Milestone set to sage-4.8
Note: See TracTickets for help on using tickets.