Opened 5 years ago

Closed 5 years ago

#22492 closed enhancement (fixed)

Repair 'V=0 make' broken by #22418 and make 'make V=0' behave the same as 'V=0 make'

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-8.0
Component: build Keywords:
Cc: jdemeyer, jhpalmieri, dimpase Merged in:
Authors: Jeroen Demeyer Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 1e2d3bb (Commits, GitHub, GitLab) Commit: 1e2d3bbb9875dcefd003d5dfbc6e257bc7ca4e92
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

This is a follow-up on #22418, which broke the V=0 make feature.

This ticket repairs it. Also, it makes make V=0 behave the same as V=0 make, in that the logfiles always contain V=1 logs.

Explanation: Setting V=1 in the sub-makes works for V=0 make but not for make V=0 because in the latter case MAKEFLAGS is set to V=0 and can't be overridden by the environment variable. Need to edit MAKEFLAGS as well.

Change History (15)

comment:1 Changed 5 years ago by mkoeppe

  • Branch set to u/mkoeppe/make__make_v_0__behave_the_same_as__v_0_make_

comment:2 Changed 5 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to fbf0bd8ca84717c155f8471a7bcb01ff144e17ff
  • Milestone changed from sage-7.6 to sage-8.0
  • Status changed from new to needs_review

New commits:

fbf0bd8Make 'make V=0' behave the same as 'V=0 make'

comment:3 Changed 5 years ago by mkoeppe

  • Cc jhpalmieri added

comment:4 Changed 5 years ago by mkoeppe

  • Cc dimpase added

comment:5 Changed 5 years ago by mkoeppe

  • Description modified (diff)
  • Summary changed from Make 'make V=0' behave the same as 'V=0 make' to Repair 'V=0 make' broken by #22418 and make 'make V=0' behave the same as 'V=0 make'

comment:6 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Why these conditions for setting MAKEFLAGS?

if [[ $use_prefix = true ]]; then

and

if [[ -n "$MAKEFLAGS" ]];

I would unconditionally always set export MAKEFLAGS="$MAKEFLAGS V=1".

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:7 follow-up: Changed 5 years ago by mkoeppe

$use_prefix distinguishes between the top-level logger (logging to install.log) and the nested loggers (for each package). The top-level logger should not set V=1. (This is how #22418 broke the V=0 feature.)

Regarding iff [[ -n "$MAKEFLAGS" ]];: This is not important, but my reasoning here is that nothing in the sage-logger is really make-specific, so it shouldn't touch such variables unless it has to.

comment:8 Changed 5 years ago by mkoeppe

  • Status changed from needs_info to needs_review

comment:9 in reply to: ↑ 7 Changed 5 years ago by jdemeyer

Replying to mkoeppe:

The top-level logger should not set V=1. (This is how #22418 broke the V=0 feature.)

That's not really correct. The correct statement is "only the bottom-level logger should set V=1". There can be situations where there is only one level and the top-level logger is also the bottom-level logger.

comment:10 follow-up: Changed 5 years ago by mkoeppe

I don't know about such situations. In any case this just restores the status before #22418, which seemed to work fine.

comment:11 in reply to: ↑ 10 Changed 5 years ago by jdemeyer

Replying to mkoeppe:

I don't know about such situations.

For example, when you run ./sage -i $PKGNAME. In that case, there will be only one level of logger.

comment:12 Changed 5 years ago by jdemeyer

  • Branch changed from u/mkoeppe/make__make_v_0__behave_the_same_as__v_0_make_ to u/jdemeyer/make__make_v_0__behave_the_same_as__v_0_make_

comment:13 Changed 5 years ago by jdemeyer

  • Authors changed from Matthias Koeppe to Matthias Koeppe, Jeroen Demeyer
  • Commit changed from fbf0bd8ca84717c155f8471a7bcb01ff144e17ff to 1e2d3bbb9875dcefd003d5dfbc6e257bc7ca4e92

I think this is a better solution to the original problem.


New commits:

6c05f14Remove redundant V=1 options from build rules
1e2d3bbNew variable SAGE_SILENT_BUILD to implement V=0

comment:14 Changed 5 years ago by mkoeppe

  • Authors changed from Matthias Koeppe, Jeroen Demeyer to Jeroen Demeyer
  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

This seems to work fine for me. I agree with this implementation.

comment:15 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/make__make_v_0__behave_the_same_as__v_0_make_ to 1e2d3bbb9875dcefd003d5dfbc6e257bc7ca4e92
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.