On 2021/03/06 13:47, Rafael Sadowski wrote:
> On Wed Mar 03, 2021 at 06:21:35PM +0000, Stuart Henderson wrote:
> > On 2021/02/27 09:43, Rafael Sadowski wrote:
> > > +_MODCMAKE_ARGS += -DCMAKE_C_COMPILER="${CC}" \
> > > + -DCMAKE_CXX_COMPILER="${CXX}" \
> > > + -DCMAKE_C_FLAGS="${CFLAGS}" \
> > > + -DCMAKE_C_FLAGS_DEBUG="${CFLAGS}" \
> > > + -DCMAKE_C_FLAGS_RELEASE="${CFLAGS}" \
> >
> > I just found something with
> >
> > -set(CMAKE_C_FLAGS_RELEASE "-O3 -DNDEBUG")
>
> It is good if we override this, isn't?
Overriding -DNDEBUG in the above, or some of the other flags that a
build might set, is not good. And I don't think we can override the -O3
without also overriding these other things.
It's useful for porters to know they can set these in CONFIGURE_ARGS
so I think it would be helpful to call these out in port-modules(7) cmake
section so they're easy to find but automatically overriding (especially
by default) seems a step too far.
I guess I wouldn't object if it was disabled by default with an option
to enable it...MODCMAKE_OVERRIDE_CFLAGS or something...or provide a
variable to use like "CONFIGURE_ARGS= ${MODCMAKE_COMPILER_OVERRIDES}"
though I would worry a bit that a porter might set it and not notice
that an important flag gets removed from the build.
> > I worry that overriding this is going to cause things to quietly build
> > with bad flags..
>
> I see your point but I don't understand what you mine with "bad flags".
> Application build/link "bad" flags?
>
> In this example we can't override CMAKE_C_FLAGS_RELEASE because FORCE is
> set.
>
> net/ettercap/patches/patch-CMakeLists_txt
> +set(CMAKE_C_FLAGS_RELEASE "-w -D_FORTIFY_SOURCE=2" CACHE STRING "" FORCE)
>
..but in other examples they aren't forced. We can't expect every upstream
to know (and use) every cmake feature..
