diff mbox

[1/1] omniorb: new package

Message ID 1378418584-21899-1-git-send-email-mlweber1@rockwellcollins.com
State Superseded
Headers show

Commit Message

Matt Weber Sept. 5, 2013, 10:03 p.m. UTC
Target packages -> Libraries -> Networking -> OmniOrb

Signed-off-by: Matt Weber <mlweber1@rockwellcollins.com>
---
 package/Config.in          |    1 +
 package/omniorb/Config.in  |   16 ++++++++++
 package/omniorb/omniorb.mk |   68 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 0 deletions(-)
 create mode 100644 package/omniorb/Config.in
 create mode 100644 package/omniorb/omniorb.mk

Comments

Thomas Petazzoni Sept. 5, 2013, 10:17 p.m. UTC | #1
Dear Matt Weber,

On Thu, 5 Sep 2013 17:03:04 -0500, Matt Weber wrote:

> +config BR2_PACKAGE_OMNIORB
> +	bool "omniorb"
> +	help
> +          omniORB is a robust high performance CORBA ORB for C++ and Python. 

Indentation of those lines should be one tab + two spaces, not two tabs.

> +          omniORB is largely CORBA 2.6 compliant. omniORB is one of only three ORBs 
> +          to have been awarded the Open Group's Open Brand for CORBA. This means that 
> +          omniORB has been tested and certified CORBA compliant, to version 2.1 of the 
> +          CORBA specification. You can find out more about the branding program at 
> +          the Open Group.

These lines looks slightly too long. They should fit in 72 columns if I
remember correctly.

> +config BR2_OMNIORB_EXTRA_CONFIG_OPTIONS
> +	string "Additional omniorb configuration options"
> +	depends on BR2_PACKAGE_OMNIORB
> +	default ""
> +	help
> +	  Any additional configure options you may want to include.

We generally don't offer such 'free' options for packages, so I'd
prefer not having it, unless there are good reasons for it.

> diff --git a/package/omniorb/omniorb.mk b/package/omniorb/omniorb.mk
> new file mode 100644
> index 0000000..ed2a7e7
> --- /dev/null
> +++ b/package/omniorb/omniorb.mk
> @@ -0,0 +1,68 @@
> +################################################################################
> +#
> +# omniorb 
> +#
> +################################################################################
> +
> +OMNIORB_VERSION = 4.1.6
> +OMNIORB_SITE = http://downloads.sourceforge.net/project/omniorb/omniORB/omniORB-${OMNIORB_VERSION}

Please use $(...) instead of ${...} everywhere.

> +OMNIORB_SOURCE = omniORB-${OMNIORB_VERSION}.tar.bz2
> +OMNIORB_INSTALL_STAGING = YES
> +OMNIORB_INSTALL_TARGET = YES

This last line is not needed, that's the default.

> +OMNIORB_DEPENDENCIES = python 

Then this package either needs a depends on BR2_PACKAGE_PYTHON or a
select BR2_PACKAGE_PYTHON. What is omniORB exactly? Is it just a set of
Python modules? Something more than that?

> +OMNIORB_LICENSE = GPL2 LGPLv2.1

Can you check it's not GPLv2+ and LGPLv2.1+ by looking at the copyright
headers in the source code.

> +OMNIORB_LICENSE_FILES = COPYING COPYING.LIB
> +
> +OMNIORB_EXTRA_CONFIG_OPTIONS = $(call qstrip,$(BR2_OMNIORB_EXTRA_CONFIG_OPTIONS))
> +
> +OMNIORB_CONF_OPT = $(OMNIORB_EXTRA_CONFIG_OPTIONS)

As per the above comment, these two lines should probably be removed.

> +
> +# The following is a space-separated list of files that need to have directory fixups
> +OMNIORB_FIXUP_FILES = ${STAGING_DIR}/usr/bin/omniidl
> +
> +define OMNIORB_INSTALL_STAGING_CMDS
> +	echo "Installing to staging dir: $(STAGING_DIR)"
> +	$(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)_INSTALL_STAGING_OPT) -C $($(PKG)_SRCDIR)
> +	for i in $(OMNIORB_FIXUP_FILES); do \
> +	$(SED) "s:$(HOST_DIR)/usr:/usr:g" $$i; \
> +	done
> +endef
> +
> +define OMNIORB_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)_INSTALL_TARGET_OPT) -C $($(PKG)_SRCDIR)
> +endef

For autotools packages, please don't overload the
<pkg>_INSTALL_STAGING_CMDS and <pkg>_INSTALL_TARGET_COMMANDS.

If you need to do some tweaking after the staging installation, use a
POST_INSTALL_STAGING_HOOKS (see the Buildroot manual for details).

> +# OMNIORB has some host tools integrated into it's build.  We first build those, then use
> +# the host IDL2CPP/depend tools to generate code for the target compilation

Hum... I believe a post configure hook might be more appropriate here.
Something like:

define OMNIORB_BUILD_HOST_TOOLS
	$(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omkdepend
	cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
	$(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omniidl/cxx/cccp
	$(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omniidl/cxx
endef

Or, the alternative is to build a complete host variant of omniorb.

> +define OMNIORB_BUILD_CMDS
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omkdepend
> +	cp $($(PKG)_SRCDIR)src/tool/omkdepend/omkdepend $($(PKG)_SRCDIR)/bin
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx/cccp
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx
> +	${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR)
> +endef
> +
> +define OMNIORB_CLEAN_CMDS
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omkdepend clean
> +	$(RM) $($(PKG)_SRCDIR)/bin/omkdepend
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx/cccp clean
> +	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx clean
> +	${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR) clean
> +endef
> +
> +define OMNIORB_UNINSTALL_STAGING_CMDS
> +	$(RM) -r ${STAGING_DIR}/usr/include/omniORB4 ${STAGING_DIR}/usr/include/omnithread
> +	$(RM) ${STAGING_DIR}/usr/include/omniconfig.h ${STAGING_DIR}/usr/include/omnithread.h
> +	$(RM) ${STAGING_DIR}/usr/lib/libomni*
> +	$(RM) ${STAGING_DIR}/usr/bin/omni* ${STAGING_DIR}/usr/bin/omkdepend
> +endef
> +
> +define OMNIORB_UNINSTALL_TARGET_CMDS
> +	$(RM) -r ${TARGET_DIR}/usr/include/omniORB4 ${TARGET_DIR}/usr/include/omnithread
> +	$(RM) ${TARGET_DIR}/usr/include/omniconfig.h ${TARGET_DIR}/usr/include/omnithread.h
> +	$(RM) ${TARGET_DIR}/usr/lib/libomni*
> +	$(RM) ${TARGET_DIR}/usr/bin/omni* ${TARGET_DIR}/usr/bin/omkdepend
> +endef

We generally don't bother implementing uninstall and clean commands.

Best regards,

Thomas
Matt Weber Sept. 6, 2013, 2:08 p.m. UTC | #2
Dear Thomas Petazzoni,

Thank you for the feedback, I've included some comments below.  Let me 
know what you think.

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 09/05/2013 
05:17:06 PM:

> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> To: Matt Weber <mlweber1@rockwellcollins.com>
> Cc: buildroot@busybox.net
> Date: 09/05/2013 05:17 PM
> Subject: Re: [Buildroot] [PATCH 1/1] omniorb: new package
> 
> Dear Matt Weber,
> 
> On Thu, 5 Sep 2013 17:03:04 -0500, Matt Weber wrote:
> 
> > +config BR2_PACKAGE_OMNIORB
> > +   bool "omniorb"
> > +   help
> > +          omniORB is a robust high performance CORBA ORB for C++ 
> and Python. 
> 
> Indentation of those lines should be one tab + two spaces, not two tabs.
> 
> > +          omniORB is largely CORBA 2.6 compliant. omniORB is one 
> of only three ORBs 
> > +          to have been awarded the Open Group's Open Brand for 
> CORBA. This means that 
> > +          omniORB has been tested and certified CORBA compliant, 
> to version 2.1 of the 
> > +          CORBA specification. You can find out more about the 
> branding program at 
> > +          the Open Group.
> 
> These lines looks slightly too long. They should fit in 72 columns if I
> remember correctly.
Agree

> 
> > +config BR2_OMNIORB_EXTRA_CONFIG_OPTIONS
> > +   string "Additional omniorb configuration options"
> > +   depends on BR2_PACKAGE_OMNIORB
> > +   default ""
> > +   help
> > +     Any additional configure options you may want to include.
> 
> We generally don't offer such 'free' options for packages, so I'd
> prefer not having it, unless there are good reasons for it.
At this time there is not, so I'll remove.

> 
> > diff --git a/package/omniorb/omniorb.mk b/package/omniorb/omniorb.mk
> > new file mode 100644
> > index 0000000..ed2a7e7
> > --- /dev/null
> > +++ b/package/omniorb/omniorb.mk
> > @@ -0,0 +1,68 @@
> > 
> 
+################################################################################
> > +#
> > +# omniorb 
> > +#
> > 
> 
+################################################################################
> > +
> > +OMNIORB_VERSION = 4.1.6
> > +OMNIORB_SITE = http://downloads.sourceforge.net/project/omniorb/
> omniORB/omniORB-${OMNIORB_VERSION}
> 
> Please use $(...) instead of ${...} everywhere.
Agree

> 
> > +OMNIORB_SOURCE = omniORB-${OMNIORB_VERSION}.tar.bz2
> > +OMNIORB_INSTALL_STAGING = YES
> > +OMNIORB_INSTALL_TARGET = YES
> 
> This last line is not needed, that's the default.
Agree

> 
> > +OMNIORB_DEPENDENCIES = python 
> 
> Then this package either needs a depends on BR2_PACKAGE_PYTHON or a
> select BR2_PACKAGE_PYTHON. What is omniORB exactly? Is it just a set of
> Python modules? Something more than that?
>
I'll add a select of python to the Config.in.  omniORB is a inter-process 
transport abstraction, similar to DBUS.  We use both the C/C++ and python 
bindings to the object broker server it provides.
 
> > +OMNIORB_LICENSE = GPL2 LGPLv2.1
> 
> Can you check it's not GPLv2+ and LGPLv2.1+ by looking at the copyright
> headers in the source code.
>
I checked the headers and they are 2+ and 2.1+.  I'll update.
 
> > +OMNIORB_LICENSE_FILES = COPYING COPYING.LIB
> > +
> > +OMNIORB_EXTRA_CONFIG_OPTIONS = $(call qstrip,$
> (BR2_OMNIORB_EXTRA_CONFIG_OPTIONS))
> > +
> > +OMNIORB_CONF_OPT = $(OMNIORB_EXTRA_CONFIG_OPTIONS)
> 
> As per the above comment, these two lines should probably be removed.
>
Removed.
 
> > +
> > +# The following is a space-separated list of files that need to 
> have directory fixups
> > +OMNIORB_FIXUP_FILES = ${STAGING_DIR}/usr/bin/omniidl
> > +
> > +define OMNIORB_INSTALL_STAGING_CMDS
> > +   echo "Installing to staging dir: $(STAGING_DIR)"
> > +   $(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)
> _INSTALL_STAGING_OPT) -C $($(PKG)_SRCDIR)
> > +   for i in $(OMNIORB_FIXUP_FILES); do \
> > +   $(SED) "s:$(HOST_DIR)/usr:/usr:g" $$i; \
> > +   done
> > +endef
> > +
> > +define OMNIORB_INSTALL_TARGET_CMDS
> > +   $(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)
> _INSTALL_TARGET_OPT) -C $($(PKG)_SRCDIR)
> > +endef
> 
> For autotools packages, please don't overload the
> <pkg>_INSTALL_STAGING_CMDS and <pkg>_INSTALL_TARGET_COMMANDS.
> 
> If you need to do some tweaking after the staging installation, use a
> POST_INSTALL_STAGING_HOOKS (see the Buildroot manual for details).
>
It doesn't look like I can use POST install hooks for a autotools package 
(I double checked in the pkg-autotools.mk).  However I think I can remove 
the overload of the INSTALL_TARGET_COMMANDS as that's just duplicated of 
what the build already does.
 
> > +# OMNIORB has some host tools integrated into it's build.  We 
> first build those, then use
> > +# the host IDL2CPP/depend tools to generate code for the target 
compilation
> 
> Hum... I believe a post configure hook might be more appropriate here.
> Something like:
> 
> define OMNIORB_BUILD_HOST_TOOLS
>    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C 
$(@D)/src/tool/omkdepend
>    cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
>    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
> tool/omniidl/cxx/cccp
>    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
> tool/omniidl/cxx
> endef
>
It doesn't look like there are POST conf hooks available for autotools 
package. 

I should have added a better comment about this build step.  The package 
has some complicated dependencies that require the tools folders to be 
built with the host(within the target build) and then when the complete 
target package build occurs it ignores the tool folder (since it's already 
built) and builds the rest of the source.  However as part of that source 
build it uses the applications in the tools folder to generate C code from 
IDL (interface definition language) for the build. 
I think I had two options to make this work...
1)Patch the source to allow a host build of the complete package to place 
the right executable(s) into the sysroot.  Then patch the source 
differently for the target build to look in the sysroot instead of it's 
tool's folder for those applications.
2)Overload the build command and insert a host build step manually for 
just those tools applications before the overall target cross compile.
I went with the option that didn't create additional patches to complicate 
if/when the version bumps.
 
