Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make make V=1 print ECHO lines in Makefile and *.mk. #5875

Closed
wants to merge 1 commit into from

Conversation

junaruga
Copy link
Member

@junaruga junaruga commented May 2, 2022

This PR fixes https://bugs.ruby-lang.org/issues/18756 .


Currently make V=1 doesn't print ECHO lines unlike make Q=.
E.g. The make main.o shows the following result in both cases.

  • In case of make Q=, set ECHO1=0, ECHO0=echo, ECHO=@echo.

    $ make Q= main.o
    compiling ./main.c
    ...
    
  • In case of make V=1, set ECHO1=:, ECHO0=:, ECHO=@:.

    $ make V=1 main.o
    ...
    

As V=1 means verbose mode, we think it should print the ECHO lines too.

This commit makes make V=1 set ECHO1=0, ECHO0=echo, ECHO=@echo.

$ make V=1 main.o
compiling ./main.c
...

[ruby-core:18756]


I tested the following commands for both before and after this PR, and confirmed the only intended ECHO lines are printed after this PR.

$ make V=1
$ make install V=1

Currently `make V=1` doesn't print `ECHO` lines unlike `make Q=`.
E.g. The `make main.o` shows the following result in both cases.

* In case of `make Q=`, set `ECHO1=0`, `ECHO0=echo`, `ECHO=@echo`.
  ```
  $ make Q= main.o
  compiling ./main.c
  ...
  ```
* In case of `make V=1`, set `ECHO1=:`, `ECHO0=:`, `ECHO=@:`.

  ```
  $ make V=1 main.o
  ...
  ```

As `V=1` means verbose mode, we think it should print the `ECHO` lines too.

This commit makes `make V=1` set `ECHO1=0`, `ECHO0=echo`, `ECHO=@echo`.

[ruby-core:18756]
@nobu
Copy link
Member

nobu commented May 3, 2022

We disabled ECHO lines because the commands are displayed.
Why do you need both?

@junaruga
Copy link
Member Author

junaruga commented May 3, 2022

We disabled ECHO lines because the commands are displayed.
Why do you need both?

I can understand your intent that we don't want to print unnecessary logs. But the reason is because it is not natural to see that the verbose mode (= a kind of log level: "debug" mode) is not printing logs in the non-verbose mode (= a kind of log level: "info" mode).

For example, in ssh command, here is a kind of log level "info" mode.

$ ssh <host> echo a
a

Here is the verbose level 1 mode, a kind of log level "debug1" mode that prints both "info" and "debug1" logs.

$ ssh -v <host> echo a
...
debug1: Sending command: echo a
a
...

Here is the verbose level 2 mode, a kind of log level "debug2" mode that prints "info", "debug1" and "debug2" logs.

$ ssh -vv <host> echo a
...
debug1: Sending command: echo a
...
debug2: chan_shutdown_write: channel 0: (i3 o1 sock -1 wfd 5 efd 6 [write])
a
...

I wanted to see the logs of non-verbose mode in verbose mode.

@@ -5,8 +5,7 @@ NULLCMD = @NULLCMD@
silence = no # yes/no
yes_silence = $(silence:no=)
no_silence = $(silence:yes=)
n=$(NULLCMD)
ECHO1 = $(V:1=$n)
ECHO1 = $(V:1=0)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this PR's function is approved, this line can be changed more.

ECHO1 = $(V:1=0)

Maybe it means if V is 1, the ECHO1 is set as 0, and if V is 0 (not equal 1), ECHO1 is set as V (= 0). It might be better for the line to be changed as follows.

ECHO1 = 0

@junaruga
Copy link
Member Author

junaruga commented May 3, 2022

I can see this behavior printing text ...ing ... for V=0 and the compile line for V=1 is not only ruby/ruby Makefile, but also Makefile created by mkmf.rb (ruby extconf.rb).

ruby/lib/mkmf.rb

Line 1977 in cf71e5f

ECHO1 = $(V:1=@ #{CONFIG['NULLCMD']})

@junaruga
Copy link
Member Author

junaruga commented May 4, 2022

I would close this PR as we can use make V=1 ECHO0=echo and make install V=1 ECHO0=echo.

Ref: https://bugs.ruby-lang.org/issues/18756#note-8

@junaruga junaruga closed this May 4, 2022
@junaruga junaruga deleted the wip/make-v1-print-echo branch May 12, 2022 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants