Makefile: Remove KBUILD_VERBOSE and quiet

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 <[email protected]>

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.

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.

Hi,

Le 2015-09-07 23:14, Arnout Vandecappelle a ?crit :

Quoted 1 line

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?

Quoted 4 lines

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:

Quoted 4 lines

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,

Quoted 5 lines

Quoted 5 lines

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.

Le 2015-09-08 14:30, Arnout Vandecappelle a ?crit :

Quoted 6 lines

Quoted 4 lines

Quoted 4 lines

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.

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,

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 <[email protected]>

---
 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?

Le 20/09/2015 15:11, Thomas Petazzoni a ?crit :

Quoted 5 lines

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.

Hi Thomas, Arnout, and all,

Le 20/09/2015 15:11, Thomas Petazzoni a ?crit :

Quoted 5 lines

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.

Quoted 5 lines

- 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.

Le 2015-09-20 23:26, Arnout Vandecappelle a ?crit :

Quoted 4 lines

Quoted 3 lines

- 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!...

Quoted 2 lines

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...

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.