Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#21539 closed enhancement (fixed)

make V=0 should silence the build

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-7.4
Component: build Keywords:
Cc: embray, jdemeyer, vbraun, vdelecroix, dimpase, leif, kcrisman, novoselt, fbissey Merged in:
Authors: Matthias Koeppe Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 740ecfb (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by mkoeppe)

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

Change History (34)

comment:1 Changed 4 years ago by mkoeppe

  • Cc embray added
  • Description modified (diff)

comment:2 Changed 4 years ago by mkoeppe

  • Branch set to u/mkoeppe/make_v_0_should_silence_the_build

comment:3 Changed 4 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc jdemeyer vbraun vdelecroix dimpase leif added
  • Commit set to 23223919aee6b54835de648ce65f471a177385c5
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

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

comment:4 Changed 4 years ago by mkoeppe

  • Cc kcrisman novoselt added

comment:5 Changed 4 years ago by mkoeppe

  • Cc fbissey added

comment:6 Changed 4 years ago by mkoeppe

Needs review.

comment:7 follow-up: Changed 4 years ago by 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.

comment:8 follow-up: Changed 4 years ago by 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.

comment:9 in reply to: ↑ 7 Changed 4 years ago by mkoeppe

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.

comment:10 Changed 4 years ago by git

  • Commit changed from 23223919aee6b54835de648ce65f471a177385c5 to 850eb86a8e4e15d0761e49e2eb660a0020f1c86e

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

04aa4f7Merge tag '7.4.beta6' into t/21539/make_v_0_should_silence_the_build
850eb86Only silence rules when V=0

comment:11 in reply to: ↑ 8 Changed 4 years ago by mkoeppe

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.

comment:12 follow-up: Changed 4 years ago by jhpalmieri

Two comments:

  • in the past (see #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 #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.

comment:13 Changed 4 years ago by jhpalmieri

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

  • build/bin/sage-logger

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

comment:14 Changed 4 years ago by mkoeppe

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

Version 0, edited 4 years ago by mkoeppe (next)

comment:15 in reply to: ↑ 12 Changed 4 years ago by jdemeyer

Replying to jhpalmieri:

  • in the past (see #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.

comment:16 Changed 4 years ago by jhpalmieri

  • Branch changed from u/mkoeppe/make_v_0_should_silence_the_build to u/jhpalmieri/make_v_0_should_silence_the_build

comment:17 Changed 4 years ago by jhpalmieri

  • Commit changed from 850eb86a8e4e15d0761e49e2eb660a0020f1c86e to 57faa5bbff420bc7f1cfcd89c2afb7d2da4bd533

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


New commits:

57faa5btrac 21539: change format of output to screen

comment:18 Changed 4 years ago by jhpalmieri

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

  • configure

     
    73117311    else
    73127312        # Normal Sage packages
    73137313        echo >&7 "\$(INST)/$PKG_VERSION:$DEPS"
    7314         echo >&7 "      +sage-logger -p '\$(SAGE_SPKG) $PKG_VERSION' '\$(SAGE_LOGS)/$PKG_VERSION.log'"
     7314        echo >&7 "      \$(AM_V_at)+sage-logger -p '\$(SAGE_SPKG) $PKG_VERSION' '\$(SAGE_LOGS)/$PKG_VERSION.log'"
    73157315        echo >&7
    73167316
    73177317        # 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?

comment:19 Changed 4 years ago by mkoeppe

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

comment:20 follow-ups: Changed 4 years ago by jhpalmieri

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

comment:21 in reply to: ↑ 20 Changed 4 years ago by jdemeyer

Replying to jhpalmieri:

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

The release manager takes care of this.

comment:22 in reply to: ↑ 20 Changed 4 years ago by mkoeppe

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.

comment:23 Changed 4 years ago by mkoeppe

I've created #21610 for that.

comment:24 Changed 4 years ago by mkoeppe

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

comment:25 Changed 4 years ago by mkoeppe

  • Branch changed from u/jhpalmieri/make_v_0_should_silence_the_build to u/mkoeppe/make_v_0_should_silence_the_build

comment:26 in reply to: ↑ 20 Changed 4 years ago by mkoeppe

  • Commit changed from 57faa5bbff420bc7f1cfcd89c2afb7d2da4bd533 to 1d033adc4ffe636b6282d95e1ff6414d3520a072

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:

1d033adIf V=0, silence 'cp' for scripts and extcode

comment:27 Changed 4 years ago by mkoeppe

Needs review.

comment:28 Changed 4 years ago by jhpalmieri

  • Status changed from needs_review to needs_work

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

comment:29 Changed 4 years ago by git

  • Commit changed from 1d033adc4ffe636b6282d95e1ff6414d3520a072 to 740ecfb6fc6b70834fffad30d0d3d1ee45291042

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

119b34eMerge tag '7.4.rc0' into t/21539/make_v_0_should_silence_the_build
740ecfbIn V=0 mode, append to logfiles

comment:30 Changed 4 years ago by mkoeppe

  • Status changed from needs_work to needs_review

Thanks for catching this. Fixed.

comment:31 Changed 4 years ago by jhpalmieri

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

comment:32 Changed 4 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

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

comment:33 Changed 4 years ago by vbraun

  • Branch changed from u/mkoeppe/make_v_0_should_silence_the_build to 740ecfb6fc6b70834fffad30d0d3d1ee45291042
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:34 Changed 3 years ago by jdemeyer

  • Commit 740ecfb6fc6b70834fffad30d0d3d1ee45291042 deleted

Related: #22418

Note: See TracTickets for help on using tickets.