Message ID | 20210412165524.17793-1-matthew.weber@rockwellcollins.com |
---|---|
State | Superseded |
Headers | show |
Series | package/ace: new package | expand |
Hello Matt, Thanks for this submission. Please see below some comments. On Mon, 12 Apr 2021 11:55:24 -0500 Matt Weber <matthew.weber@rockwellcollins.com> wrote: > diff --git a/package/ace/Config.in b/package/ace/Config.in > new file mode 100644 > index 0000000000..7755327b21 > --- /dev/null > +++ b/package/ace/Config.in > @@ -0,0 +1,10 @@ > +config BR2_PACKAGE_ACE > + bool "ace" > + depends on BR2_TOOLCHAIN_USES_GLIBC > + depends on !BR2_STATIC_LIBS These means a Config.in comment is needed. > + select BR2_PACKAGE_OPENSSL > + help > + The ADAPTIVE Communication Environment (ACE(TM)) > + An OO Network Programming Toolkit in C++. > + > + See http://www.dre.vanderbilt.edu/~schmidt/ACE.html Drop the "See " at the beginning, we want the last thing in the help text to be just the URL of upstream project home page. > diff --git a/package/ace/ace.mk b/package/ace/ace.mk > new file mode 100644 > index 0000000000..80d50665d9 > --- /dev/null > +++ b/package/ace/ace.mk > @@ -0,0 +1,63 @@ > +################################################################################ > +# > +# ace > +# > +################################################################################ > + > +ACE_VERSION=7.0.1 > +ACE_SOURCE=ACE-$(ACE_VERSION).tar.gz > +ACE_SITE=http://download.dre.vanderbilt.edu/previous_versions > +ACE_DEPENDENCIES=openssl Spaces around = sign. I think this is reported by "make check-package". > +ACE_LICENSE = DOC I checked, this is indeed a valid SPDX identifier. Interesting special license... > +ACE_LICENSE_FILES = COPYING > +ACE_INSTALL_STAGING=YES Spaces around =. > +ACE_CPE_ID_VENDOR = vanderbilt > +ACE_CPE_ID_PRODUCT = adaptive_communication_environment > + > +# configure the target build > +# refer: http://www.dre.vanderbilt.edu/~schmidt/DOC_ROOT/ACE/ACE-INSTALL.html#unix > +define ACE_CONFIGURE_CMDS > + # create a config file > + echo ' #include "ace/config-linux.h" ' > $(@D)/ace/config.h > + # fix link error with ARM EABI tools > + # http://list.isis.vanderbilt.edu/pipermail/ace-users/2008-January/002742.html > + echo 'no_hidden_visibility = 1' > $(@D)/include/makeinclude/platform_macros.GNU > + # create a platform macros file > + echo 'include $(@D)/include/makeinclude/platform_linux.GNU' \ > + >> $(@D)/include/makeinclude/platform_macros.GNU > + # enable ssl > + echo 'ssl = 1' >> $(@D)/include/makeinclude/platform_macros.GNU Does that mean OpenSSL is an optional dependency ? :-) > + # disable RPATH > + echo 'install_rpath = 0' >> $(@D)/include/makeinclude/platform_macros.GNU > + # set the installation prefix > + echo 'INSTALL_PREFIX = /usr' >> $(@D)/include/makeinclude/platform_macros.GNU Why not just provide platform_macros.GNU in package/ace/ instead ? All values are hardcoded at least as it is now. The EABI stuff from 2008 is odd though... > +endef > + > +# Note: We are excluding examples, apps and tests > +# Only compiling ACE libraries (no TAO) > +ACE_LIBARIES = ace ace/SSL ACEXML Kokyu netsvcs protocols/ace > + > +# compile ace,ACEXML, Kokyu, netsvcs & protocols/ace > +define ACE_BUILD_CMDS > + for dir in $(ACE_LIBARIES) ; do \ > + $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" all || exit 1 ; \ > + done Use a make $(foreach ...) loop. > +endef > + > +define ACE_INSTALL_STAGING_CMDS > + # create below folder required by ACE makefiles during install > + mkdir -p $(STAGING_DIR)/usr/share/ace > + for dir in $(ACE_LIBARIES) ; do \ > + $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" DESTDIR=$(STAGING_DIR) install || exit 1 ; \ > + done Same a make $(foreach ...) loop. Will avoid the need for the || exit 1 > +endef > + > +define ACE_INSTALL_TARGET_CMDS > + # create below folder required by ACE makefiles during install > + mkdir -p $(TARGET_DIR)/usr/share/ace > + for dir in $(ACE_LIBARIES) ; do \ > + $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" DESTDIR=$(TARGET_DIR) install || exit 1 ; \ > + done Well, same. Other than that, looks pretty straightforward. Thanks! Thomas
. Thomas, On Mon, Apr 12, 2021 at 3:30 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Matt, > > Thanks for this submission. Please see below some comments. > > On Mon, 12 Apr 2021 11:55:24 -0500 > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > diff --git a/package/ace/Config.in b/package/ace/Config.in > > new file mode 100644 > > index 0000000000..7755327b21 > > --- /dev/null > > +++ b/package/ace/Config.in > > @@ -0,0 +1,10 @@ > > +config BR2_PACKAGE_ACE > > + bool "ace" > > + depends on BR2_TOOLCHAIN_USES_GLIBC > > + depends on !BR2_STATIC_LIBS > > These means a Config.in comment is needed. > > > + select BR2_PACKAGE_OPENSSL > > + help > > + The ADAPTIVE Communication Environment (ACE(TM)) > > + An OO Network Programming Toolkit in C++. > > + > > + See http://www.dre.vanderbilt.edu/~schmidt/ACE.html > > Drop the "See " at the beginning, we want the last thing in the help > text to be just the URL of upstream project home page. > > > diff --git a/package/ace/ace.mk b/package/ace/ace.mk > > new file mode 100644 > > index 0000000000..80d50665d9 > > --- /dev/null > > +++ b/package/ace/ace.mk > > @@ -0,0 +1,63 @@ > > +################################################################################ > > +# > > +# ace > > +# > > +################################################################################ > > + > > +ACE_VERSION=7.0.1 > > +ACE_SOURCE=ACE-$(ACE_VERSION).tar.gz > > +ACE_SITE=http://download.dre.vanderbilt.edu/previous_versions > > +ACE_DEPENDENCIES=openssl > > Spaces around = sign. I think this is reported by "make check-package". It looks like "make check-package"did not catch it but we've cleaned those assignments + other items up in http://patchwork.ozlabs.org/project/buildroot/patch/20210413134139.13281-1-matthew.weber@rockwellcollins.com/ Regards, Matt
diff --git a/DEVELOPERS b/DEVELOPERS index 5c36fc413e..02553a76b0 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1722,6 +1722,7 @@ F: board/qemu/ppc64-e5500/ F: configs/freescale_p* F: configs/freescale_t* F: configs/qemu_ppc64_e5500_defconfig +F: package/ace/ F: package/argp-standalone/ F: package/aufs/ F: package/aufs-util/ diff --git a/package/Config.in b/package/Config.in index f47ee9f1c5..373d85f28f 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1851,6 +1851,7 @@ menu "Networking" endmenu menu "Other" + source "package/ace/Config.in" source "package/appstream-glib/Config.in" source "package/apr/Config.in" source "package/apr-util/Config.in" diff --git a/package/ace/Config.in b/package/ace/Config.in new file mode 100644 index 0000000000..7755327b21 --- /dev/null +++ b/package/ace/Config.in @@ -0,0 +1,10 @@ +config BR2_PACKAGE_ACE + bool "ace" + depends on BR2_TOOLCHAIN_USES_GLIBC + depends on !BR2_STATIC_LIBS + select BR2_PACKAGE_OPENSSL + help + The ADAPTIVE Communication Environment (ACE(TM)) + An OO Network Programming Toolkit in C++. + + See http://www.dre.vanderbilt.edu/~schmidt/ACE.html diff --git a/package/ace/ace.hash b/package/ace/ace.hash new file mode 100644 index 0000000000..92fd42e131 --- /dev/null +++ b/package/ace/ace.hash @@ -0,0 +1,3 @@ +# Locally Computed: +sha256 a28339750620c70cd29a8a7088a4bc6ebaf1ff7ba667498a0279ac97f0e32e01 ACE-7.0.1.tar.gz +sha256 687bf9d16119e0caf6fb5c18214928fd6ea0da10df91e906255b7613af8061d8 COPYING diff --git a/package/ace/ace.mk b/package/ace/ace.mk new file mode 100644 index 0000000000..80d50665d9 --- /dev/null +++ b/package/ace/ace.mk @@ -0,0 +1,63 @@ +################################################################################ +# +# ace +# +################################################################################ + +ACE_VERSION=7.0.1 +ACE_SOURCE=ACE-$(ACE_VERSION).tar.gz +ACE_SITE=http://download.dre.vanderbilt.edu/previous_versions +ACE_DEPENDENCIES=openssl +ACE_LICENSE = DOC +ACE_LICENSE_FILES = COPYING +ACE_INSTALL_STAGING=YES +ACE_CPE_ID_VENDOR = vanderbilt +ACE_CPE_ID_PRODUCT = adaptive_communication_environment + +# configure the target build +# refer: http://www.dre.vanderbilt.edu/~schmidt/DOC_ROOT/ACE/ACE-INSTALL.html#unix +define ACE_CONFIGURE_CMDS + # create a config file + echo ' #include "ace/config-linux.h" ' > $(@D)/ace/config.h + # fix link error with ARM EABI tools + # http://list.isis.vanderbilt.edu/pipermail/ace-users/2008-January/002742.html + echo 'no_hidden_visibility = 1' > $(@D)/include/makeinclude/platform_macros.GNU + # create a platform macros file + echo 'include $(@D)/include/makeinclude/platform_linux.GNU' \ + >> $(@D)/include/makeinclude/platform_macros.GNU + # enable ssl + echo 'ssl = 1' >> $(@D)/include/makeinclude/platform_macros.GNU + # disable RPATH + echo 'install_rpath = 0' >> $(@D)/include/makeinclude/platform_macros.GNU + # set the installation prefix + echo 'INSTALL_PREFIX = /usr' >> $(@D)/include/makeinclude/platform_macros.GNU +endef + +# Note: We are excluding examples, apps and tests +# Only compiling ACE libraries (no TAO) +ACE_LIBARIES = ace ace/SSL ACEXML Kokyu netsvcs protocols/ace + +# compile ace,ACEXML, Kokyu, netsvcs & protocols/ace +define ACE_BUILD_CMDS + for dir in $(ACE_LIBARIES) ; do \ + $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" all || exit 1 ; \ + done +endef + +define ACE_INSTALL_STAGING_CMDS + # create below folder required by ACE makefiles during install + mkdir -p $(STAGING_DIR)/usr/share/ace + for dir in $(ACE_LIBARIES) ; do \ + $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" DESTDIR=$(STAGING_DIR) install || exit 1 ; \ + done +endef + +define ACE_INSTALL_TARGET_CMDS + # create below folder required by ACE makefiles during install + mkdir -p $(TARGET_DIR)/usr/share/ace + for dir in $(ACE_LIBARIES) ; do \ + $(MAKE) -C $(@D)/$${dir} ACE_ROOT="$(@D)" DESTDIR=$(TARGET_DIR) install || exit 1 ; \ + done +endef + +$(eval $(generic-package))