Hi all,
after all the time where we've all been complaining about the difficulty
to adjust CFLAGS during the build, I could tackle the problem for a first
step in the right direction.
First, let's start with a bit of history to explain the situation and why
it was bad. Originally, a trivial makefile with very simple rules and a
single object to build (haproxy.o) had just CFLAGS set to "-O2 -g" or
something like that, and that was perfect. It was possible to just pass a
different CFLAGS value on the "make" command line and be done with it.
Options started to pile up, but in a way that remained manageable for a
long time (e.g. add PCRE support, later dlmalloc), so CFLAGS was still the
only thing to override if needed. With 32/64 bit variants and so on, we
started to feel the need to split those CFLAGS into multiple pieces for
more flexibility. But these pieces were still aggregated into CFLAGS so
that those used to overriding it were still happy. This situation forced
us not to break these precious CFLAGS that some users were relying on.
And that went like this for a long time, though the definition of this
CFLAGS variable became more and more complicated by inheriting from some
automatic options. For example, in 3.0-dev7, CFLAGS is initialized to
"$(ARCH_FLAGS) $(CPU_CFLAGS) $(DEBUG_CFLAGS) $(SPEC_CFLAGS)", i.e. it
concatenates 4 variables (in apparence). The 4th one (SPEC_CFLAGS) is
already a concatenation of a fixed one (WARN_CFLAGS) and a series of
automatically detected ones (the rest of it). ARCH_FLAGS is set from a
a fixed list of presets depending on the ARCH variable, and CPU_CFLAGS
is set from a list of presets depending on the CPU variable. And the
most beautiful is that CFLAGS alone is no longer sufficient since some
of the USE_* options append their own values behind it, and we complete
with $(TARGET_CFLAGS) $(SMALL_OPTS) $(DEFINE).
Yeah I know that's ugly. We all know it. Whenever someone asks me "how can
I enable -fsanitize=address because I'd like to run ASAN", I respond "hmmm
it depends what options you already use and which ones area easiest to
hack".
Some distros simply found that stuffing their regular CFLAGS into
DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
other combinations depending on the tricks they figured.
After this analysis, I figured that we needed to simplify all this, get
back to what is more natural to use or more commonly found, without
breaking everything for everyone. What is certain however in the current
situation, is that nobody is overriding CFLAGS since it's too rich, too
complex and too unpredictable. So it was really time to reset it.
Thus here's what was done:
- CFLAGS is now empty by default and can be passed some build options
that are appended at the end of the automatic flags. This means that
-Os, -ggdb3, -Wfoobar, -Wno-foobar, -I../secret-place/include etc
will properly be honored without having to trick anything anymore.
Thus for package maintainers, building with CFLAGS="$DISTRO_CFLAGS"
should just get the work done.
- LDFLAGS is now empty by default and can be passed some build options
that are prepended to the list of linker options.
- ARCH_FLAGS now defaults to -g and is passed to both the compiler and
the linker. It may be overridden to change the word size (-m32/-m64),
enable alternate debugging formats (-ggdb3), enable LTO (-flto),
ASAN (-fsanitize=address) etc. It's in fact for flags that must be
consistent between the compiler and the linker. It was not renamed
since it was already there and used quite a bit already.
- CPU_CFLAGS was preserved for ease of porting but is empty by default.
Some may find it convenient to pass their -march, -mtune etc there.
- CPU and ARCH are gone. Let's face it, only 2 of them were usable and
no single maintainer will be crazy enough to use these options here
and resort to another approach for other archs. However the previous
values are mentioned in the doc as a hint about what's known to work
well.
- OPT_CFLAGS was created with "-O2" and nothing else. As developers,
we spend our time juggling between -O2 and -O0 depending on whether
we're testing or debugging. Some embedded distros also have options
to pass -O2 or -Os to choose between fast and small, and that may
fit very well there.
- STD_CFLAGS contains a few flags related to standards compliance.
It is currently set to -fwrapv, -fno-strict-overflow and/or empty
depending on what the compiler prefers. It's important not to touch
it unless you know exactly what you're doing, and previously these
options used to be lost by accident when overriding other ones.
- WARN_CFLAGS is now set to the list of warnings to be enabled. It's
discovered automatically but may be overridden without breaking the
rest.
- NOWARN_CFLAGS is now set to the list of warnings to be disabled. It's
discovered automatically byt may also be overridden.
- DEBUG_CFLAGS is gone. It used to only contain -g by default and was
often abused as a way to pass CFLAGS.
- ERROR_CFLAGS contains a synthetic list of -Werror and -Wfatal-errors
depending on ERR and FAILFAST. It may easily be overridden if desired
though I don't see much value in doing this.
- the rest is unchanged for now.
This means that now one can simply produce a smaller build this way:
$ make -j$(nproc) TARGET=linux-glibc CFLAGS=-Os
In practice it will emit "-O2 ... -Os" but that's fine and very common.
A cleaner method if your build system already has this option alone is:
$ make -j$(nproc) TARGET=linux-glibc OPT_CFLAGS=-Os
Those who have a collection of flags to pass to gcc both at the compilation
and linking stages can use ARCH_FLAGS:
$ distro_flags="-Og -ggdb3 -flto"
$ make -j$(nproc) TARGET=linux-glibc ARCH_FLAGS="$distro_flags"
Those who build with an antique compiler and need to shut up some
warnings will now have a better experience:
$ make TARGET=linux-glibc CC=gcc-4.4 CFLAGS=-fno-strict-aliasing
$ make TARGET=linux-glibc CC=gcc-4.2 WARN_CFLAGS= NOWARN_CFLAGS=
The rest continues to work as previously (e.g. ADDINC and ADDLIB). When
the removed variables are set, the makefile automatically detects it and
emits a warning indicating where it's now recommended to set these values,
so the porting effort should remain very very low. My local scripts even
continue to work!
There are still a few entries I'm not much decided upon at this point:
- DEBUG continues to support passing -DDEBUG_foo and so on, but it's
technically a list of CFLAGS. Many of the developers and probably
prod users who build themselves use it. I'd be tempted to kill it
"just because I can" but I think it would be more annoying to all of
us than helpful, so I tend to think it could stay.
- DEFINE is exactly the same, but it's used for general defines (e.g.
change some internal defaults). Technically both are interchangable.
It could be thought that DEBUG would be moved into DEFINE but again,
I sense it would cause more grief than it could help, and frankly,
overall these are not the parts that raised any complaints, so as
we use to say "if it ain't broke don't fix it".
- CPU_CFLAGS could possibly go if everyone wants to concatenate all
their arch-specific flags into CFLAGS, but I tend to find this more
readable in build scripts, especially when cross-compiling.
I took care of updating the CI matrix BTW and the changes were really
tiny.
On a related topic, after a discussion with William I also performed three
minor changes:
- support USE_FOO=0 : previously, USE_FOO had to be either empty (off)
or filled with whatever to be turned on. I agree that it's sometimes
confusing and not always convenient when it gets the output from a
command (e.g. USE_FOO=$? after a test). Now the value 0 will work as
the empty string and will disable the feature. If you think that you
might have left some USE_FOO=0 to enable FOO in your scripts, please
double-check them.
- emit a warning when writing USE_FOOBRA=1 while it was supposed to be
USE_FOOBAR=1. It happened to all of us quite a few times already,
particularly with USE_QUIC_OPENSSL_COMPAT which has enough words to
permit a reversal. Now if a USE_something variable is passed and is
not known, a warning will just mention that it is ignored. This is
really helpful IMHO as it saves debugging time.
- the warning options are no longer reported in "haproxy -vv". That was
totally pointless (since automatically detected, and not affecting
the resulting code) and it was polluting the output making it more
difficult to spot the important options. However, they're reported
on their own lines in the output of the less known "make opts".
William suggested that the warning on the misspelled USE_foo could be
backported since it's not much intrusive and helps spot manipulation
problems. I'm definitely open to this.
I'm interested in hearing about comments, questions or even complaints
from package maintainers, developers, and generally speaking all those
who build a lot from sources. We can still adjust certain things if there
are good ideas or if I suppressed something that was important for someone.
Please let me know.
Thanks,
Willy
