Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
@ 2015-09-07 20:47 Cédric Marie
  2015-09-07 21:14 ` Arnout Vandecappelle
  2015-09-20 13:11 ` Thomas Petazzoni
  0 siblings, 2 replies; 16+ messages in thread
From: Cédric Marie @ 2015-09-07 20:47 UTC (permalink / raw)
  To: buildroot

KBUILD_VERBOSE and quiet variables are set and exported, but they are
not used. We can safely remove them.

These variables are inherited from the Makefile of the Linux kernel,
and are not used in Buildroot.

In support/scripts/mkmakefile, quiet value is checked, but the test is
always true (quiet is never set to silent_), so the test can be removed
as well.

Signed-off-by: C?dric Marie <cedric.marie@openmailbox.org>
---
 Makefile                   | 20 ++++++--------------
 support/scripts/mkmakefile |  4 +---
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 23e2ee6..05dd2db 100644
--- a/Makefile
+++ b/Makefile
@@ -218,23 +218,15 @@ endif
 
 # To put more focus on warnings, be less verbose as default
 # Use 'make V=1' to see the full commands
+Q = @
 ifeq ("$(origin V)", "command line")
-  KBUILD_VERBOSE = $(V)
-endif
-ifndef KBUILD_VERBOSE
-  KBUILD_VERBOSE = 0
-endif
-
-ifeq ($(KBUILD_VERBOSE),1)
-  quiet =
-  Q =
+ifeq ($(V),1)
+Q =
 ifndef VERBOSE
-  VERBOSE = 1
+VERBOSE = 1
 endif
 export VERBOSE
-else
-  quiet = quiet_
-  Q = @
+endif
 endif
 
 # we want bash as shell
@@ -245,7 +237,7 @@ SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 # kconfig uses CONFIG_SHELL
 CONFIG_SHELL := $(SHELL)
 
-export SHELL CONFIG_SHELL quiet Q KBUILD_VERBOSE
+export SHELL CONFIG_SHELL Q
 
 ifndef HOSTAR
 HOSTAR := ar
diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
index 833be6a..37162a3 100755
--- a/support/scripts/mkmakefile
+++ b/support/scripts/mkmakefile
@@ -15,9 +15,7 @@ if test -e $2/Makefile && ! grep -q Automatically $2/Makefile
 then
 	exit 0
 fi