> Or, the alternative is to build a complete host variant of omniorb.
> 
> > +define OMNIORB_BUILD_CMDS
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omkdepend
> > +   cp $($(PKG)_SRCDIR)src/tool/omkdepend/omkdepend 
$($(PKG)_SRCDIR)/bin
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omniidl/cxx/cccp
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omniidl/cxx
> > +   ${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR)
> > +endef
> > +
> > +define OMNIORB_CLEAN_CMDS
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omkdepend clean
> > +   $(RM) $($(PKG)_SRCDIR)/bin/omkdepend
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omniidl/cxx/cccp clean
> > +   ${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($
> (PKG)_SRCDIR)src/tool/omniidl/cxx clean
> > +   ${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR) clean
> > +endef
> > +
> > +define OMNIORB_UNINSTALL_STAGING_CMDS
> > +   $(RM) -r ${STAGING_DIR}/usr/include/omniORB4 ${STAGING_DIR}/
> usr/include/omnithread
> > +   $(RM) ${STAGING_DIR}/usr/include/omniconfig.h ${STAGING_DIR}/
> usr/include/omnithread.h
> > +   $(RM) ${STAGING_DIR}/usr/lib/libomni*
> > +   $(RM) ${STAGING_DIR}/usr/bin/omni* 
${STAGING_DIR}/usr/bin/omkdepend
> > +endef
> > +
> > +define OMNIORB_UNINSTALL_TARGET_CMDS
> > +   $(RM) -r ${TARGET_DIR}/usr/include/omniORB4 ${TARGET_DIR}/usr/
> include/omnithread
> > +   $(RM) ${TARGET_DIR}/usr/include/omniconfig.h ${TARGET_DIR}/
> usr/include/omnithread.h
> > +   $(RM) ${TARGET_DIR}/usr/lib/libomni*
> > +   $(RM) ${TARGET_DIR}/usr/bin/omni* ${TARGET_DIR}/usr/bin/omkdepend
> > +endef
> 
> We generally don't bother implementing uninstall and clean commands.
I'll go ahead and remove these.

> 
> Best regards,
> 
> Thomas
> -- 
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com


Thanks!



Matthew L Weber / Sr Software Engineer / Platform SW 
MS 137-157, 855 35th Street NE, Cedar Rapids, IA, 52498, USA
Phone: 319-295-7349 / VPN: 295-7349 
mlweber1@rockwellcollins.com
www.rockwellcollins.com 

CONFIDENTIALITY NOTICE:  This email message is intended only for the 
person or entity to which it is addressed and may contain confidential 
and/or privileged material.  Any unauthorized review, use, disclosure or 
distribution is prohibited.  If you are not  the intended recipient, 
please contact the sender by email and destroy all copies of the original 
message.
Arnout Vandecappelle Sept. 9, 2013, 6:12 a.m. UTC | #3
On 06/09/13 16:08, mlweber1@rockwellcollins.com wrote:
> Dear Thomas Petazzoni,
>
> Thank you for the feedback, I've included some comments below.  Let me
> know what you think.
>
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
> 09/05/2013 05:17:06 PM:
>
>  > From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>  > To: Matt Weber <mlweber1@rockwellcollins.com>
>  > Cc: buildroot@busybox.net
>  > Date: 09/05/2013 05:17 PM
>  > Subject: Re: [Buildroot] [PATCH 1/1] omniorb: new package
>  >
>  > Dear Matt Weber,
>  >
>  > On Thu, 5 Sep 2013 17:03:04 -0500, Matt Weber wrote:
>  >
[snip]
>  > > +OMNIORB_DEPENDENCIES = python
>  >
>  > Then this package either needs a depends on BR2_PACKAGE_PYTHON or a
>  > select BR2_PACKAGE_PYTHON. What is omniORB exactly? Is it just a set of
>  > Python modules? Something more than that?
>  >
> I'll add a select of python to the Config.in.  omniORB is a inter-process
> transport abstraction, similar to DBUS.  We use both the C/C++ and python
> bindings to the object broker server it provides.

  I guess the python bindings are optional, right? Then you shouldn't 
select python in Config.in. Instead, you should make the dependency in 
the .mk file optional. We also try to explicitly set the configure 
options. E.g.

ifeq ($(BR2_PACKAGE_PYTHON),y)
OMNIORB_DEPENDENCIES += python
OMNIORB_CONF_OPT += --enable-python-bindings
else
OMNIORB_CONF_OPT += --disable-python-bindings
endif

(I just inventend the --enable option, of course).

[snip]
>  > > +define OMNIORB_INSTALL_TARGET_CMDS
>  > > +   $(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)
>  > _INSTALL_TARGET_OPT) -C $($(PKG)_SRCDIR)
>  > > +endef
>  >
>  > For autotools packages, please don't overload the
>  > <pkg>_INSTALL_STAGING_CMDS and <pkg>_INSTALL_TARGET_COMMANDS.
>  >
>  > If you need to do some tweaking after the staging installation, use a
>  > POST_INSTALL_STAGING_HOOKS (see the Buildroot manual for details).
>  >
> It doesn't look like I can use POST install hooks for a autotools package
> (I double checked in the pkg-autotools.mk).

  Yes you can. If you look at the top of pkg-generic.mk, you'll see that 
the POST_ hooks are called whatever the package build system is.


>  However I think I can remove
> the overload of the INSTALL_TARGET_COMMANDS as that's just duplicated of
> what the build already does.
>
>  > > +# OMNIORB has some host tools integrated into it's build.  We
>  > first build those, then use
>  > > +# the host IDL2CPP/depend tools to generate code for the target
> compilation
>  >
>  > Hum... I believe a post configure hook might be more appropriate here.
>  > Something like:
>  >
>  > define OMNIORB_BUILD_HOST_TOOLS
>  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C
> $(@D)/src/tool/omkdepend
>  >    cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
>  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
>  > tool/omniidl/cxx/cccp
>  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
>  > tool/omniidl/cxx
>  > endef
>  >
> It doesn't look like there are POST conf hooks available for autotools
> package.

  Same here.


> I should have added a better comment about this build step.  The package
> has some complicated dependencies that require the tools folders to be
> built with the host(within the target build) and then when the complete
> target package build occurs it ignores the tool folder (since it's
> already built) and builds the rest of the source.  However as part of
> that source build it uses the applications in the tools folder to
> generate C code from IDL (interface definition language) for the build.

  Does it actually require them to be installed? Generally, packages are 
smart enough to use something like $(TOPDIR)/tools/omkdepend/omkdepend 
instead of relying on omkdepend to be in the path...

> I think I had two options to make this work...
> 1)Patch the source to allow a host build of the complete package to place
> the right executable(s) into the sysroot.

  That usually doesn't require patching the source. Just adding
$(eval $(host-autotools-package)) is probably enough. And of course you 
should add a dependency on host-omniorb.

>  Then patch the source
> differently for the target build to look in the sysroot instead of it's
> tool's folder for those applications.

  If it looks in the path, then no patching is necessary. You can

  If it doesn't look in the path, then actually autotools has the 
infrastructure to build host utilities during the cross-build. That is 
why CC_FOR_BUILD is passed to configure. So it would be sufficient to 
patch the Makefile.am in the tools directory to use CC_FOR_BUILD (and 
CPPFLAGS_FOR_BUILD and CFLAGS_FOR_BUILD). If you want an example, look at 
yasm or nettle.

  If you patch the build system this way, the patch can be upstreamed and 
we don't have to carry it for eternity.


  Regards,
  Arnout

> 2)Overload the build command and insert a host build step manually for
> just those tools applications before the overall target cross compile.
> I went with the option that didn't create additional patches to
> complicate if/when the version bumps.
>
>  > Or, the alternative is to build a complete host variant of omniorb.
[snip]
Matt Weber Sept. 11, 2013, 2:04 p.m. UTC | #4
Arnout Vandecappelle <arnout@mind.be> wrote on 09/09/2013 01:12:52 AM:

> From: Arnout Vandecappelle <arnout@mind.be>
> To: mlweber1@rockwellcollins.com
> Cc: thomas.petazzoni@free-electrons.com, buildroot@busybox.net
> Date: 09/09/2013 01:45 AM
> Subject: Re: [Buildroot] [PATCH 1/1] omniorb: new package
> 
> On 06/09/13 16:08, mlweber1@rockwellcollins.com wrote:
> > Dear Thomas Petazzoni,
> >
> > Thank you for the feedback, I've included some comments below.  Let me
> > know what you think.
> >
> > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
> > 09/05/2013 05:17:06 PM:
> >
> >  > From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >  > To: Matt Weber <mlweber1@rockwellcollins.com>
> >  > Cc: buildroot@busybox.net
> >  > Date: 09/05/2013 05:17 PM
> >  > Subject: Re: [Buildroot] [PATCH 1/1] omniorb: new package
> >  >
> >  > Dear Matt Weber,
> >  >
> >  > On Thu, 5 Sep 2013 17:03:04 -0500, Matt Weber wrote:
> >  >
> [snip]
> >  > > +OMNIORB_DEPENDENCIES = python
> >  >
> >  > Then this package either needs a depends on BR2_PACKAGE_PYTHON or a
> >  > select BR2_PACKAGE_PYTHON. What is omniORB exactly? Is it just a 
set of
> >  > Python modules? Something more than that?
> >  >
> > I'll add a select of python to the Config.in.  omniORB is a 
inter-process
> > transport abstraction, similar to DBUS.  We use both the C/C++ and 
python
> > bindings to the object broker server it provides.
> 
>   I guess the python bindings are optional, right? Then you shouldn't 
> select python in Config.in. Instead, you should make the dependency in 
> the .mk file optional. We also try to explicitly set the configure 
> options. E.g.
> 
> ifeq ($(BR2_PACKAGE_PYTHON),y)
> OMNIORB_DEPENDENCIES += python
> OMNIORB_CONF_OPT += --enable-python-bindings
> else
> OMNIORB_CONF_OPT += --disable-python-bindings
> endif
> 
> (I just inventend the --enable option, of course).
> 
Agree

