Skip to content

Conversation

@btriller
Copy link
Contributor

@btriller btriller commented Apr 1, 2025

Set CFLAGS before AC_PROG_CC as it might add -g/-O options if CFLAGS is unset.
Don't add CFLAGS to CORE_FLAGS because I don't know, but maybe CFLAGS is populated otherwise.

@olehermanse
Copy link
Contributor

@btriller Why did you close this? IIRC, the change looked reasonable, but more work was needed to fix the failing tests(?)

@btriller
Copy link
Contributor Author

@btriller Why did you close this? IIRC, the change looked reasonable, but more work was needed to fix the failing tests(?)

I looked further into it on macOS (with clang). With -Wall -Werror, there were many errors about redefinition that I wasn't sure I'm able to resolve it. IIRC on Linux with gcc and, for starters, more ifdefs in libcfecompat, it looked better.

Set CFLAGS before AC_PROG_CC as it might add -g/-O options if CFLAGS is
unset.
Don't add CFLAGS to CORE_FLAGS because I don't know, but maybe CFLAGS is
populated otherwise.
In case CFLAGS were given to configure, keep them for later stages.
Check for clang compiler so strchrnul() can be used for macOS >= 15.4.
Replace some obsolete autoconf macros.
Make -Werror works in configure.
@olehermanse
Copy link
Contributor

@btriller Thanks for working on this. We're a bit busy with getting the releases out, but I'll get back to reviewing the PR shortly after that :)

@larsewi
Copy link
Contributor

larsewi commented Nov 3, 2025

Generally we should not really touch CFLAGS at all. This variable is intended for the end user only. People familiar with autotools would expect the AC_PROG_CC defaults. Anything else would come as a surprise. However, we do already touch them. Hence, not touching them would probably come as a surprise for people familiar with compiling CFEngine.

In this thread it is suggested to set : ${CFLAGS=""} to empty before AC_PROG_CC if not already set by the user. This way we could control the optimization flags through AM_CFLAGS which would be the proper way to set our own flags. Do you want to try this out? Or do you want us to take a look at it?

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.

3 participants