Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] omniorb: new package
@ 2013-09-11 18:13 Matt Weber
  2013-09-12 16:55 ` Thomas Petazzoni
  0 siblings, 1 reply; 2+ messages in thread
From: Matt Weber @ 2013-09-11 18:13 UTC (permalink / raw)
  To: buildroot


Signed-off-by: Matt Weber <mlweber1@rockwellcollins.com>
---
Changes v1 -> v2:
  * Fixed formatting of help and added select of python in  package/omniorb/Config.in (suggested by Thomas)
  * Removed extra open ended config option as it isn't used  package/omniorb/Config.in (suggested by Thomas)
  * Cleaned up use of brackets and replaced with braces package/omniorb/Config.in (suggested by Thomas)
  * Updated licenses to be GPL and LGPL with a + package/omniorb/omniorb.mk (suggested by Thomas)
  * Removed redundant conf opt and target install sets package/omniorb/omniorb.mk (suggested by Thomas)
  * Added a post install step to replace overloading the default staging installs package/omniorb/omniorb.mk (suggested by Thomas)
  * Broke out tools(host) build into seperate post conf step package/omniorb/omniorb.mk (suggested by Thomas)
  * Removed clean and uninstall package/omniorb/omniorb.mk (suggested by Thomas)

 package/Config.in          |    1 +
 package/omniorb/Config.in  |    9 ++++++++
 package/omniorb/omniorb.mk |   50 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 0 deletions(-)
 create mode 100644 package/omniorb/Config.in
 create mode 100644 package/omniorb/omniorb.mk

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..eaa1585
--- /dev/null
+++ b/package/omniorb/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_OMNIORB
+	bool "omniorb"
+	select BR2_PACKAGE_PYTHON
+	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. 
diff --git a/package/omniorb/omniorb.mk b/package/omniorb/omniorb.mk
new file mode 100644
index 0000000..15f7ced
--- /dev/null
+++ b/package/omniorb/omniorb.mk
@@ -0,0 +1,50 @@
+################################################################################
+#
+# 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_LICENSE = GPL2+ LGPLv2.1+
+OMNIORB_LICENSE_FILES = COPYING COPYING.LIB
+OMNIORB_DEPENDENCIES += host-python
+
+ifeq ($(BR2_PACKAGE_PYTHON),y)
+	OMNIORB_DEPENDENCIES += python
+	OMNIORB_CONF_OPT += --enable-python-bindings
+else
+	OMNIORB_CONF_OPT += --disable-python-bindings
+endif
+
+
+# The following is a space-separated list of files that need to have directory fixups
+OMNIORB_FIXUP_FILES = ${STAGING_DIR}/usr/bin/omniidl
+
+# Should be able to create a patch that cleans this up.........
+define OMNIORB_FIXUP_FILE_PATHS_HOOK
+	echo "Fixing file paths..."
+	for i in $(OMNIORB_FIXUP_FILES); do \
+	$(SED) "s:$(HOST_DIR)/usr:/usr:g" $$i; \
+	done
+endef
+OMNIORB_POST_INSTALL_STAGING_HOOKS += OMNIORB_FIXUP_FILE_PATHS_HOOK 
+
+# 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 couple 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_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_TOOLS_HOOK
+
+$(eval $(autotools-package))
+
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Buildroot] [PATCH v2] omniorb: new package
  2013-09-11 18:13 [Buildroot] [PATCH v2] omniorb: new package Matt Weber
@ 2013-09-12 16:55 ` Thomas Petazzoni
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Petazzoni @ 2013-09-12 16:55 UTC (permalink / raw)
  To: buildroot

Dear Matt Weber,

Thanks for this updated version. There are still a few things to fix,
but we're definitely approaching something that can be applied. See
below.

On Wed, 11 Sep 2013 13:13:59 -0500, Matt Weber wrote:

> 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..eaa1585
> --- /dev/null
> +++ b/package/omniorb/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_OMNIORB
> +	bool "omniorb"
> +	select BR2_PACKAGE_PYTHON

You shouldn't select BR2_PACKAGE_PYTHON here: it is not a mandatory
dependency, and your package automatically enables Python bindings when
the target Python package is enabled.

> +	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. 

Please add an empty line here, and then the upstream URL of the project
(http://omniorb.sourceforge.net/). See
http://buildroot.org/downloads/manual/manual.html#writing-rules-config-in.

> diff --git a/package/omniorb/omniorb.mk b/package/omniorb/omniorb.mk
> new file mode 100644
> index 0000000..15f7ced
> --- /dev/null
> +++ b/package/omniorb/omniorb.mk
> @@ -0,0 +1,50 @@
> +################################################################################
> +#
> +# 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_LICENSE = GPL2+ LGPLv2.1+
> +OMNIORB_LICENSE_FILES = COPYING COPYING.LIB
> +OMNIORB_DEPENDENCIES += host-python

Is Python really needed to *build* omniorb? Note that Python is already
a core dependency of Buildroot, so normally there's no need to depend
on host-python, but I agree that this is not a rule that is
consistently applied across Buildroot today.

(Also, note that the += is unnecessary, we usually use a normal = here).

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

This looks good, provided you remove the "select BR2_PACKAGE_PYTHON" in
the Config.in file.

> +# The following is a space-separated list of files that need to have directory fixups
> +OMNIORB_FIXUP_FILES = ${STAGING_DIR}/usr/bin/omniidl
> +
> +# Should be able to create a patch that cleans this up.........
> +define OMNIORB_FIXUP_FILE_PATHS_HOOK
> +	echo "Fixing file paths..."
> +	for i in $(OMNIORB_FIXUP_FILES); do \
> +	$(SED) "s:$(HOST_DIR)/usr:/usr:g" $$i; \
> +	done
> +endef
> +OMNIORB_POST_INSTALL_STAGING_HOOKS += OMNIORB_FIXUP_FILE_PATHS_HOOK 

It seems a bit too much to have a loop when a single file needs to be
fixed. I believe the fixup you're doing here is the right solution, so
no need for a comment that explains it should be turned into a patch.

Also, something that is incorrect in various places in your .mk file:
you should *always* use $(...) to reference make variables, and not
${...}.

So, you can do something like:

define OMNIORB_FIXUP_FILE_PATHS
	$(SED) "s:$(HOST_DIR)/usr:/usr:g" $(STAGING_DIR)/usr/bin/omniidl
endef
OMNIORB_POST_INSTALL_STAGING_HOOKS += OMNIORB_FIXUP_FILE_PATHS

> +# 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 couple tools required for the host side generation of code.
> +# It then leaves/places them in the locations where the existing build infrastructure expects.

Excellent comment. Can you just wrap the lines at a slightly smaller
length?

> +define OMNIORB_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

Please use $(...) instead of ${...} for all variables. Also, you should
use quotes when passing CC or CXX, or you'll break the ccache support
that adds a space within the value of $(HOSTCXX) and $(HOSTCC). And in
fact, instead of passing HOSTCXX and HOSTCC manually, we can just use
$(HOST_CONFIGURE_OPTS) :

	$(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omkdepend
	cp $(@D)/src/tool/omkdepend/omkdepend $(@D)/bin/
	$(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omniidl/cxx/cccp
	$(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D)/src/tool/omniidl/cxx

Thanks!

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-09-12 16:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-11 18:13 [Buildroot] [PATCH v2] omniorb: new package Matt Weber
2013-09-12 16:55 ` Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox