On 05/29/2015 08:19 AM, Ulrich Windl wrote:
> Some remarks on the CFLAGS thing:
> In one of my projects I used this approach for messing with CFLAGS:
> --
> CFLAGS=-pthread
> CWARNFLAGS=-Wall -Wextra -Wshadow
> COPTFLAGS=-pipe -O2
> CDEBUGFLAGS=-g -fstack-protector-all
> CFLAGS+=$(CWARNFLAGS)
> CFLAGS+=$(COPTFLAGS)
> #CFLAGS+=$(CDEBUGFLAGS)
> LDFLAGS=-lpthread -lrt
> --
> It's somewhat GNU-makish, but you can easily switch CFLAGS by
> (un)commenting some of the "CFLAGS+=" lines.
> [...]
> So my CFLAGS are the absolute minimum required for a correct compile,
> and everything else are extras.
The code you posted has the opposite effect of what my patch wants to
achieve. There appears to be some confusion as to the purpose of my
patch, so maybe I should go a bit into detail about what the point here
is.
When compiling software, either for distributions that want to create a
package from it, or for end users that want to call it, one will often
want to set some global compiler and linker flags to influence package
build.
- Sometimes you need to debug something and disable optimization for
that. Then you might need want to rebuild a package with a different
-O level than the package typically has. Of course, one can
tediously look at the means that different software provides to do
so (if at all) - or one can just assume that setting CFLAGS="-O0"
before running configure / make will do the trick.
- When compiling with a compiler other than gcc, you might need to add
some flags in order to make it work.
- There is a goal to harden as much software as possible in Debian, so
there's a recommendation that things like -fstack-protector-strong
are to be used (unless the software doesn't work otherwise).
It has long been standard practice among open source projects that if
external CC, CFLAGS, CPPFLAGS, CXXFLAGS, LDFLAGS, ... are supplied when
building, those flags will influence the build. So what one typically
does is partition the CFLAGS into to sets: those required for
compilation (such as -pthread) and a default set of flags if no
external ones are specified.
For example, if you use a default autoconf + automake setup, these will
typically set -O2 -g as the default set of flags (if on gcc and the
compiler supports -g). But if one does
CFLAGS="-sometingelse" ./configure
or
./configure CFLAGS="-sometingelse"
it will use -somethingelse instead of -O2 -g. Same thing with the other
environment variables.
And those flags that are required to make the package compile should
simply be added to the current set of flags. In a standard autoconf and
automake setup, you can do it in configure.ac along the
lines of:
CFLAGS+=" -additional-flags"
(Of course, checking that the compiler supports your flag is always
better, there is a macro for that; and things like pthread actually
have a standardized macro that do that automatically.)
Alternatively, you can set it directly in Makefile.am via
AM_CFLAGS = -additional-flags
which will automatically be added to the list of flags.
Since this is somewhat standardized, this has the great advantage that
you don't need to invest time into figuring out how each different
piece of software requires to change its own build flags. If you
package things for your distribution, there are often a lot of
automatisms that use this mechanism to provide set default set of
flags - and you don't have to do anything for all these things to just
work[tm]. But if a package doesn't do it the standard way, then you
have to work around all these sorts of things.
But your piece of code goes even further than than a custom mechanism:
it doesn't provide a way to influence CFLAGS externally at all! With a
custom mechanism, you can figure it out (both as a user that wants
to compile it as well as a packager) and then use it. But if you
hard-code CFLAGS = ... in your Makefile (as per your example), one
actually needs to patch the software to achieve this goal.
Now what is the state of open-iscsi's current git?
- There's a custom mechanism for setting CFLAGS by the way of a
variable named OPTFLAGS. It mostly works, with a couple of
excpetions:
- OPTFLAGS is not properly respected in utils/sysdeps/Makefile
because while it's referenced there, -O2 and -g are appended in
all cases.
- OPTFLAGS is not properly respected for the compilation of
iscsiuio for two reasons:
- iscsiuio/configure.ac explicitly overrides CFLAGS
- top-level Makefile doesn't pass CFLAGS="$(OPTFLAGS)" to
iscsiuio's configure statement (as it does with
open-isns)
- LDFLAGS are not used in the implicit linker rules for the binaries,
thus not being honored if set (open-iscsi doesn't need any custom
ones here)
- other variables, such as CPPFLAGS and CC are indeed honored, because
open-iscsi uses implicit rules for its iscsid etc. code and automake
for open-isns + iscsiui. And there it automatically works without
any changes.
What does my patch now do?
- We don't want to get rid of OPTFLAGS completely just now, because it
might still be used by some automated packaging scripts. So have it
override the default CFLAGS if set, but completely disregard it if
unset. Put it only in the top-level Makefile, so it may be removed
at some point in the future.
- In the Makefile, if CFLAGS has been set, export it so that both
configure commands actually use it. Remove the direct
CFLAGS="$(OPTFLAGS)" call to the first configure command. Used like
this, the default gets used if nothing is set (previously, open-isns
would have empty CFLAGS by default because of this).
- Fix iscsiuio/configure.ac to not override CFLAGS but to just have a
default.
- In usr/Makefile, utils/Makefile, utils/*/Makefile use the following
pattern:
CFLAGS ?= default flags that might be overridden
(mainly -O2 -g)
CFLAGS += always used flags
- In usr/Makefile, since it uses explicit rules for linking, add
LDFLAGS here.
Then you can do things like:
CFLAGS="-O2" make clean all
CFLAGS="-O2 -g" make clean all
CFLAGS="-O0 -g" make clean all
and the flags will affect compilation.
I hope that makes it a lot clearer as to the reasoning behind this
patch.
> And if you need "defines" they should go to CPPFLAGS.
Yes, in principle you are right here - defines and includes should go
to CPPFLAGS, the rest into CFLAGS. On the other hand, that doesn't
really affect functionality, and I didn't want to completely rewrite
the entire build system here.
Best regards,
Christian
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
