make V=0 should silence the build #21539

mkoeppe · 2016-09-19T04:15:04Z

Building sage outputs a lot. Especially in highly parallelized builds, the output is not very useful.

This ticket implements an option to silence the build, using make argument (or environment variable) V=0. (This mimics the interface of Automake silent rules; so this is one step towards #21566):

$ make -j8 V=0 autotools
make -j8 build/make/Makefile
make[1]: warning: -jN forced in submake: disabling jobserver mode.
make[1]: `build/make/Makefile' is up to date.
*** ALL ENVIRONMENT VARIABLES BEFORE BUILD: ***
... long list of environment variables elided -- this ticket does nothing about that ...
***********************************************
make[1]: warning: -jN forced in submake: disabling jobserver mode.
Running 'sage-spkg xz-5.2.2', output appears in /Users/mkoeppe/cvs/sage/logs/pkgs/xz-5.2.2.log...
Running 'sage-spkg xz-5.2.2', output appears in /Users/mkoeppe/cvs/sage/logs/pkgs/xz-5.2.2.log... done
Running 'sage-spkg autotools-20141105', output appears in /Users/mkoeppe/cvs/sage/logs/pkgs/autotools-20141105.log...
=========================== WARNING ===========================
You are about to download and install the experimental package
autotools-20141105.  This probably won't work at all for you! There
is no guarantee that it will build correctly, or behave as 
expected. Use at your own risk!
===============================================================
Are you sure you want to continue [Y/n]? 
OK, installing autotools-20141105 now.
Running 'sage-spkg autotools-20141105', output appears in /Users/mkoeppe/cvs/sage/logs/pkgs/autotools-20141105.log... exit status 1
make[1]: *** [/Users/mkoeppe/cvs/sage/local/var/lib/sage/installed/autotools-20141105] Error 1

Note that, though all output is redirected, the interactive questions (#20884, #21082) still go through to the terminal. This is achieved by using /dev/tty.

The patch also unconditionally silences a few make recipe lines involving sage-logger.

Follow-up ticket: #21589

CC: @embray @jdemeyer @vbraun @videlec @dimpase @nexttime @kcrisman @novoselt @kiwifb

Component: build

Author: Matthias Koeppe

Branch: 740ecfb

Reviewer: John Palmieri

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

The text was updated successfully, but these errors were encountered:

mkoeppe · 2016-09-24T21:34:16Z

Branch: u/mkoeppe/make_v_0_should_silence_the_build

mkoeppe · 2016-09-24T21:52:35Z

New commits:

`75223db`	`Silence some make rules`
`0361466`	`Use /dev/tty for interaction`
`2322391`	`If V=0, redirect (not tee) to logfile`

mkoeppe · 2016-09-24T21:52:35Z

Commit: 2322391

mkoeppe · 2016-09-24T21:52:35Z

Author: Matthias Koeppe

mkoeppe · 2016-09-27T18:52:19Z

comment:6

Needs review.

jdemeyer · 2016-09-27T20:06:18Z

comment:7

I think the /dev/tty thing should be a separate ticket, since it's very specific and might require a specific reviewer who knows what it is about.

jdemeyer · 2016-09-27T20:11:36Z

comment:8

I really dislike @ in Makefiles (unless the rule is truly trivial). It has happened to me several times that I was debugging some build issue and the @ was hiding important stuff.

mkoeppe · 2016-09-27T20:28:14Z

comment:9

Replying to @jdemeyer:

I think the /dev/tty thing should be a separate ticket, since it's very specific and might require a specific reviewer who knows what it is about.

If Erik doesn't mind, I could repurpose #21082 for that.

sagetrac-git · 2016-09-27T21:08:11Z

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

`04aa4f7`	`Merge tag '7.4.beta6' into t/21539/make_v_0_should_silence_the_build`
`850eb86`	`Only silence rules when V=0`

sagetrac-git · 2016-09-27T21:08:11Z

Changed commit from 2322391 to 850eb86

mkoeppe · 2016-09-27T21:09:29Z

comment:11

Replying to @jdemeyer:

I really dislike @ in Makefiles (unless the rule is truly trivial). It has happened to me several times that I was debugging some build issue and the @ was hiding important stuff.

OK, I've changed it so this is only done in V=0 mode.

jhpalmieri · 2016-09-27T22:09:39Z

comment:12

Two comments:

in the past (see decrease the size of SAGE_ROOT/install.log #12729, for example), there were objections (by Jeroen, and maybe others) to not producing the full install.log file as is done now: this can be useful for debugging race conditions.
I think the output hard to read. I would like something like the proposed output at decrease the size of SAGE_ROOT/install.log #12729 better. That is, change

Running 'sage-spkg mpc-1.0.3.p0', output appears in /path/to/SAGE_ROOT/log/pkgs/mpc-1.0.3.p0.log ...
Running 'sage-spkg mpc-1.0.3.p0', output appears in /path/to/SAGE_ROOT/log/pkgs/mpc-1.0.3.p0.log ... done

to something like

[mpc-1.0.3.p0] installing. Log file: logs/pkgs/mpc-1.0.3.p0.log
  [mpc-1.0.3.p0] installed successfully.

The path to the log file should be given relative to SAGE_ROOT, which shortens the message, making it less likely that the line wraps, since line-wrapping is part of what makes the output hard to read.

jhpalmieri · 2016-09-27T22:49:53Z

comment:13

For the output, I'm thinking of something like this change (although I cheat in constructing the shortened path for the logfile here -- there should be a more robust solution):

diff --git a/build/bin/sage-logger b/build/bin/sage-logger
index c6afae1..de6cff8 100755
--- a/build/bin/sage-logger
+++ b/build/bin/sage-logger
@@ -57,15 +57,15 @@ mkdir -p "$logdir"
 if [[ "$V" = 0 && $use_prefix = true ]]; then
     # Silent build.
     # Similar to https://www.gnu.org/software/automake/manual/html_node/Automake-Silent-Rules.html#Automake-Silent-Rules
-    echo "Running '$cmd', output appears in $logfile..."
+    echo "[$logname] installing. Log file: logs/pkg/$logname"
     # Use verbose mode for output to logfiles.
     export V=1
     ( exec> $logfile 2>&1 ; eval "$cmd" )
     status=$?
     if [[ $status != 0 ]]; then
-       echo "Running '$cmd', output appears in $logfile... exit status $status"
+       echo "  [$logname] error installing, exit status $status. Log file: $logfile"
     else
-       echo "Running '$cmd', output appears in $logfile... done"
+       echo "  [$logname] successfully installed"
     fi
     exit $status
 else

mkoeppe · 2016-09-27T22:54:04Z

comment:14

Feel free to push (a more robust version of) this to the ticket

jdemeyer · 2016-09-28T08:38:14Z

comment:15

Replying to @jhpalmieri:

in the past (see decrease the size of SAGE_ROOT/install.log #12729, for example), there were objections (by Jeroen, and maybe others) to not producing the full install.log file as is done now: this can be useful for debugging race conditions.

Right. However, in this case the logs are still there by default. This ticket will only affect people who explicitly set V=0.

jhpalmieri · 2016-09-28T16:53:50Z

Changed branch from u/mkoeppe/make_v_0_should_silence_the_build to u/jhpalmieri/make_v_0_should_silence_the_build

jhpalmieri · 2016-09-28T17:06:56Z

Changed commit from 850eb86 to 57faa5b

jhpalmieri · 2016-09-28T17:06:56Z

comment:17

Here's a branch that makes the easy parts of those changes but does not change how the log file is printed.

New commits:

`57faa5b`	`trac 21539: change format of output to screen`

jhpalmieri · 2016-09-28T17:57:03Z

comment:18

I would also be tempted to silence the sage-logger commands to build each package when V=0:

--- configure	2016-09-27 16:34:53.000000000 -0700
+++ configure	2016-09-28 10:47:33.000000000 -0700
@@ -7311,7 +7311,7 @@
     else
         # Normal Sage packages
         echo >&7 "\$(INST)/$PKG_VERSION:$DEPS"
-        echo >&7 "	+sage-logger -p '\$(SAGE_SPKG) $PKG_VERSION' '\$(SAGE_LOGS)/$PKG_VERSION.log'"
+        echo >&7 "	\$(AM_V_at)+sage-logger -p '\$(SAGE_SPKG) $PKG_VERSION' '\$(SAGE_LOGS)/$PKG_VERSION.log'"
         echo >&7
 
         # Add a target with just the bare package name for "sage -i"

I don't know anything about configure scripts or how they are produced, so I don't know what would be required to make this change. Is it even a good idea?

mkoeppe · 2016-09-28T19:03:16Z

comment:19

Configure.ac already does that.
Just do sage -i autotools and the regenerate the script using autoreconf.

jhpalmieri · 2016-09-28T22:32:39Z

comment:20

Okay, so I assume that if this is merged, configure will automatically be updated.

This doesn't silence warnings, maybe it's not silencing STDERR, only STDOUT. So I see messages on the screen like

make[3]: warning: -jN forced in submake: disabling jobserver mode.
make[3]: `/Users/jpalmier/Desktop/Sage_stuff/sage_builds/TESTING/sage/local/var/lib/sage/installed/zlib-1.2.8.p0' is up to date.

I want to see warnings if there is a serious problem, but it would be nice if I didn't have to see these minor ones.

I'm also seeing many lines of the form

cp /Users/jpalmier/Desktop/Sage_stuff/sage_builds/TESTING/sage/src/ext/doctest/invalid/syntax_error.tachyon /Users/jpalmier/Desktop/Sage_stuff/sage_builds/TESTING/sage/local/share/sage/ext/doctest/invalid/syntax_error.tachyon

I guess they come from the target $(SAGE_EXTCODE). I think that output should be silenced, too, or at least abbreviated ("[sage-extcode] installing...").

jdemeyer · 2016-09-29T06:08:07Z

comment:21

Replying to @jhpalmieri:

Okay, so I assume that if this is merged, configure will automatically be updated.

The release manager takes care of this.

mkoeppe · 2016-09-29T06:37:10Z

comment:22

Replying to @jhpalmieri:

I see messages on the screen like
make[3]: warning: -jN forced in submake: disabling jobserver mode.

I think this indicates we are using $(MAKE) or $(MAKEFLAGS) wrong.

mkoeppe · 2016-09-29T06:53:50Z

comment:23

I've created #21610 for that.

mkoeppe · 2016-09-29T06:56:01Z

comment:24

To silence even more make output, one can of course do make -s V=0.

mkoeppe · 2016-10-03T18:35:36Z

Changed branch from u/jhpalmieri/make_v_0_should_silence_the_build to u/mkoeppe/make_v_0_should_silence_the_build

mkoeppe · 2016-10-03T18:37:47Z

Changed commit from 57faa5b to 1d033ad

mkoeppe · 2016-10-03T18:37:47Z

comment:26

Replying to @jhpalmieri:

I'm also seeing many lines of the form
cp /Users/jpalmier/Desktop/Sage_stuff/sage_builds/TESTING/sage/src/ext/doctest/invalid/syntax_error.tachyon /Users/jpalmier/Desktop/Sage_stuff/sage_builds/TESTING/sage/local/share/sage/ext/doctest/invalid/syntax_error.tachyon
I guess they come from the target $(SAGE_EXTCODE). I think that output should be silenced, too, or at least abbreviated ("[sage-extcode] installing...").

I've added some more $(AM_V_at) for that.

New commits:

`1d033ad`	`If V=0, silence 'cp' for scripts and extcode`

mkoeppe · 2016-10-09T18:13:43Z

comment:27

Needs review.

jhpalmieri · 2016-10-10T23:15:23Z

comment:28

When I use make V=0, the log files seem to be overwritten, not appended to. For example, sagelib-7.4.beta6.log contains a lot of information before docbuilding, but then after docbuilding it gets overwritten and is nearly empty at the end. To reproduce:

$ make   # produces full log files
$ make V=0   # overwrites at least sagelib-...log and dochtml.log

sagetrac-git · 2016-10-11T01:03:54Z

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

`119b34e`	`Merge tag '7.4.rc0' into t/21539/make_v_0_should_silence_the_build`
`740ecfb`	`In V=0 mode, append to logfiles`

sagetrac-git · 2016-10-11T01:03:54Z

Changed commit from 1d033ad to 740ecfb

mkoeppe · 2016-10-11T16:20:15Z

comment:30

Thanks for catching this. Fixed.

jhpalmieri · 2016-10-11T16:28:05Z

comment:31

This looks okay to me now. Jeroen, any objections to a positive review?

jhpalmieri · 2016-10-18T04:15:26Z

Reviewer: John Palmieri

jhpalmieri · 2016-10-18T04:15:26Z

comment:32

Jeroen, please add yourself as a reviewer if you would like.

vbraun · 2016-10-21T07:04:08Z

Changed branch from u/mkoeppe/make_v_0_should_silence_the_build to 740ecfb

jdemeyer · 2017-02-23T07:55:33Z

comment:34

Related: #22418

jdemeyer · 2017-02-23T07:55:33Z

Changed commit from 740ecfb to none

mkoeppe added this to the sage-7.4 milestone Sep 19, 2016

mkoeppe added c: build labels Sep 19, 2016

This comment has been minimized.

Sign in to view

mkoeppe added the s: needs review label Sep 24, 2016

jhpalmieri added s: needs work and removed s: needs review labels Oct 10, 2016

mkoeppe added s: needs review and removed s: needs work labels Oct 11, 2016

jhpalmieri added s: positive review and removed s: needs review labels Oct 18, 2016

vbraun removed the s: positive review label Oct 21, 2016

vbraun closed this as completed in a7e687e Oct 21, 2016

jdemeyer mentioned this issue Mar 2, 2017

Always use V=1 for logfiles #22418

Closed

make V=0 should silence the build #21539

make V=0 should silence the build #21539

Comments

mkoeppe commented Sep 19, 2016

This comment has been minimized.

mkoeppe commented Sep 24, 2016

mkoeppe commented Sep 24, 2016

mkoeppe commented Sep 24, 2016

mkoeppe commented Sep 24, 2016

This comment has been minimized.

mkoeppe commented Sep 27, 2016

jdemeyer commented Sep 27, 2016

jdemeyer commented Sep 27, 2016

mkoeppe commented Sep 27, 2016

sagetrac-git mannequin commented Sep 27, 2016

sagetrac-git mannequin commented Sep 27, 2016

mkoeppe commented Sep 27, 2016

jhpalmieri commented Sep 27, 2016

jhpalmieri commented Sep 27, 2016

mkoeppe commented Sep 27, 2016

jdemeyer commented Sep 28, 2016

jhpalmieri commented Sep 28, 2016

jhpalmieri commented Sep 28, 2016

jhpalmieri commented Sep 28, 2016

jhpalmieri commented Sep 28, 2016

mkoeppe commented Sep 28, 2016

jhpalmieri commented Sep 28, 2016

jdemeyer commented Sep 29, 2016

mkoeppe commented Sep 29, 2016

mkoeppe commented Sep 29, 2016

mkoeppe commented Sep 29, 2016

mkoeppe commented Oct 3, 2016

mkoeppe commented Oct 3, 2016

mkoeppe commented Oct 3, 2016

mkoeppe commented Oct 9, 2016

jhpalmieri commented Oct 10, 2016

sagetrac-git mannequin commented Oct 11, 2016

sagetrac-git mannequin commented Oct 11, 2016

mkoeppe commented Oct 11, 2016

jhpalmieri commented Oct 11, 2016

jhpalmieri commented Oct 18, 2016

jhpalmieri commented Oct 18, 2016

vbraun commented Oct 21, 2016

jdemeyer commented Feb 23, 2017

jdemeyer commented Feb 23, 2017