Fwd: Re: [PATCH][REVIEW] gbuild conversion: idlc module -- problem with --enable-werror

Hi,

my patch (that is apparently not pushed yet ;-) breaks the build with 
--enable-werror option enabled.
I didn't try it yersterday, sorry for that.

I have two error: in C and in C+ code.

I removed the patch and compiled idlc with old dmake with 
--enable-werror enabled.
It worked.

C-code error:

The dfference between the two is:
old dmake build: only -Werror option on C code is present,
new gbuild build: -Werror and -Wshadow are present.

The reason is here:

on dmake:
solenv/inc/unxgcc.mk
# -Wshadow does not work for C with nested uses of pthread_cleanup_push:
CFLAGSWARNBOTH=-Wall -Wextra -Wendif-labels

on gbuild:

solenv/gbuild/platform/unxgcc.mk
gb_CFLAGS := \
    $(gb_CFLAGS_COMMON) \
    -fPIC \
    -Wdeclaration-after-statement \
    -Wshadow \


The C error that I'm getting is:

home/david/projects/libreoffice-core/git/libo/idlc/source/preproc/eval.c: In 
Funktion »eval«:
/home/david/projects/libreoffice-core/git/libo/idlc/source/preproc/eval.c:265:15: 
Fehler: Deklaration von »rand« überdeckt eine globale Deklaration 
[-Werror=shadow]

translated:
Declaration of `rand` shadows a global declaration [-Werror=shadow]

With -Wshadow option commented out, the new build with my patch works again.
gb_CFLAGS := \
    $(gb_CFLAGS_COMMON) \
    -fPIC \
    -Wdeclaration-after-statement \
#    -Wshadow \


C++ error is about bison generated parser:

idlc/source/parser.y:261:13: Fehler: #pragma system_header außerhalb 
include-Datei ignoriert [-Werror]
source/parser.y: In Funktion »int yyparse()«:
source/parser.y:604:65: Fehler: Um Zuweisung, die als Wahrheitswert 
verwendet wird, werden Klammern empfohlen [-Werror=parentheses]

I'm investigating this.

David

-------- Original Message --------
Subject:        Re: [PATCH][REVIEW] gbuild conversion: idlc module
Date:   Thu, 12 Apr 2012 23:45:51 +0200
From:   David Ostrovsky <david.ostrovsky@gmx.de>
To: 	David Tardon <dtardon@redhat.com>, Matús( Kukan 
<matus.kukan@gmail.com>
CC:     Libreoffice-dev <libreoffice@lists.freedesktop.org>



Hi David,
hi Matus,

thank you for your comments on my patch. I changed it accordingly.

*@Matú?:* thank you for your help on IRC.

Special usage gnu_getopt lib in windows:
Matú? proposed the follow simple solution to include in 
Executable_idlcpp.mk:

ifeq ($(OS),WNT)
$(eval $(call gb_Executable_use_libraries,idlcpp,\
    gnu_getopt \
))
endif

because

1. HAVE_GETOPT is allways NO on windows

2. gnu_getopt is registered already on windows: 
gb_Library_PLAINLIBS_NONE += [...] gnu_getopt [...]
here solenv/gbuild/platform/WNT_INTEL_GCC.mk
and here solenv/gbuild/platform/WNT_INTEL_MSC.mk

3. original make file snippet was only active on windows

.IF "$(HAVE_GETOPT)" != "YES"
.IF "$(GUI)$(COM)" == "WNTMSC"
APP1STDLIBS=gnu_getopt.lib
.ELIF "$(GUI)$(COM)" == "WNTGCC"
APP1STDLIBS=-lgnu_getopt
.ENDIF
.ENDIF

Having said that, how could we simplify the gnu_getopt treatment in 
tools/Executable_rscdep.mk?

David

-------- Original Message --------
Subject:        Re: [PATCH][REVIEW] gbuild conversion: idlc module
Date:   Thu, 12 Apr 2012 06:33:19 +0200
From:   David Tardon <dtardon@redhat.com>
To:     David Ostrovsky <david.ostrovsky@gmx.de>
CC:     Libreoffice-dev <libreoffice@lists.freedesktop.org>



On Wed, Apr 11, 2012 at 11:18:46PM +0200, David Ostrovsky wrote:
>  Hi,
>
>  this is gbuild conversion for idlc module.

Good. But it is not quite ready yet, see comments below.

>  diff --git a/Repository.mk b/Repository.mk
>  index 1a3851e..638f4f8 100644
>  --- a/Repository.mk
>  +++ b/Repository.mk
>  @@ -31,6 +31,8 @@ $(eval $(call gb_Helper_register_executables,NONE, \
>       bmp \
>       bmpsum \
>       cppunit/cppunittester \
>  +    idlc \
>  +    idlcpp \
>       g2g \
>       gencoll_rule \
>       genconv_dict \

Both binaries should be registered in layer SDKBIN (because of
APP1RPATH=SDK).

>  +
>  +$(eval $(call gb_Executable_use_libraries,idlc,\
>  +    sal \
>  +    reg \

Originally idlc is linked with salhelper too. It is possible it is no
longer necessary, but it is also possible that it is necessary on a
different platform. (Nitpick: please, keep the libs sorted
alphabetically :-)

>  +))
>  +$(eval $(call gb_Executable_use_linked_libs,idlcpp,\
>  +    gnu_getopt \
>  +))

This is wrong: there is no unconditionally built library gnu_getopt. It
only worked for you because there is no function
gb_Executable_use_linked_libs either (it is called
gb_Executable_use_libraries) and your platform's libc has getopt .-)

You need to use gb_Executable_use_externals and add a new definition to
RepositoryExternals.mk for it, like:

ifeq ($(HAVE_GETOPT),YES)

# nothing needed
define gb_LinkTarget__use_gnu_getopt

endef

else # !HAVE_GETOPT

define gb_LinkTarget__use_gnu_getopt
$(call gb_LinkTarget_use_libraries,$(1),\
    gnu_getopt \
)

endef

$(eval $(call gb_Helper_register_libraries,PLAINLIBS_NONE,\
    gnu_getopt \
))

endif # HAVE_GETOPT

D.