Issue 116959 - [gbuild] user-defined CFLAGS etc.
Summary: [gbuild] user-defined CFLAGS etc.
Status: CLOSED FIXED
Alias: None
Product: Build Tools
Classification: Code
Component: GNU make (show other issues)
Version: DEV300m100
Hardware: All All
: P3 Normal (vote)
Target Milestone: ---
Assignee: mst.ooo
QA Contact: issues@tools
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-24 15:02 UTC by hans_werner67
Modified: 2017-05-20 10:34 UTC (History)
6 users (show)

See Also:
Issue Type: ENHANCEMENT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description mst.ooo 2011-02-14 17:27:36 UTC
hbrinkm asked me today how a user could add CFLAGS etc.

in the old build system we had ENV_CFLAGS etc.

it seems to me that this is missing currently in gbuild.

shoud i just add support for ENV_CFLAGS etc?
or does somebody have a different proposal?
Comment 1 Stephan Bergmann 2011-02-15 09:31:45 UTC
It would be cool if we would use CFLAGS etc. like a well-behaved project does: 
Allow the user to override behavior through those variables (see e.g.,
<http://www.gnu.org/software/autoconf/manual/standards.html#Command-Variables>).

The old build system uses CFLAGS etc. as dmake variables for internal purposes,
so for user overrides new environment variables ENVCFLAGS etc. had to be invented.

If the new build system does not use CFLAGS etc. for internal purposes (and I
hope so), I would suggest the following:  While the two build systems are used
in parallel, add support for ENVCFLAGS etc. into the new build system.  When the
old build system is no longer used, change the new build system from honoring
ENVCFLAGS etc. to honoring CFLAGS etc. standards conformance.
Comment 2 Mathias_Bauer 2011-02-15 13:16:39 UTC
I already had started working on that but finally didn't want to endanger the
integration of gnumake3.

We can easily add ENV_CFLAGS to the CXXFLAGS, the problem is: where. It
shouldn't go into the PCHCXXFLAGS but we must make sure that it is used in all
other cases.

My proposal is to add it in Linktarget.mk like shown here:

define gb_LinkTarget_add_cxxobject
$(call gb_LinkTarget_get_target,$(1)) : CXXOBJECTS += $(2)
$(call gb_LinkTarget_get_clean_target,$(1)) : CXXOBJECTS += $(2)

$(call gb_LinkTarget_get_target,$(1)) : $(call gb_CxxObject_get_target,$(2))
$(call gb_CxxObject_get_target,$(2)) : | $(call
gb_LinkTarget_get_headers_target,$(1))
$(call gb_CxxObject_get_target,$(2)) : CXXFLAGS += $(3) $(ENV_CFLAGS)

ifeq ($(gb_FULLDEPS),$(true))
$(call gb_LinkTarget_get_dep_target,$(1)) : CXXOBJECTS += $(2)
$(call gb_LinkTarget_get_dep_target,$(1)) : $(call gb_CxxObject_get_dep_target,$(2))
endif

endef
Comment 3 mst.ooo 2011-02-15 13:47:03 UTC
one of the requirements for such a variable is that it should be possible to
override the optimization settings.

iirc g++ -O2 .... -O0 would disable optimization, because later arguments
override earlier arguments.

one of the ways of implementing this is to initialize CFLAGS etc. not
unconditionally with empty string, but conditionally.

so if user gives make CFLAGS=-foo it would be used.

but because we append everything to CFLAGS the user-defined stuff would be at
the front, and the gbuild stuff would override it.

another way, like mba has proposed, to add ENV_CFLAGS somewhere in LinkTarget,
has a risk of running into the same problem: what if somewhere else something is
appended?

only way out seems to me to put ENV_CFLAGS directly into compile commands, where
we can guarantee that it comes last.


regardless of the above, we could also, as sb suggested, rename the existing
CFLAGS to gb_CFLAGS or something, and let user specify CFLAGS instead of ENV_CFLAGS.
Comment 4 Stephan Bergmann 2011-02-15 14:36:25 UTC
Note that the old build system calls this "ENVCFLAGS" not "ENV_CFLAGS".
Comment 5 mst.ooo 2011-04-07 12:20:28 UTC
came up with an implementation that uses standard CFLAGS etc. variables,
without requiring the existing target-local variables in gbuild that
have the same name to be renamed.

the user CFLAGS etc. override the debug and optimization flags,
but not anything else.

fixed in CWS gnumake4
http://hg.services.openoffice.org/hg/cws/gnumake4/rev/6af861d98643
Comment 6 Stephan Bergmann 2011-04-07 12:29:55 UTC
@mst, "without requiring the existing target-local variables in gbuild that have the same name to be renamed":  Is it an advantage not to rename the existing target-local variables?
Comment 7 mst.ooo 2011-04-07 13:01:31 UTC
@sb:
unfortunately it is, because user makefiles can reference it.
for example, look at the tools makefiles....
so changing it could lead to breakage :(
Comment 8 Stephan Bergmann 2011-04-07 13:13:40 UTC
then I would argue to do the rename and adapt the client code accordingly; it leads to a cleaner design (we should not yet start to ruin the still-actively-developed gbuild system)
Comment 9 mst.ooo 2011-04-08 09:30:59 UTC
actually what annoys me more than the override is that user makefiles reference the target local variable at all.

perhaps gb_LinkTarget_set_cxxflags should be replaced with gb_LinkTarget_addcxxflags...
Comment 10 mst.ooo 2011-04-11 13:30:47 UTC
now with less ruin:
renamed the target-local variables to T_CFLAGS etc.
also added gb_LinkTarget_add_cflags etc.
perhaps i should remove the set_cflags etc.?
http://hg.services.openoffice.org/hg/cws/gnumake4/rev/8801515221cf

also added user-defined LDFLAGS and CPPFLAGS while we're at it...
(renaming target-local LDFLAGS to T_LDFLAGS)
http://hg.services.openoffice.org/hg/cws/gnumake4/rev/5ed23c4158ac
http://hg.services.openoffice.org/hg/cws/gnumake4/rev/6ae64fc2b62e
Comment 11 Oliver-Rainer Wittmann 2012-06-13 12:28:50 UTC
getting rid of value "enhancement" for field "severity".
For enhancement the field "issue type" shall be used.