Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-10 Thread David Tardon
Hi,

On Wed, Jan 09, 2013 at 03:41:38PM +0100, Petr Mladek wrote:
> PS: I have troubles to understand the related gbuild code. It seems that
> both CXXFLAGS and gb_Library_add_cxxflags are added to the T_CXXFLAGS
> variable, see:
> 
> --- cut solenv/gbuild/LinkTarget.mk ---
> $(call gb_LinkTarget_get_dep_target,$(1)) : T_CXXFLAGS :=
> $$(gb_LinkTarget_CXXFLAGS)
> [...]
> define gb_LinkTarget_add_cxxflags
> $(call gb_LinkTarget_get_headers_target,$(1)) \ 
> $(call gb_LinkTarget_get_target,$(1)) : T_CXXFLAGS += $(2)
> ifeq ($(gb_FULLDEPS),$(true))
> $(call gb_LinkTarget_get_dep_target,$(1)) : T_CXXFLAGS += $(2)
> endif
> endef
> --- cut ---
> 
> It looks like T_CXXFLAGS is initialized with CXXFLAGS and
> gb_LinkTarget_add_cxxflags is added at the end. Though, in reality, they
> are listed in the opposite order. So, I am lost :-)

The problem is in variable inheritance. gb_LinkTarget_add_cxxobject
contains:

$(call gb_CxxObject_get_target,$(2)) : T_CXXFLAGS += $(3)

but there is _no_ T_CXXFLAGS variable defined on the target yet,
therefore make creates a new _recursive_ variable. When it is expanded,
it uses the current T_CXXFLAGS variable visible at the expansion point,
which is the one inherited from the CxxObject's LinkTarget. This is why
T_CXXFLAGS set on LinkTarget end up _before_ the flags passed to
gb_LinkTarget_add_cxxobject (these contain $(CXXFLAGS)).

I think all that is needed is
-$(call gb_CxxObject_get_target,$(2)) : T_CXXFLAGS += $(3)
+$(call gb_CxxObject_get_target,$(2)) : T_OBJECT_CXXFLAGS := $(3)

and then use both T_CXXFLAGS and T_OBJECT_CXXFLAGS in
gb_CxxObject__command. (The same think must then be done for other types
of objects--C, ObjC etc.)

There is still a problem with this: gb_LinkTarget_add_cxxobject can be
used directly in makefiles, in which case the passed flags are specific
for one or more cxx files and should (IMHO) override the LinkTarget
flags, which would not be the case here. On the other side,
gb_LinkTarget_add_cxxobject is mainly used to avoid passing optimization
flags, so hopefully this would not cause a problem.

D.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-10 Thread Lubos Lunak
On Thursday 10 of January 2013, Matúš Kukan wrote:
> On 10 January 2013 09:16, Stephan Bergmann  wrote:
> > Ah, so Petr's "IMHO, it would make sense to use CXXFLAGS after the
> > default global flags but before the source file specific flags" sounds
> > reasonable to me.
>
> See https://gerrit.libreoffice.org/#/c/1632/ for a reasonable easy way
> to do this.
> It should work, etc.
> But it may break PCH_CXXFLAGS. I don't feel like testing --enable-pch
> build.

- for PCH gb_CxxObject__set_pchflags should be modified too.

- shouldn't the new flags be initialized in gb_LinkTarget_LinkTarget?

- (as a general rule) for non-obvious commits, please include in the commit 
log message not only what but also why, such as a link to this thread. In a 
year somebody's bound to want to reshuffle the flags and wonder about the 
commit.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-10 Thread Matúš Kukan
On 10 January 2013 09:16, Stephan Bergmann  wrote:
> Ah, so Petr's "IMHO, it would make sense to use CXXFLAGS after the default
> global flags but before the source file specific flags" sounds reasonable to
> me.

See https://gerrit.libreoffice.org/#/c/1632/ for a reasonable easy way
to do this.
It should work, etc.
But it may break PCH_CXXFLAGS. I don't feel like testing --enable-pch build.

HTH,
Matus
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-10 Thread Stephan Bergmann

On 01/09/2013 06:28 PM, Lubos Lunak wrote:

On Wednesday 09 of January 2013, Stephan Bergmann wrote:

On 01/09/2013 03:41 PM, Petr Mladek wrote:

+ openSUSE uses:

CXXFLAGS=-fomit-frame-pointer ...


What do you mean with "openSUSE uses [that]"?  Is the env var preset
when you run a shell?  Why would openSUSE do that in the first place?


  It's when building RPM packages, which are supposed to be built with
$RPM_OPT_FLAGS.


Ah, so Petr's "IMHO, it would make sense to use CXXFLAGS after the 
default global flags but before the source file specific flags" sounds 
reasonable to me.


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-09 Thread Lubos Lunak
On Wednesday 09 of January 2013, Stephan Bergmann wrote:
> On 01/09/2013 03:41 PM, Petr Mladek wrote:
> > + openSUSE uses:
> >
> > CXXFLAGS=-fomit-frame-pointer ...
>
> What do you mean with "openSUSE uses [that]"?  Is the env var preset
> when you run a shell?  Why would openSUSE do that in the first place?

 It's when building RPM packages, which are supposed to be built with 
$RPM_OPT_FLAGS.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-09 Thread Stephan Bergmann

On 01/09/2013 03:41 PM, Petr Mladek wrote:

+ openSUSE uses:

CXXFLAGS=-fomit-frame-pointer ...


What do you mean with "openSUSE uses [that]"?  Is the env var preset 
when you run a shell?  Why would openSUSE do that in the first place?


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


gmake: CXXFLAGS override gb_Library_add_cxxflags

2013-01-09 Thread Petr Mladek
Hi,

CXXFLAGS are used after gb_Library_add_cxxflags in gbuild.
This causes problems because gb_Library_add_cxxflags usually enforce
some setting that is strictly needed.

For example, my LO-4.0 build for openSUSE crashes in bridge test. It is
related to omit-frame-pointer gcc feature.

+ openSUSE uses:

CXXFLAGS=-fomit-frame-pointer ...

bridges/Library_gcc3_linux_intel.mk includes:

--- cut ---
# In case someone enabled the non-standard -fomit-frame-pointer which
does not
# work with the .cxx sources of this library.
$(eval $(call gb_Library_add_cxxflags,gcc3_uno,\
-fno-omit-frame-pointer \
-fno-strict-aliasing \
$(if $(filter TRUE,$(HAVE_GCC_AVX)),\
-mno-avx \
) \
))
--- cut ---

Unfortunately, CXXFLAGS is listed later on the commandline, so 
the bridge is compiled with the global -f-omit-frame-pointer and is
miscompiled.

IMHO, it would make sense to use CXXFLAGS after the default global flags
but before the source file specific flags.

Another possibility would be to introduce another macro, for example
gb_Library_add_post_cxxflags or gb_Library_add_enforcing_cxxflags.
But then we would need to convert most (if not all)
gb_Library_add_cxxflags calls to the new macro :-)

What do you think?
Could you give me some hints how to do it?

Best Regards,
Petr

PS: I have troubles to understand the related gbuild code. It seems that
both CXXFLAGS and gb_Library_add_cxxflags are added to the T_CXXFLAGS
variable, see:

--- cut solenv/gbuild/LinkTarget.mk ---
$(call gb_LinkTarget_get_dep_target,$(1)) : T_CXXFLAGS :=
$$(gb_LinkTarget_CXXFLAGS)
[...]
define gb_LinkTarget_add_cxxflags
$(call gb_LinkTarget_get_headers_target,$(1)) \ 
$(call gb_LinkTarget_get_target,$(1)) : T_CXXFLAGS += $(2)
ifeq ($(gb_FULLDEPS),$(true))
$(call gb_LinkTarget_get_dep_target,$(1)) : T_CXXFLAGS += $(2)
endif
endef
--- cut ---

It looks like T_CXXFLAGS is initialized with CXXFLAGS and
gb_LinkTarget_add_cxxflags is added at the end. Though, in reality, they
are listed in the opposite order. So, I am lost :-)

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice