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

Makefile.include: add verbosity control (make V=1 for old behaviour) #305

Merged
merged 1 commit into from Jul 28, 2013

Conversation

wpwrak
Copy link
Contributor

@wpwrak wpwrak commented Jul 28, 2013

The Contiki build is rather chatty, which makes it very difficult to spot compiler warnings. This adds short $(CC) and $(AR) invocations that don't show the entire command. The old behaviour can be obtained by running make with V=1.

I suggested this on the mailing list a few weeks ago but didn't get a response.

…our)

This shortens $(CC) and $(AR) lines to a much more readable length,
making warnings stick out clearly.

The spaces after "CC" and "AR" are to reserve space for other operations
that may use longer names, such as the communly found "BUILD" or
"GENERATE".
@adamdunkels
Copy link
Member

Looks like a good idea! Do you have an example of what the shorter output looks like? I wasn't sure exactly what the Makefile lines would amount too.

@wpwrak
Copy link
Contributor Author

wpwrak commented Jul 28, 2013

Here's an example of a run: http://pastebin.com/5SxQbukW

I stole the idea from the Linux kernel, which has had this sort of verbosity control for years. Many other projects do, too.

@adamdunkels
Copy link
Member

Love it. Very nice how the warnings stand out like that. A big 👍 from me.

adamdunkels added a commit that referenced this pull request Jul 28, 2013
Makefile.include: add verbosity control (make V=1 for old behaviour)
@adamdunkels adamdunkels merged commit 231e772 into contiki-os:master Jul 28, 2013
@wpwrak wpwrak deleted the pull-verbosity branch July 29, 2013 00:01
@oliverschmidt
Copy link
Contributor

This change breaks compatibility with cmd.exe :-((

We have quite some pull requests waiting months for review. I don't know why we then have others being merged without a chance for discussion within an hour !

I request to have this change reverted !

adamdunkels added a commit to adamdunkels/contiki-fork that referenced this pull request Jul 29, 2013
…ity"

This reverts commit 231e772, reversing
changes made to e13d69c.
@wpwrak
Copy link
Contributor Author

wpwrak commented Jul 29, 2013

cmd.exe is Window's shell, correct ? Does it dislike the "&&" ? If yes, would it be happier with ";" instead ? (Which in this case is almost equivalent.)

@adamdunkels
Copy link
Member

Ooops, my bad - being quite a bit too trigger-happy. Sorry about that, and thanks for spotting this!

I created a reverse-pull request: #308

@oliverschmidt
Copy link
Contributor

@wpwrak First of all thanks for your quick reaction.

Yes, cmd.exe is the default shell for Windows. There's a port of GNU make to Win32 ("native", non-Cygwin) that has special precautions to work (as far as possible) with cmd.exe as shell for make recipes. So far the Contiki build is compatible with this setup.

The functionality of cmd.exe isn't comparable at all to "usual" shells. There's no meaningfull concatenation of commands.

AFAIK the usual way to control build output is to have several independent lines in the make recipe like:
@echo "compiling foo.c ..."
@cc foo.c

A somewhat popular way to control that is by introducing a variable $(AT) which is either empty or set to '@' and used like:
$(AT)$(CC) ...

oliverschmidt added a commit that referenced this pull request Jul 29, 2013
Revert "Merge pull request #305 from frtos-wpan/pull-verbosity"
@wpwrak
Copy link
Contributor Author

wpwrak commented Jul 29, 2013

Okay, we can use that approach. For example libopencm3 calls the "AT" variable "Q". To avoid doubling the number of output lines, it also needs an opposing variable. It's a little more intrusive because each recipe affected has to be changed, instead of simply redefining CC et al. I'll give it a try and post a new pull request.

@oliverschmidt
Copy link
Contributor

To avoid doubling the number of output lines, it also needs an opposing variable.

Maybe I don't understand you correctly but one might replace the whole "@echo ..." thingy with a line consistly only of a variable like $(PRINT_GOAL) which is empty if $(Q) is empty.

It's a little more intrusive because each recipe affected has to be changed, instead of simply redefining CC et al.

Yeah, that's more or less the reason why it wasn't done before. But the "quick way" unfortunately doesn't get us there...

I'll give it a try and post a new pull request.

:-)

@wpwrak
Copy link
Contributor Author

wpwrak commented Jul 29, 2013

I think I found a solution that should please all involved. Does this look good ?
frtos-wpan@0694936

Architectures that override rules also need to be adapted. Here's an example for stm32-ocm3 (which is out of tree):
frtos-wpan@a598574

Architectures whose overriding rules aren't adapted will behave as they did before verbosity control.

The output is almost the same as with my first try:
http://pastebin.com/93unLVTr

The main difference is that I used the added flexibility to turn "CC foo.co" into the more useful "LD foo-nosyms.stm32-e407".

If this looks good, I'll send a pull request. Not sure if I should try to adapt the various architectures as well, or leave that to people who use them and can actually test what they're doing.

@oliverschmidt
Copy link
Contributor

Hm, I'm wondering if there's am misunderstanding here or if we have just different opinions.

I already mentioned above that I'd prefer variables containing the whole echo command(s) - especially because of the distributed nature of the Makefiles.

Instead of explaining how the echo is supposed to be formatted you can simply supply the correctly formatted echo commands as variables. And as a side effect you can set the variables only in the V=0 scenario so there's no need for $(VS):

TRACE_CC = echo " CC " $<
TRACE_LD = echo " LD " $@

In case you don't know: If a command line in a recipe is empty then GNU make just completely skips the line, so in the V=1 scenario both $(TRACE_CC) and $(Q) can be just kept unset and this this will work as expected:

foo.o: foo.c
$(TRACE_CC)
$(Q)($CC) ...

@wpwrak
Copy link
Contributor Author

wpwrak commented Jul 30, 2013

Okay, that's tidier. I like it, thanks ! New versions:

frtos-wpan@0a7db98

and

frtos-wpan@f10010c

The empty initializations of TRACE_* on V=1 are there to prevent environment variables from sneaking in.

@oliverschmidt
Copy link
Contributor

I like it

:-)

thanks !

You're welcome !

If you send a pull request with this I'll merge it.

Please send then a message to the list about the change as it heavily modifies the default behavior. And "invite" the Makefile maintainers to make use of it in their custom recipes.

EarthLord pushed a commit to EarthLord/contiki that referenced this pull request Jun 18, 2014
Makefile.include: add verbosity control (make V=1 for old behaviour)
EarthLord pushed a commit to EarthLord/contiki that referenced this pull request Jun 18, 2014
…ity"

This reverts commit 231e772, reversing
changes made to e13d69c.
EarthLord pushed a commit to EarthLord/contiki that referenced this pull request Jun 18, 2014
Revert "Merge pull request contiki-os#305 from frtos-wpan/pull-verbosity"
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.

None yet

3 participants