-if [ "${quiet}" != "silent_" ]; then
-	echo "  GEN     $2/Makefile"
-fi
+echo "  GEN     $2/Makefile"
 
 cat << EOF > $2/Makefile
 # Automatically generated by $0: don't edit
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-07 20:47 [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet Cédric Marie
@ 2015-09-07 21:14 ` Arnout Vandecappelle
  2015-09-08 12:14   ` Cédric Marie
  2015-09-20 13:11 ` Thomas Petazzoni
  1 sibling, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-07 21:14 UTC (permalink / raw)
  To: buildroot

On 07-09-15 22:47, C?dric Marie wrote:
> KBUILD_VERBOSE and quiet variables are set and exported, but they are
> not used. We can safely remove them.
> 
> These variables are inherited from the Makefile of the Linux kernel,
> and are not used in Buildroot.
> 
> In support/scripts/mkmakefile, quiet value is checked, but the test is
> always true (quiet is never set to silent_), so the test can be removed
> as well.
> 
> Signed-off-by: C?dric Marie <cedric.marie@openmailbox.org>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 [A few build tests with and without V= ]

 Don't forget to version your patches and keep a patch changelog...

 I would already have introduced the VERBOSE construct in this patch, but I
guess this one is less controversial, so OK.


 Regards,
 Arnout

[snip]

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-07 21:14 ` Arnout Vandecappelle
@ 2015-09-08 12:14   ` Cédric Marie
  2015-09-08 12:30     ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2015-09-08 12:14 UTC (permalink / raw)
  To: buildroot

Hi,

Le 2015-09-07 23:14, Arnout Vandecappelle a ?crit?:
>  Don't forget to version your patches and keep a patch changelog...

I had this remark in mind - you told me some time ago - but I intended 
to do it with the patch that will handle verbosity in infrastructures. I 
kind of considered this one as a new one.
Since the patch has been split into several patches, should I increment 
the version for every new patch?


>  I would already have introduced the VERBOSE construct in this patch, 
> but I
> guess this one is less controversial, so OK.

I have tried to follow your guidelines:

> 1. Remove KBUILD_VERBOSE and quiet (this one will be uncontroversial I 
> think)
> 2. Define VERBOSE based on V= passed on command line
> 3. Use VERBOSE in autotools and cmake
> 4. Update documentation.


Do you confirm I still should send two separate patches for steps 2 and 
3?

Regards,

-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-08 12:14   ` Cédric Marie
@ 2015-09-08 12:30     ` Arnout Vandecappelle
  2015-09-08 13:01       ` Cédric Marie
  0 siblings, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-08 12:30 UTC (permalink / raw)
  To: buildroot

On 08-09-15 14:14, C?dric Marie wrote:
> Hi,
> 
> Le 2015-09-07 23:14, Arnout Vandecappelle a ?crit :
>>  Don't forget to version your patches and keep a patch changelog...
> 
> I had this remark in mind - you told me some time ago - but I intended to do it
> with the patch that will handle verbosity in infrastructures. I kind of
> considered this one as a new one.
> Since the patch has been split into several patches, should I increment the
> version for every new patch?

 Yeah, that's more convenient. The goal of the version number is to make it
clear that something similar has come by before and that that previous thing is
superseded by the new thing.

> 
> 
>>  I would already have introduced the VERBOSE construct in this patch, but I
>> guess this one is less controversial, so OK.
> 
> I have tried to follow your guidelines:
> 
>> 1. Remove KBUILD_VERBOSE and quiet (this one will be uncontroversial I think)
>> 2. Define VERBOSE based on V= passed on command line
>> 3. Use VERBOSE in autotools and cmake
>> 4. Update documentation.

 Oops, you're right, I proposed that myself :-) But I didn't realise that the
VERBOSE variable existed already.

> 
> 
> Do you confirm I still should send two separate patches for steps 2 and 3?

 No, since VERBOSE already exists, my proposed step 2 is useless.

 Regards,
 Arnout

> 
> Regards,
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-08 12:30     ` Arnout Vandecappelle
@ 2015-09-08 13:01       ` Cédric Marie
  2015-09-08 13:34         ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2015-09-08 13:01 UTC (permalink / raw)
  To: buildroot

Le 2015-09-08 14:30, Arnout Vandecappelle a ?crit?:
>>> 1. Remove KBUILD_VERBOSE and quiet (this one will be uncontroversial 
>>> I think)
>>> 2. Define VERBOSE based on V= passed on command line
>>> 3. Use VERBOSE in autotools and cmake
>>> 4. Update documentation.

>  No, since VERBOSE already exists, my proposed step 2 is useless.

OK. Please note that I will change it a little bit:
- remove export VERBOSE
- VERBOSE=1 if V=1, even if VERBOSE is already defined (i.e. remove 
"ifndef VERBOSE" test)
- VERBOSE=0 if V=0
- VERBOSE=(empty) otherwise to prevent this variable from being 
inherited from the shell.

Regards,

-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-08 13:01       ` Cédric Marie
@ 2015-09-08 13:34         ` Arnout Vandecappelle
  2015-09-11  7:52           ` Cédric Marie
  0 siblings, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-08 13:34 UTC (permalink / raw)
  To: buildroot

On 08-09-15 15:01, C?dric Marie wrote:
> Le 2015-09-08 14:30, Arnout Vandecappelle a ?crit :
>>>> 1. Remove KBUILD_VERBOSE and quiet (this one will be uncontroversial I think)
>>>> 2. Define VERBOSE based on V= passed on command line
>>>> 3. Use VERBOSE in autotools and cmake
>>>> 4. Update documentation.
> 
>>  No, since VERBOSE already exists, my proposed step 2 is useless.
> 
> OK. Please note that I will change it a little bit:
> - remove export VERBOSE
> - VERBOSE=1 if V=1, even if VERBOSE is already defined (i.e. remove "ifndef
> VERBOSE" test)
> - VERBOSE=0 if V=0
> - VERBOSE=(empty) otherwise to prevent this variable from being inherited from
> the shell.

 Good!

 Note in the commit log why it has to be that way (for cmake, I guess).

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-08 13:34         ` Arnout Vandecappelle
@ 2015-09-11  7:52           ` Cédric Marie
  2015-09-11 14:14             ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2015-09-11  7:52 UTC (permalink / raw)
  To: buildroot

Hi,

I've just realized that this patch is breaking linux and busybox verbose 
mode.

Exporting KBUILD_VERBOSE makes it available for linux and busybox 
makefile. This makefile sets KBUILD_VERBOSE if V is set in the command 
line, but it also uses KBUILD_VERBOSE if exported by the shell.

In fact it seems that the original idea was to export all variables that 
could be understood by different build systems:
- KBUILD_VERBOSE for linux and busybox
- VERBOSE for CMake packages.

So, if we still want to handle verbosity in infrastructures (and some 
packages that use generic infrastructure) - and I think we should:
I think the right way to forward verbose mode to linux and busybox is to 
add V=1 in linux.mk and busybox.mk.

As a consequence, I believe it is not possible to split my patch into 
different steps.
The patch must:
- remove quiet
- remove KBUILD_VERBOSE
- use VERBOSE but in a different way (not exported), as already 
described
- handle VERBOSE in pkg-cmake.mk, pkg-autotools.mk, linux.mk, 
busybox.mk, and qt.mk (this one is possibly already correct).

If you agree, I will provide a single patch that will cancel the two 
previous ones (I will add patch version and log this time).

Regards,

-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-11  7:52           ` Cédric Marie
@ 2015-09-11 14:14             ` Arnout Vandecappelle
  0 siblings, 0 replies; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-11 14:14 UTC (permalink / raw)
  To: buildroot



On 11-09-15 09:52, C?dric Marie wrote:
> Hi,
> 
> I've just realized that this patch is breaking linux and busybox verbose mode.
> 
> Exporting KBUILD_VERBOSE makes it available for linux and busybox makefile. This
> makefile sets KBUILD_VERBOSE if V is set in the command line, but it also uses
> KBUILD_VERBOSE if exported by the shell.
> 
> In fact it seems that the original idea was to export all variables that could
> be understood by different build systems:
> - KBUILD_VERBOSE for linux and busybox
> - VERBOSE for CMake packages.
> 
> So, if we still want to handle verbosity in infrastructures (and some packages
> that use generic infrastructure) - and I think we should:
> I think the right way to forward verbose mode to linux and busybox is to add V=1
> in linux.mk and busybox.mk.

 I'm not so sure of that. The KBUILD_VERBOSE approach worked well, there wasn't
really a problem with it. The problem was with VERBOSE, which is interpreted
differently by cmake. Removing KBUILD_VERBOSE and quiet was just because
(quoting your commit message): "KBUILD_VERBOSE and quiet variables are set and
exported, but they are not used. We can safely remove them."

 The current way (exporting KBUILD_VERBOSE) is in fact quite elegant: it's just
a couple of lines of code, and it works for any package based on Kbuild. Like
uClibc, barebox, who knows, maybe they also use KBUILD_VERBOSE? This way is much
easier than handling it for every package separately.

 So I think the approach should be:
- remove quiet
- add an explanation about why KBUILD_VERBOSE is useful
- use VERBOSE but in a different way.

and in fact, since we already have KBUILD_VERBOSE, maybe VERBOSE can just be
dropped? (internally, it's still used for cmake.)

> 
> As a consequence, I believe it is not possible to split my patch into different
> steps.
> The patch must:
> - remove quiet
> - remove KBUILD_VERBOSE
> - use VERBOSE but in a different way (not exported), as already described
> - handle VERBOSE in pkg-cmake.mk, pkg-autotools.mk, linux.mk, busybox.mk, and
> qt.mk (this one is possibly already correct).
> 
> If you agree, I will provide a single patch that will cancel the two previous
> ones (I will add patch version and log this time).

 The reason for splitting was that the first one would be uncontroversial - but
it clearly isn't, so I'm OK with keeping them merged.


 Thanks for keeping this up!

 Regards,
 Arnout

> 
> Regards,
> 

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-07 20:47 [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet Cédric Marie
  2015-09-07 21:14 ` Arnout Vandecappelle
@ 2015-09-20 13:11 ` Thomas Petazzoni
  2015-09-20 20:43   ` Cédric Marie
  1 sibling, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2015-09-20 13:11 UTC (permalink / raw)
  To: buildroot

C?dric,

On Mon,  7 Sep 2015 22:47:26 +0200, C?dric Marie wrote:
> KBUILD_VERBOSE and quiet variables are set and exported, but they are
> not used. We can safely remove them.
> 
> These variables are inherited from the Makefile of the Linux kernel,
> and are not used in Buildroot.
> 
> In support/scripts/mkmakefile, quiet value is checked, but the test is
> always true (quiet is never set to silent_), so the test can be removed
> as well.
> 
> Signed-off-by: C?dric Marie <cedric.marie@openmailbox.org>
> ---
>  Makefile                   | 20 ++++++--------------
>  support/scripts/mkmakefile |  4 +---
>  2 files changed, 7 insertions(+), 17 deletions(-)

Following the discussion you had with Arnout, and your own report that
this patch was breaking things in the Busybox and Linux kernel build, I
marked this patch as "Changes Requested" in patchwork.

Will you work on a new iteration of this patch?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-20 13:11 ` Thomas Petazzoni
@ 2015-09-20 20:43   ` Cédric Marie
  2015-09-20 21:04     ` Thomas Petazzoni
  2015-09-20 21:26     ` Arnout Vandecappelle
  0 siblings, 2 replies; 16+ messages in thread
From: Cédric Marie @ 2015-09-20 20:43 UTC (permalink / raw)
  To: buildroot

Hi Thomas, Arnout, and all,

Le 20/09/2015 15:11, Thomas Petazzoni a ?crit :
> Following the discussion you had with Arnout, and your own report that
> this patch was breaking things in the Busybox and Linux kernel build, I
> marked this patch as "Changes Requested" in patchwork.
>
> Will you work on a new iteration of this patch?

Yes, I've started to work on a new iteration. It is working fine with 
CMake, autotools, and Kbuild packages (I keep on exporting 
KBUILD_VERBOSE as suggested by Arnout).

I still have to:

- modify qt.mk because it currently enables verbose mode if VERBOSE is 
defined and not empty, but my modification implies that VERBOSE may be 
equal to 0, and it should not enable verbose mode in that case!...
- check that no other package .mk file is using VERBOSE variable.
- write clear comments and commit message.

Sorry for the delay, but I've got only very occasional free time to 
spend on it.


-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-20 20:43   ` Cédric Marie
@ 2015-09-20 21:04     ` Thomas Petazzoni
  2015-09-20 21:26     ` Arnout Vandecappelle
  1 sibling, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2015-09-20 21:04 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 20 Sep 2015 22:43:54 +0200, C?dric Marie wrote:

> Le 20/09/2015 15:11, Thomas Petazzoni a ?crit :
> > Following the discussion you had with Arnout, and your own report that
> > this patch was breaking things in the Busybox and Linux kernel build, I
> > marked this patch as "Changes Requested" in patchwork.
> >
> > Will you work on a new iteration of this patch?
> 
> Yes, I've started to work on a new iteration. It is working fine with 
> CMake, autotools, and Kbuild packages (I keep on exporting 
> KBUILD_VERBOSE as suggested by Arnout).
> 
> I still have to:
> 
> - modify qt.mk because it currently enables verbose mode if VERBOSE is 
> defined and not empty, but my modification implies that VERBOSE may be 
> equal to 0, and it should not enable verbose mode in that case!...
> - check that no other package .mk file is using VERBOSE variable.
> - write clear comments and commit message.

Excellent, thanks!

> Sorry for the delay, but I've got only very occasional free time to 
> spend on it.

No problem at all. I just e-mailed you to let you know that your patch
has been marked "Changes Requested": it means that it is no longer
visible in the list of pending patches to be considered by the
maintainers. So when this is done, it is up to the submitter to come
back at some point with a new iteration of the patch, otherwise the
topic will simply be forgotten by the maintainers. This is why I
generally send a notification e-mail when changing a patch to "Changes
Requested".

There is obviously no problem if the new iteration only comes in
several weeks or even months.

Thanks again,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-20 20:43   ` Cédric Marie
  2015-09-20 21:04     ` Thomas Petazzoni
@ 2015-09-20 21:26     ` Arnout Vandecappelle
  2015-09-21 10:23       ` Cédric Marie
  1 sibling, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-20 21:26 UTC (permalink / raw)
  To: buildroot

On 20-09-15 22:43, C?dric Marie wrote:
> Hi Thomas, Arnout, and all,
> 
> Le 20/09/2015 15:11, Thomas Petazzoni a ?crit :
>> Following the discussion you had with Arnout, and your own report that
>> this patch was breaking things in the Busybox and Linux kernel build, I
>> marked this patch as "Changes Requested" in patchwork.
>>
>> Will you work on a new iteration of this patch?
> 
> Yes, I've started to work on a new iteration. It is working fine with CMake,
> autotools, and Kbuild packages (I keep on exporting KBUILD_VERBOSE as suggested
> by Arnout).
> 
> I still have to:
> 
> - modify qt.mk because it currently enables verbose mode if VERBOSE is defined
> and not empty, but my modification implies that VERBOSE may be equal to 0, and
> it should not enable verbose mode in that case!...

 Well, just unset VERBOSE when V=0. So VERBOSE is either empty or 1. Similar
with how Config.in works.

> - check that no other package .mk file is using VERBOSE variable.

git grep -w VERBOSE
-> only in qt.mk

> - write clear comments and commit message.

 Yep, very important!

 Regards,
 Arnout

> 
> Sorry for the delay, but I've got only very occasional free time to spend on it.
> 
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-20 21:26     ` Arnout Vandecappelle
@ 2015-09-21 10:23       ` Cédric Marie
  2015-09-21 17:11         ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2015-09-21 10:23 UTC (permalink / raw)
  To: buildroot

Le 2015-09-20 23:26, Arnout Vandecappelle a ?crit?:
>> - modify qt.mk because it currently enables verbose mode if VERBOSE is 
>> defined
>> and not empty, but my modification implies that VERBOSE may be equal 
>> to 0, and
>> it should not enable verbose mode in that case!...
> 
>  Well, just unset VERBOSE when V=0. So VERBOSE is either empty or 1. 
> Similar
> with how Config.in works.

Well, the idea was to set VERBOSE in Makefile and use its value in 
infrastructures.
In autotools I need to make the difference between VERBOSE=1 (force 
verbose), VERBOSE=0 (force non-verbose), and VERBOSE not set (keep 
package default verbose level).

Unless we decide that finally there is no real interest in forcing 
non-verbose and adding a new possibility with 'make V=0'.
I'm not so sure it can be very usefull, since it will only be taken into 
account by a few autotools package.
If you want a quiet output, you'd rather use 'make -s'
NB: I found it in Makefile, but it doesn't seem to be documented...


-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-21 10:23       ` Cédric Marie
@ 2015-09-21 17:11         ` Arnout Vandecappelle
  2015-10-02 15:52           ` Cédric Marie
  0 siblings, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-09-21 17:11 UTC (permalink / raw)
  To: buildroot

On 21-09-15 12:23, C?dric Marie wrote:
> Le 2015-09-20 23:26, Arnout Vandecappelle a ?crit :
>>> - modify qt.mk because it currently enables verbose mode if VERBOSE is defined
>>> and not empty, but my modification implies that VERBOSE may be equal to 0, and
>>> it should not enable verbose mode in that case!...
>>
>>  Well, just unset VERBOSE when V=0. So VERBOSE is either empty or 1. Similar
>> with how Config.in works.
> 
> Well, the idea was to set VERBOSE in Makefile and use its value in infrastructures.
> In autotools I need to make the difference between VERBOSE=1 (force verbose),
> VERBOSE=0 (force non-verbose), and VERBOSE not set (keep package default verbose
> level).

 Right, sorry, I forgot about that.

> 
> Unless we decide that finally there is no real interest in forcing non-verbose
> and adding a new possibility with 'make V=0'.
> I'm not so sure it can be very usefull, since it will only be taken into account
> by a few autotools package.

 A few? Isn't it going to be a very large subset of the 1041 autotools packages?

> If you want a quiet output, you'd rather use 'make -s'
> NB: I found it in Makefile, but it doesn't seem to be documented...

 True. 'make -s' is documented in the make documentation of course, but it would
make sense to also mention it in the buildroot documentation close to the V=1.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-09-21 17:11         ` Arnout Vandecappelle
@ 2015-10-02 15:52           ` Cédric Marie
  2015-10-03 11:57             ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2015-10-02 15:52 UTC (permalink / raw)
  To: buildroot

Hi,

The more I try to fix it, the more I believe there is nothing to fix... 
:)

Arnout has pointed out that KBUILD_VERBOSE should be kept, because it 
was a nice way to set verbose level in Kbuild packages, without having 
to modify any .mk files.

I have also recently realized that autotools infrastructure didn't need 
anything more to have support for V=0 or 1.
When V is in the command line, it is exported to all sub-makes. As a 
consequence, an autotools package already sees the value of V. It 
doesn't check whether it comes from the command line or not.

I thought there was a problem because I first checked with autotools 
packages that did not support verbose level, and couldn't see any 
difference. With libpthsem, I have realized that V=0/1 was already 
supported.

The fact that V is exported is not enough for Kbuild packages, because 
they also check that it comes from the command line. That's why they 
need KBUILD_VERBOSE to be exported. Autotools packages don't check that 
V comes from the command line.

So, in the end, I think it would be stupid to convert V=0/1 into 
VERBOSE=0/1, and then, in pkg-autotools.mk, convert VERBOSE=0/1 into 
V=0/1 - which is already set and exported - again!...

So, to sum up:
- we keep KBUILD_VERBOSE for Kbuild packages
- we don't need to change anything for autotools packages
- If CMake is the only one that needs a conversion (from V to VERBOSE), 
wouldn't it be more simple to keep on exporting VERBOSE as it is done 
currently?

