Re: [PATCH][REVIEW] gbuild conversion: idlc module

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.