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: Simplify compiler echo string during make #507

Merged
merged 3 commits into from
Sep 21, 2015

Conversation

vhda
Copy link
Contributor

@vhda vhda commented Aug 10, 2015

This was only implemented in the main Makefile and mk_mingw.

@masatake
Copy link
Member

This makes the output beautiful but sometimes I would like to see the arguments passed to compiler.
Could you prepare an option to see the arguments like:

  $ make VERBOSE=1

or

  $ make V=1

?

@masatake
Copy link
Member

BTW, "make" prefix in the subject of this issue implies "make.c" parser.
Could you find something good alternative? How about "build" or "building"?

@vhda
Copy link
Contributor Author

vhda commented Aug 11, 2015

Excellent suggestions!
Will implement as soon as possible, but it may take some days until I have the time, ok?

@masatake
Copy link
Member

Sure.
kbuild system of linux kernel can be used as interesting source of ideas.

@b4n
Copy link
Member

b4n commented Aug 11, 2015

FWIW Automake's silent feature is implemented with the non-standard yet quite portable (at least GNU and BSD makes support it) variable macro names, something like this:

# quiet by default
V ?= 0

V_CC_0 = @echo '  CC $@';
#V_CC_1 =
V_CC = $(V_CC_$(V))

V_GEN_0 = @echo ' GEN $@';
#V_GEN_1 =
V_GEN = $(V_GEN_$(V))

all: foo bar
    $(V_CC) echo "warning: Input files are empty" >&2

foo bar:
    $(V_GEN) touch $@

clean:
    rm -f foo bar

.PHONY: all clean

And then depending on the value of $(V) $(V_CC) expands either to $(V_CC_0) or to something empty (undefined macro).

@vhda
Copy link
Contributor Author

vhda commented Aug 12, 2015

@b4n In that case, can't we include that code automatically through the configure script?
I'm the John Snow of autoconf: I know nothing! ;)

@masatake
Copy link
Member

@vhda, in my understanding, this is not about autoconf.
If you want to include the code automatically, automake is for you.

@vhda
Copy link
Contributor Author

vhda commented Aug 14, 2015

I've updated the implementation of this feature as suggested by @b4n. After understanding how it works, it is a really nice idea. Thanks a lot! :)

Two doubts:

  1. The directory creation is always silent. Do you see a problem in this such that I should also make it verbose when V=1?
  2. I should probably document the use of V=1. Where should I do this?

@masatake
Copy link
Member

@vhda, thank you.

About 1. I don't care. I will modify (and make a PR) when I need.
About 2. I have a big request. If you have time, could you add help target to Makefile.in? I want you to write about $(V) in the message printed when "help" target is triggered.

About units/fuzz/noise/tmain/tinst, I will write them. What I need is a stab. The stub is important. With it we can work together.

V ?= 0
V_CC = $(V_CC_$(V))
V_CC_0 = @echo [CC] $@; $(CC)
V_CC_1 = $(CC)
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather have V_CC only expand to empty or an @echo command -- e.g. not including $(CC).
Yes, this will mean slight duplication, but it is more robust (e.g. would still work if the user sets V=2) and allows to have preliminary commands hidden by it, like your mkdir below.

@vhda
Copy link
Contributor Author

vhda commented Sep 16, 2015

Could you guys check if the new implementation fills all requirements you mentioned in your comments before?
I currently have two separate commits so that you can compare the files. I'll squash them or drop the last according to your feedback.

@vhda vhda assigned vhda and masatake and unassigned vhda Sep 16, 2015
@b4n
Copy link
Member

b4n commented Sep 16, 2015

The technique to silent LGTM.

Whether all what you silenced should be silenced I don't know, but why not.

@vhda
Copy link
Contributor Author

vhda commented Sep 16, 2015

@b4n you proposed the approach in the second commit, do you still prefer that when compared with the first commit?

@b4n
Copy link
Member

b4n commented Sep 16, 2015

yes

@b4n
Copy link
Member

b4n commented Sep 16, 2015

My question was whether it was a good idea to silent some things, i.e. ./config.status or autoreconf -vi calls, that will generate a lot of output themselves anyway. Similarly, silencing clean operation seem mildly useful to me, as it only happens once and generally result from an explicit call.

This doesn't mean you shouldn't keep silencing them, just that I'm not sure it's very important. And remember that while it gives a more readable output, it also gives less info: a bug report with silenced output is less useful than one with all the verbose details.

@vhda
Copy link
Contributor Author

vhda commented Sep 16, 2015

We can still close this PR if you feel that this does not bring any advantages.
And I can also remove some of the $(SILENT) calls.

@b4n
Copy link
Member

b4n commented Sep 16, 2015

We can still close this PR if you feel that this does not bring any advantages.

I think it does at least for the compilation phase, as it makes it a lot easier to see what is being built, and removes clutter around messages from the compiler -- and most of the time a developer is working on the code, not debugging the build phase.

But I also think silencing everything might not make sense, if it doesn't provide a specific advantage. However, I don't mind much and one can always change the value of V when calling make, so it won't be any kind of real issue.

@vhda
Copy link
Contributor Author

vhda commented Sep 16, 2015

I'm fine with any solution, so let's see what's @masatake 's opinion and we'll move on from there.

@masatake
Copy link
Member

If you provide "help" target where V is also explained, I would like to merge the change.

@vhda
Copy link
Contributor Author

vhda commented Sep 16, 2015

My idea was to create a separate PR for this change, but there's no harm in including it here.

@p-montanus
Copy link
Contributor

FWIW Automake's silent feature is implemented with the non-standard yet quite portable (at least GNU and BSD makes support it) variable macro names, something like this:

After automake 1.11.3, nested make variables are not used directly. [1][2][3]

[1] http://git.savannah.gnu.org/cgit/automake.git/tree/bin/automake.in?id=6357a630dc3cac6682a0f17b255104b4dd78f89a#n1034
[2] http://git.savannah.gnu.org/cgit/automake.git/tree/NEWS?id=6357a630dc3cac6682a0f17b255104b4dd78f89a#n1381
[3] http://git.savannah.gnu.org/cgit/automake.git/commit/?id=8493499b54da3694f820fa7eab5e58ef44448b66

Before using nested variable, automake check whether make supports nested variables. [4]
[4] http://git.savannah.gnu.org/cgit/automake.git/tree/m4/silent.m4?id=6357a630dc3cac6682a0f17b255104b4dd78f89a#n31

In current automake, AM_V_GEN* vars are defind in Makefile.in as:

# Makefile.in
AM_V_GEN = $(am__v_GEN_@AM_V@)
am__v_GEN_ = $(am__v_GEN_@AM_DEFAULT_V@)
am__v_GEN_0 = @echo "  GEN     " $@;
am__v_GEN_1 = 

@AM_V@ and @AM_DEFAULT_V@ are replaced by configure.[5]
[5] http://git.savannah.gnu.org/cgit/automake.git/tree/m4/silent.m4?id=6357a630dc3cac6682a0f17b255104b4dd78f89a#n52

(A) when make supports nested variable, replaced as:

# Makefile (w/ nested-vars)
AM_V_GEN = $(am__v_GEN_$(V))
am__v_GEN_ = $(am__v_GEN_$(AM_DEFAULT_VERBOSITY))
am__v_GEN_0 = @echo "  GEN     " $@;
am__v_GEN_1 = 

AM_DEFAULT_VERBOSITY contains '0' or '1', is controlled by configure '--enable/disable-silent-rules' options. [6]
[6] http://git.savannah.gnu.org/cgit/automake.git/tree/m4/silent.m4?id=6357a630dc3cac6682a0f17b255104b4dd78f89a#n21

V is not defined in Makefie/Makefile.in.

(B-1) when make does not supports nested variable, and '--enable-silent-rules' specified:

# Makefile (w/o nested-vars, --enable-silent-rules)
AM_V_GEN = $(am__v_GEN_0)
am__v_GEN_ = $(am__v_GEN_0)
am__v_GEN_0 = @echo "  GEN     " $@;
am__v_GEN_1 = 

this case, V is not verbosity control variable.

(B-2) when make does not supports nested variable, and '--disable-silent-rules' specified:

# Makefile (w/o nested-vars, --disable-silent-rules)
AM_V_GEN = $(am__v_GEN_1)
am__v_GEN_ = $(am__v_GEN_1)
am__v_GEN_0 = @echo "  GEN     " $@;
am__v_GEN_1 = 

this case, V is not verbosity control variable, too.

This was only implemented in the main Makefile and mk_mingw.
The normal verbose output can be enabled by calling make as following:

    make V=1
@b4n
Copy link
Member

b4n commented Sep 18, 2015

@p-montanus nice, thanks for the details :)
Though, in our case AFAIK we already use some GNU make features, so we probably don't really need to worry about that. Tho we could do the same if we want (given we can easily detect whether Make supports nested vars, which might not be trivial, I don't know).

@vhda
Copy link
Contributor Author

vhda commented Sep 19, 2015

Should this patch be merged?

@masatake
Copy link
Member

@vhda, yes, please, merge them.

vhda added a commit that referenced this pull request Sep 21, 2015
make: Simplify compiler echo string during make
@vhda vhda merged commit c13d9dd into universal-ctags:master Sep 21, 2015
@vhda vhda deleted the main/make-simple-echo branch September 21, 2015 14:06
@masatake
Copy link
Member

I'm looking new output. Really nice.
help target is really useful. Thank you.

@vhda
Copy link
Contributor Author

vhda commented Sep 24, 2015

Thanks for the positive feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants