Re: [PATCH] xen/Makefile: drop -Werror

[Fabrice Fontaine <fontaine.fabrice@xxxxxxxxx>]

Dear all,

Le lun. 5 juil. 2021 à 10:16, Jan Beulich <jbeulich@xxxxxxxx> a écrit :
>
> On 02.07.2021 19:51, Fabrice Fontaine wrote:
> > Le ven. 2 juil. 2021 à 19:34, Andrew Cooper
> > <andrew.cooper3@xxxxxxxxxx> a écrit :
> >>
> >> On 02/07/2021 18:06, Fabrice Fontaine wrote:
> >>> Drop -Werror to avoid the following build failure with -DNDEBUG:
> >>>
> >>> In file included from <command-line>:0:0:
> >>> /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/build/xen-4.14.2/xen/include/xen/config.h:94:0:
> >>>  error: "NDEBUG" redefined [-Werror]
> >>>  #define NDEBUG
> >>>
> >>> <command-line>:0:0: note: this is the location of the previous definition
> >>>
> >>> Fixes:
> >>>  - 
> >>> http://autobuild.buildroot.org/results/66573ad0abc4244c0dfeac8b684a7bfcc31c0d4d
> >>>
> >>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@xxxxxxxxx>
> >>
> >> For better or worse, It is Xen's policy that -Werror will remain.  95%
> >> of the time, it is the right thing.  We will however build failures
> >> whenever they crop up.
> >>
> >> This one is weird though.  How is NDEBUG getting in twice?  What does
> >> the rest of this build environment look like?
> > NDEBUG is added by buildroot in the command line if the user sets
> > BR2_ENABLE_RUNTIME_DEBUG to false since
> > https://git.buildroot.net/buildroot/commit/?id=5a8c50fe05afacc3cbe8e7347e238da9f242fab0
>
> I suppose the build environment setting is really intended for user mode
> code. I question its applicability to the building of kernels or
> hypervisors, but I can see that opinions may vary here. If we wanted to
> honor a pre-existing NDEBUG, how about simply making xen/config.h have
>
> #if !defined(CONFIG_DEBUG) && !defined(NDEBUG)
> #define NDEBUG
> #endif
>
> ? This then raises the question though how an external environment could
> achieve the opposite effect of suppressing NDEBUG's definition despite
> CONFIG_DEBUG being set. (The main point - hence my view expressed above -
> is that we switched to Kconfig to centralize where settings get
> established, moving away from taking ones from environment or make
> command line.)
FYI, we have reverted the commit that allowed the user to set -DNDEBUG
as it was raising too many build failures:
https://git.buildroot.net/buildroot/commit/?id=a1c7cff1a081765c082c196bd9e6c1e72ceee797
So, this patch is not needed anymore on buildroot side.
>
> Jan
>
Best Regards,

Fabrice