> [snip]
> >  > > +define OMNIORB_INSTALL_TARGET_CMDS
> >  > > +   $(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)
> >  > _INSTALL_TARGET_OPT) -C $($(PKG)_SRCDIR)
> >  > > +endef
> >  >
> >  > For autotools packages, please don't overload the
> >  > <pkg>_INSTALL_STAGING_CMDS and <pkg>_INSTALL_TARGET_COMMANDS.
> >  >
> >  > If you need to do some tweaking after the staging installation, use 
a
> >  > POST_INSTALL_STAGING_HOOKS (see the Buildroot manual for details).
> >  >
> > It doesn't look like I can use POST install hooks for a autotools 
package
> > (I double checked in the pkg-autotools.mk).
> 
>   Yes you can. If you look at the top of pkg-generic.mk, you'll see that 

> the POST_ hooks are called whatever the package build system is.
> 
Ok, I'll take a look, I wonder if I possibly had a typo in my naming and 
they weren't getting called.

> 
> >  However I think I can remove
> > the overload of the INSTALL_TARGET_COMMANDS as that's just duplicated 
of
> > what the build already does.
> >
> >  > > +# OMNIORB has some host tools integrated into it's build.  We
> >  > first build those, then use
> >  > > +# the host IDL2CPP/depend tools to generate code for the target
> > compilation
> >  >
> >  > Hum... I believe a post configure hook might be more appropriate 
here.
> >  > Something like:
> >  >
> >  > define OMNIORB_BUILD_HOST_TOOLS
> >  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C
> > $(@D)/src/tool/omkdepend
> >  >    cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
> >  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
> >  > tool/omniidl/cxx/cccp
> >  >    $(HOST_MAKE_ENV) $(HOST) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/
> >  > tool/omniidl/cxx
> >  > endef
> >  >
> > It doesn't look like there are POST conf hooks available for autotools
> > package.
> 
>   Same here.
> 
Ok

> 
> > I should have added a better comment about this build step.  The 
package
> > has some complicated dependencies that require the tools folders to be
> > built with the host(within the target build) and then when the 
complete
> > target package build occurs it ignores the tool folder (since it's
> > already built) and builds the rest of the source.  However as part of
> > that source build it uses the applications in the tools folder to
> > generate C code from IDL (interface definition language) for the 
build.
> 
>   Does it actually require them to be installed? Generally, packages are 

> smart enough to use something like $(TOPDIR)/tools/omkdepend/omkdepend 
> instead of relying on omkdepend to be in the path...
> 

The build doesn't require them to be in the path. It assumes they will 
build as part of the over all pkg build and are located in their 
respective 
folders in the pkg source.  Then they're used at a later point in the same 
build to 
generate C code from a description language...  It would take modification 
to 
the package's pathing to get this fixed (support use of host approach).

> > I think I had two options to make this work...
> > 1)Patch the source to allow a host build of the complete package to 
place
> > the right executable(s) into the sysroot.
> 
>   That usually doesn't require patching the source. Just adding
> $(eval $(host-autotools-package)) is probably enough. And of course you 
> should add a dependency on host-omniorb.

The build currently isn't smart enough to look in the path, it is assuming 

you're doing a build for the current host architecture and not cross 
building.
They assume the tools files are being built as part of the overall build 
and 
used during the later half of the overall build.  Would the following be 
acceptable
for now until the package improves it's infrastructure for cross builds? 

# OMNIORB is currently not cross-compile friendly and has some assumptions 
where
# a couple host tools are built in place and then used during the build. 
The tools
# generate code from the IDL description language, which is then built 
into the.
# cross compiled OMNIORB application.
# So this hook first builds the tools required for the host side 
generation of code.
# It then leaves/places them in the locations where the existing build 
infrastructure expects.
define OMNIORB_BUILD_TOOLS_HOOK
>.......${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C 
$(@D)/src/tool/omkdepend
>.......cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
>.......${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C 
$(@D)/src/tool/omniidl/cxx/cccp
>.......${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C 
$(@D)/src/tool/omniidl/cxx
endef
OMNIORB_POST_CONFIGURE_HOOKS += OMNIORB_BUILD_TOOLS_HOOK

> 
> >  Then patch the source
> > differently for the target build to look in the sysroot instead of 
it's
> > tool's folder for those applications.
> 
>   If it looks in the path, then no patching is necessary. You can
Ok

> 
>   If it doesn't look in the path, then actually autotools has the 
> infrastructure to build host utilities during the cross-build. That is 
> why CC_FOR_BUILD is passed to configure. So it would be sufficient to 
> patch the Makefile.am in the tools directory to use CC_FOR_BUILD (and 
> CPPFLAGS_FOR_BUILD and CFLAGS_FOR_BUILD). If you want an example, look 
at 
> yasm or nettle.
> 
>   If you patch the build system this way, the patch can be upstreamed 
and 
> we don't have to carry it for eternity.

It looks like the autotools version used for the package doesn't have 
CC_FOR_BUILD. 
I wish I could do it this way, since the files I need it for just need to 
be build 
for the host architecture and left in the tools/bin folder for use by the 
cross build. 
It would be perfect to have a single package build satisfy this.  I'll 
investigate 
if there is any traction on improving the cross building of this package.

> 
> 
>   Regards,
>   Arnout
> 
> > 2)Overload the build command and insert a host build step manually for
> > just those tools applications before the overall target cross compile.
> > I went with the option that didn't create additional patches to
> > complicate if/when the version bumps.
> >
> >  > Or, the alternative is to build a complete host variant of omniorb.
> [snip]
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F




Matthew L Weber / Sr Software Engineer / Platform SW 
mlweber1@rockwellcollins.com
www.rockwellcollins.com
Arnout Vandecappelle Sept. 11, 2013, 4:37 p.m. UTC | #5
On 11/09/13 16:04, mlweber1@rockwellcollins.com wrote:
> Arnout Vandecappelle <arnout@mind.be> wrote on 09/09/2013 01:12:52 AM:
>
>  > From: Arnout Vandecappelle <arnout@mind.be>
>  > To: mlweber1@rockwellcollins.com
>  > Cc: thomas.petazzoni@free-electrons.com, buildroot@busybox.net
>  > Date: 09/09/2013 01:45 AM
>  > Subject: Re: [Buildroot] [PATCH 1/1] omniorb: new package
[snip]
>  >   If it doesn't look in the path, then actually autotools has the
>  > infrastructure to build host utilities during the cross-build. That is
>  > why CC_FOR_BUILD is passed to configure. So it would be sufficient to
>  > patch the Makefile.am in the tools directory to use CC_FOR_BUILD (and
>  > CPPFLAGS_FOR_BUILD and CFLAGS_FOR_BUILD). If you want an example, look at
>  > yasm or nettle.
>  >
>  >   If you patch the build system this way, the patch can be upstreamed and
>  > we don't have to carry it for eternity.
>
> It looks like the autotools version used for the package doesn't have
> CC_FOR_BUILD.
> I wish I could do it this way, since the files I need it for just need to
> be build
> for the host architecture and left in the tools/bin folder for use by the
> cross build.
> It would be perfect to have a single package build satisfy this.  I'll
> investigate
> if there is any traction on improving the cross building of this package.

  I've also taken a look, and the build system is indeed horribly broken.

  Unfortunately, your solution still has an important shortcoming: it 
will install the host binaries for those tools to the target. I think the 
solution is rather simple:

define OMNIORB_BUILD_TOOLS
	$(HOST_MAKE_ENV) $(MAKE) CXX=$(HOSTCXX) CC=$(HOSTCC) \
		-C $(@D)/src/tool export
endef

OMNIORB_POST_CONFIGURE_HOOKS += OMNIORB_BUILD_TOOLS

define OMNIORB_CLEAN_TOOLS
	$(HOST_MAKE_ENV) $(MAKE) CXX=$(HOSTCXX) CC=$(HOSTCC) \
		-C $(@D)/src/tool clean
endef

OMNIORB_POST_BUILD_HOOKS += OMNIORB_CLEAN_TOOLS

  This will build the tools and install them locally in the build tree. 
After building, the tools are cleaned again, so that the install target 
will re-build them. It's a huge hack so make sure to add some explanatory 
comments.

  Note: I haven't tried this :-)

  BTW Ideally the POST_CONFIGURE hook should be a PRE_BUILD hook but we 
don't have that.

  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index a94cb62..b4cc869 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -600,6 +600,7 @@  source "package/libupnp/Config.in"
 source "package/libvncserver/Config.in"
 source "package/libwebsockets/Config.in"
 source "package/nss-mdns/Config.in"
+source "package/omniorb/Config.in"
 source "package/openpgm/Config.in"
 source "package/ortp/Config.in"
 source "package/slirp/Config.in"
diff --git a/package/omniorb/Config.in b/package/omniorb/Config.in
new file mode 100644
index 0000000..4c8c3fc
--- /dev/null
+++ b/package/omniorb/Config.in
@@ -0,0 +1,16 @@ 
+config BR2_PACKAGE_OMNIORB
+	bool "omniorb"
+	help
+          omniORB is a robust high performance CORBA ORB for C++ and Python. 
+          omniORB is largely CORBA 2.6 compliant. omniORB is one of only three ORBs 
+          to have been awarded the Open Group's Open Brand for CORBA. This means that 
+          omniORB has been tested and certified CORBA compliant, to version 2.1 of the 
+          CORBA specification. You can find out more about the branding program at 
+          the Open Group.
+
+config BR2_OMNIORB_EXTRA_CONFIG_OPTIONS
+	string "Additional omniorb configuration options"
+	depends on BR2_PACKAGE_OMNIORB
+	default ""
+	help
+	  Any additional configure options you may want to include.
diff --git a/package/omniorb/omniorb.mk b/package/omniorb/omniorb.mk
new file mode 100644
index 0000000..ed2a7e7
--- /dev/null
+++ b/package/omniorb/omniorb.mk
@@ -0,0 +1,68 @@ 
+################################################################################
+#
+# omniorb 
+#
+################################################################################
+
+OMNIORB_VERSION = 4.1.6
+OMNIORB_SITE = http://downloads.sourceforge.net/project/omniorb/omniORB/omniORB-${OMNIORB_VERSION}
+OMNIORB_SOURCE = omniORB-${OMNIORB_VERSION}.tar.bz2
+OMNIORB_INSTALL_STAGING = YES
+OMNIORB_INSTALL_TARGET = YES
+OMNIORB_DEPENDENCIES = python 
+OMNIORB_LICENSE = GPL2 LGPLv2.1
+OMNIORB_LICENSE_FILES = COPYING COPYING.LIB
+
+OMNIORB_EXTRA_CONFIG_OPTIONS = $(call qstrip,$(BR2_OMNIORB_EXTRA_CONFIG_OPTIONS))
+
+OMNIORB_CONF_OPT = $(OMNIORB_EXTRA_CONFIG_OPTIONS)
+
+# The following is a space-separated list of files that need to have directory fixups
+OMNIORB_FIXUP_FILES = ${STAGING_DIR}/usr/bin/omniidl
+
+define OMNIORB_INSTALL_STAGING_CMDS
+	echo "Installing to staging dir: $(STAGING_DIR)"
+	$(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)_INSTALL_STAGING_OPT) -C $($(PKG)_SRCDIR)
+	for i in $(OMNIORB_FIXUP_FILES); do \
+	$(SED) "s:$(HOST_DIR)/usr:/usr:g" $$i; \
+	done
+endef
+
+define OMNIORB_INSTALL_TARGET_CMDS
+	$(TARGET_MAKE_ENV) $($(PKG)_MAKE_ENV) $($(PKG)_MAKE) $($(PKG)_INSTALL_TARGET_OPT) -C $($(PKG)_SRCDIR)
+endef
+
+# OMNIORB has some host tools integrated into it's build.  We first build those, then use
+# the host IDL2CPP/depend tools to generate code for the target compilation
+define OMNIORB_BUILD_CMDS
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omkdepend
+	cp $($(PKG)_SRCDIR)src/tool/omkdepend/omkdepend $($(PKG)_SRCDIR)/bin
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx/cccp
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx
+	${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR)
+endef
+
+define OMNIORB_CLEAN_CMDS
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omkdepend clean
+	$(RM) $($(PKG)_SRCDIR)/bin/omkdepend
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx/cccp clean
+	${HOST_MAKE_ENV} ${MAKE} CXX=${HOSTCXX} CC=${HOSTCC} -C $($(PKG)_SRCDIR)src/tool/omniidl/cxx clean
+	${TARGET_MAKE_ENV} ${MAKE} -C $($(PKG)_SRCDIR) clean
+endef
+
+define OMNIORB_UNINSTALL_STAGING_CMDS
+	$(RM) -r ${STAGING_DIR}/usr/include/omniORB4 ${STAGING_DIR}/usr/include/omnithread
+	$(RM) ${STAGING_DIR}/usr/include/omniconfig.h ${STAGING_DIR}/usr/include/omnithread.h
+	$(RM) ${STAGING_DIR}/usr/lib/libomni*
+	$(RM) ${STAGING_DIR}/usr/bin/omni* ${STAGING_DIR}/usr/bin/omkdepend
+endef
+
+define OMNIORB_UNINSTALL_TARGET_CMDS
+	$(RM) -r ${TARGET_DIR}/usr/include/omniORB4 ${TARGET_DIR}/usr/include/omnithread
+	$(RM) ${TARGET_DIR}/usr/include/omniconfig.h ${TARGET_DIR}/usr/include/omnithread.h
+	$(RM) ${TARGET_DIR}/usr/lib/libomni*
+	$(RM) ${TARGET_DIR}/usr/bin/omni* ${TARGET_DIR}/usr/bin/omkdepend
+endef
+
+$(eval $(autotools-package))
+