In the end, the only thing I could do, is just to remove "quiet" :)
I can still provide a patch for that, if it makes sense.

Sorry for the time we have spent on this subject, while there is - 
almost - nothing to change in the end...
There are many subtleties in makefiles that I was not aware of, in the 
first place.


-- 
C?dric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet
  2015-10-02 15:52           ` Cédric Marie
@ 2015-10-03 11:57             ` Arnout Vandecappelle
  0 siblings, 0 replies; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-10-03 11:57 UTC (permalink / raw)
  To: buildroot

On 02-10-15 16:52, C?dric Marie wrote:
> Hi,
> 
> The more I try to fix it, the more I believe there is nothing to fix... :)
> 
> Arnout has pointed out that KBUILD_VERBOSE should be kept, because it was a nice
> way to set verbose level in Kbuild packages, without having to modify any .mk
> files.
> 
> I have also recently realized that autotools infrastructure didn't need anything
> more to have support for V=0 or 1.
> When V is in the command line, it is exported to all sub-makes. As a
> consequence, an autotools package already sees the value of V. It doesn't check
> whether it comes from the command line or not.
> 
> I thought there was a problem because I first checked with autotools packages
> that did not support verbose level, and couldn't see any difference. With
> libpthsem, I have realized that V=0/1 was already supported.
> 
> The fact that V is exported is not enough for Kbuild packages, because they also
> check that it comes from the command line. That's why they need KBUILD_VERBOSE
> to be exported. Autotools packages don't check that V comes from the command line.
> 
> So, in the end, I think it would be stupid to convert V=0/1 into VERBOSE=0/1,
> and then, in pkg-autotools.mk, convert VERBOSE=0/1 into V=0/1 - which is already
> set and exported - again!...
> 
> So, to sum up:
> - we keep KBUILD_VERBOSE for Kbuild packages
> - we don't need to change anything for autotools packages
> - If CMake is the only one that needs a conversion (from V to VERBOSE), wouldn't
> it be more simple to keep on exporting VERBOSE as it is done currently?
> 
> In the end, the only thing I could do, is just to remove "quiet" :)
> I can still provide a patch for that, if it makes sense.

 Yep, sounds like a good idea!

> 
> Sorry for the time we have spent on this subject, while there is - almost -
> nothing to change in the end...
> There are many subtleties in makefiles that I was not aware of, in the first place.

 It's great that you investigated all this, I wouldn't call this time wasted.
Now I think we really understand most of the subtleties of the different verbose
options.

 One more fix you could do is to document all this in comments in the top-level
Makefile. Something like:

# autotools packages use the V=0/1 option, which is passed implicitly by make.
# Kbuild packages use KBUILD_VERBOSE
# CMake packages use VERBOSE
# Others don't have a supported verbosity switch
export VERBOSE KBUILD_VERBOSE


 I wonder if exporting the Q is useful at all?

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-10-03 11:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-07 20:47 [Buildroot] [PATCH] Makefile: Remove KBUILD_VERBOSE and quiet Cédric Marie
2015-09-07 21:14 ` Arnout Vandecappelle
2015-09-08 12:14   ` Cédric Marie
2015-09-08 12:30     ` Arnout Vandecappelle
2015-09-08 13:01       ` Cédric Marie
2015-09-08 13:34         ` Arnout Vandecappelle
2015-09-11  7:52           ` Cédric Marie
2015-09-11 14:14             ` Arnout Vandecappelle
2015-09-20 13:11 ` Thomas Petazzoni
2015-09-20 20:43   ` Cédric Marie
2015-09-20 21:04     ` Thomas Petazzoni
2015-09-20 21:26     ` Arnout Vandecappelle
2015-09-21 10:23       ` Cédric Marie
2015-09-21 17:11         ` Arnout Vandecappelle
2015-10-02 15:52           ` Cédric Marie
2015-10-03 11:57             ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox