[Buildroot] [PATCH v2 1/3] package/openpowerlink2: new package

Romain Naour romain.naour at openwide.fr
Wed May 6 12:20:21 UTC 2015


Hi Arnout,

Le 02/05/2015 16:53, Arnout Vandecappelle a écrit :
> On 01/05/15 17:35, Romain Naour wrote:
>> The openpowerlink2 package archive contains several cmake projects,
>> and each of them must be packaged separately in Buildroot.
>>
>> "With version 2.0, the source code has been cleanly split into an
>> application-oriented user library and a time-critical stack driver."
> 
>  Is the library's API also sufficiently different to warrant making it
> powerlink2? It's difficult to evaluate for us since we don't have anything that
> makes use of it in buildroot...

No, it's a complete rewrite of the powerlink stack, so the API has changed.
Example: EPLApiInitialize() became oplk_init().
And some openpowerlink variable type has changed...

> 
>>
>> This complicates the packaging but it help to refine the dependencies
>                                         helps
> 
>> compared to openpowerlink v1 build system.
>> (openpowerlink v1 require C++ only for the qt demo)
>                     requires
> 
>>
>> The generic openpowerlink2 package provide all sources files and
>> patches which are then used by other openpowerlink2-* packages
>> (sub-packages).
>> Doing this avoids patches duplication over all sub-packages.
>>
>> All patches fixes several issues with the cmake build system.
>                fix
> 
>  What's the upstream status of these patches?
> 
>>
>> For each sub-packages, the extract hook create a symlink to the
>                                           creates
> 
>> sources and the path to the cmake project is provided by *_SUBDIR
>> value.
> 
>  Ouch, I really really don't like that. I'd much prefer to re-extract it from
> the same source for each individual package - a bit like what we do with gcc.
> The patches can be symlinked or applied with a hook (like is done for gcc).

Ok, I'll try with a hook then. But it seems that the build system is not really
sysroot aware, I'm expecting some build issue with the demo apps.

> 
>>
>> There is one project for the EPL stack libraries wich provides:
> 
>  Perhaps explain what EPL is.

EPL means Ethernet PowerLink but it seems that upstream use opkl now.

> 
>>  * an user space EPL stack to be linked into application
>>  * an user space EPL stack pcap daemon
>>  * an kernel space EPL library interface
>>
>> There is two projects for EPL network driver:
>         are               the
> 
>>  * pcap daemon driver
>>  * kernel module driver.
>>
>> Note: On x86/x86_64 only few EPL ethernet driver are available for
>                            a                drivers
> 
>> the EPL kernel stack implementation:
>>  * Intel 82573
>>  * Intel 8255x
>>  * Intel I210
>>  * Realtek RTL-8111/8168 (new since V2.1.0)
>>  * Realtek RTL-8139
>>
>> There are one project for each demo applications:
>         is                            application
> 
>>  * demo_cn_embedded
>>  * demo_mn_embedded
>>  * demo_cn_console
>>  * demo_mn_console
>>  * demo_mn_qt
>>
>> Only demo_mn_console will be packaged in Buildroot for now.
>>
>> This patch add the package for the stack libraries.
>              adds
> 
>  This patch actually does two things: it adds the overall openpowerlink
> infrastructure, and it adds the stack libraries. So it should be split into two
> patches: one which adds the openpowerlink2 package, and one which adds the
> openpowerlink2-stack package.

I'll try to merge openpowerlink2 and openpowerlink2-stack into the same
Buildroot's package to simplify thanks to the apply-patch hook.

> 
>  Also, the policy is not to create more subdirectories. So openpowerlink2-stack
> should appear directly under packages/.

OK.

> 
>>
>> [1] http://sourceforge.net/p/openpowerlink/discussion/newbie/thread/3f13af65/
>>
> [snip]
>> diff --git a/package/openpowerlink2/0002-FIX-Don-t-link-demos-with-Debug-and-Release-librarie.patch b/package/openpowerlink2/0002-FIX-Don-t-link-demos-with-Debug-and-Release-librarie.patch
>> new file mode 100644
>> index 0000000..16602ac
>> --- /dev/null
>> +++ b/package/openpowerlink2/0002-FIX-Don-t-link-demos-with-Debug-and-Release-librarie.patch
>> @@ -0,0 +1,116 @@
>> +From 545fc1d84a224093f7f79b5193aa3f7308f86b3a Mon Sep 17 00:00:00 2001
>> +From: Romain Naour <romain.naour at openwide.fr>
>> +Date: Wed, 17 Sep 2014 13:45:19 +0200
>> +Subject: [PATCH] [FIX] Don't link demos with Debug and Release libraries
>> +
>> +This is acutualy used by Visual Studio:
>            actually
> 
>> +
>> +"When you create a Visual Studio Solution on Windows by CMake,
>> +the solution contains both Release and Debug build configurations.
>> +You can switch between them in Visual Studio.
>> +Therefore, you need both release and debug libraries."
>> +
>> +This patch break the build for Windows.
> 
>  I don't by that.
> 
>  Yes, you'll need to build both Release and Debug libraries. But you need to
> link with only one of them.
> 
>  But in fact, that's exactly what TARGET_LINK_LIBRARIES does [1] - the libraries
> after 'optimized' will be used in all configurations except Debug, and the
> target after 'debug' will be used in the Debug configuration. So this patch is
> doing what CMake should already do according to the documentation...
> However, it turns out that CMake checks if ${OPLKLIB_DEBUG} exists even if we
> don't need it because we're not in a Debug configuration.

Since I reported this issue, a note has been added to the documentation:
doc/build-stack.md but the issue hasn't be fixed...

> 
>  So, I think this patch should be acceptable upstream with an adapted commit
> message:
> 
> [FIX] Don't link demos with Debug and Release libraries
> 
> TARGET_LINK_LIBRARIES() will link only with the 'debug' libraries in debug mode
> and only with the 'optimized' libraries in any other mode, but CMake still
> checks if the libraries that you don't link with exist. That makes it impossible
> to release just an optimized library without debug library [1].
> 
> Therefore, this patch provides an alternative way to distinguish between debug
> and optimized, with an explicit condition.
> 
> [1] http://sourceforge.net/p/openpowerlink/discussion/newbie/thread/3f13af65/

Thanks, commit log updated :)

>> +
>> +Signed-off-by: Romain Naour <romain.naour at openwide.fr>
>> +---
>> + apps/demo_cn_console/CMakeLists.txt  |    6 +++++-
>> + apps/demo_cn_embedded/CMakeLists.txt |    6 +++++-
>> + apps/demo_mn_console/CMakeLists.txt  |    6 +++++-
>> + apps/demo_mn_embedded/CMakeLists.txt |    6 +++++-
>> + apps/demo_mn_qt/CMakeLists.txt       |    6 +++++-
>> + 5 files changed, 25 insertions(+), 5 deletions(-)
>> +
>> +diff --git a/apps/demo_cn_console/CMakeLists.txt b/apps/demo_cn_console/CMakeLists.txt
>> +index 2ba2bbd..4051bc8 100644
>> +--- a/apps/demo_cn_console/CMakeLists.txt
>> ++++ b/apps/demo_cn_console/CMakeLists.txt
>> +@@ -105,8 +105,12 @@ SET_PROPERTY(TARGET demo_cn_console
>> + 
>> + ################################################################################
>> + # Libraries to link
>> ++IF(${CMAKE_BUILD_TYPE} STREQUAL "Debug")
>> ++    TARGET_LINK_LIBRARIES(demo_cn_console debug ${OPLKLIB_DEBUG})
> 
>  The 'debug' part is now redundant.
> 
>> ++ELSE ()
> 
>  The coding style seems to be not to insert a space here.
> 
>> ++    TARGET_LINK_LIBRARIES(demo_cn_console optimized ${OPLKLIB})
> 
>  And here the optimized part.

Yes indeed.

> 
>> ++ENDIF()
>> + 
> [snip]
> 
>> diff --git a/package/openpowerlink2/0003-FIX-install-the-stack-libraries-to-lib-subdirectory.patch b/package/openpowerlink2/0003-FIX-install-the-stack-libraries-to-lib-subdirectory.patch
>> new file mode 100644
>> index 0000000..9d68288
>> --- /dev/null
>> +++ b/package/openpowerlink2/0003-FIX-install-the-stack-libraries-to-lib-subdirectory.patch
>> @@ -0,0 +1,108 @@
>> +From e322ca01613f6a51f1465711c2f890a372053a30 Mon Sep 17 00:00:00 2001
>> +From: Romain Naour <romain.naour at openwide.fr>
>> +Date: Fri, 1 May 2015 12:19:34 +0200
>> +Subject: [PATCH] [FIX] install the stack libraries to "lib" subdirectory
>> +
>> +Using '.' to install the stack libraries is not correct since
>> +it will install them to /usr/ when CMAKE_INSTALL_PREFIX is set
> 
>  CMAKE_INSTALL_PREFIX isn't needed here, CMake will add that automatically.
> 
>> +to "/usr/".
>> +
>> +ls /usr/liboplkmnapp-kernelintf.so
>> +
>> +Fix this by using ${CMAKE_INSTALL_PREFIX}/lib instead of '.'
> 
>  I guess there must be a reason why upstream does this crazy sh*t, so probably
> this patch will be harder for them to accept...
> 
> [snip]
>> diff --git a/package/openpowerlink2/Config.in b/package/openpowerlink2/Config.in
>> new file mode 100644
>> index 0000000..8e61a7d
>> --- /dev/null
>> +++ b/package/openpowerlink2/Config.in
>> @@ -0,0 +1,46 @@
>> +comment "openpowerlink2 needs a toolchain w/ threads"
>> +	depends on BR2_i386 || BR2_x86_64
>> +	depends on !BR2_TOOLCHAIN_HAS_THREADS
> 
>  Did you test with uClibc and/or musl?

No, only with a glibc toolchain, you know my cs ia32 toolchain 2014.11
sgxx-glibc ;-)
Ok, I'll try just with a test build.

> 
>> +
>> +menuconfig BR2_PACKAGE_OPENPOWERLINK2
>> +	bool "openpowerlink2"
>> +	depends on BR2_TOOLCHAIN_HAS_THREADS
>> +	depends on BR2_i386 || BR2_x86_64
>> +	help
>> +	  openPOWERLINK2 is an Open Source Industrial Ethernet
>> +	  stack implementing the POWERLINK protocol for Managing Node
>> +	  (MN, POWERLINK Master) and Controlled Node (CN, POWERLINK Slave).
>> +
>> +	  It is provided by
>> +	  SYSTEC electronic (http://www.systec-electronic.com),
>> +	  B&R (http://www.br-automation.com) and
>> +	  Kalycito (http://www.kalycito.com).
>> +
>> +	  https://sourceforge.net/projects/openpowerlink/
> 
>  This refers to http://openpowerlink.sourceforge.net/web/ as the website.

Ok

> 
>> +
>> +if BR2_PACKAGE_OPENPOWERLINK2
>> +
>> +choice
>> +	prompt "Select MN/CN mode"
>> +
>> +	config BR2_PACKAGE_OPENPOWERLINK2_MN
>> +	bool "MN"
>> +	help
>> +	  Enable Managing Node mode
> 
>  Perhaps add (Master) here.

ok

> 
>> +
>> +	config BR2_PACKAGE_OPENPOWERLINK2_CN
>> +	bool "CN"
>> +	help
>> +	  Enable Controlled Node mode

I also added (slave) here.

>> +
>> +endchoice
>> +
>> +source "package/openpowerlink2/openpowerlink2-stack/Config.in"
>> +
>> +config BR2_PACKAGE_OPENPOWERLINK2_DEBUG_LEVEL
>> +	string "debug level for openpowerlink stack"
>> +	default "0xEC000000L"
>> +	help
>> +	  Debug level to be used for openPOWERLINK debugging functions.
> 
>  Maybe slightly more explanation what the bits mean? At least you should explain
> that it has to be a hex number starting with 0x and ending with L.
> 
>  Why do you have this default?

Well, it's the default value I found in stack/CMakeLists.txt.
I added a link to the header file where the bit field is defined (oplk/debug.h).
I'm not sure that L is really necessary.

> 
>> +
>> +endif # BR2_PACKAGE_OPENPOWERLINK2
>> diff --git a/package/openpowerlink2/openpowerlink2-stack/Config.in b/package/openpowerlink2/openpowerlink2-stack/Config.in
>> new file mode 100644
>> index 0000000..c3ddf47
>> --- /dev/null
>> +++ b/package/openpowerlink2/openpowerlink2-stack/Config.in
>> @@ -0,0 +1,34 @@
>> +
>> +choice
>> +	prompt "Select openPOWERLINK library type"
>> +
>> +config BR2_PACKAGE_OPENPOWERLINK2_STACK_MONOLITHIC_USER_STACK_LIB
>> +	bool "the EPL stack is directly linked into application."
>> +	select BR2_PACKAGE_LIBPCAP
>> +	help
>> +	  Compile a monolithic openPOWERLINK library. The library contains
>> +	  an Ethernet driver which is using the PCAP library for accessing
>> +	  the network.
> 
>  Add "No kernel-side driver is needed."
> 
>  Or actually, replace driver with stack everywhere.

They use "Linux user space driver" for PCAP Ethernet driver. I'm trying to
follow to documentation found in build-stack.md. So, I want to avoid another
terminology here.

> 
>> +
>> +config BR2_PACKAGE_OPENPOWERLINK2_STACK_USERSPACE_DAEMON_LIB
>> +	bool "build EPL stack as linux userspace pcap daemon."
>> +	select BR2_PACKAGE_LIBPCAP
>> +	help
>> +	  Compile openPOWERLINK application library which contains the
>> +	  interface to a Linux user space driver, and the Linux user space
> 
>  I wouldn't call it driver, and there's no need to mention Linux so often.

So, I may reword entirely the help text without borrowing the text from
build-stack.md.

> 
>> +	  driver. It is used for implementing a multi-process solution
>> +	  where the openPOWERLINK kernel layer is running as a separate
> 
>  This 'kernel' is confusing, so just remove it.
> 
>> +	  Linux user space daemon (e.g. a PCAP based user space daemon).
> 
>  e.g. -> i.e.
> 
>  And add "No kernel-side stack is needed."
> 
>> +
>> +config BR2_PACKAGE_OPENPOWERLINK2_STACK_KERNEL_STACK_LIB
>> +	bool "build EPL stack as linux kernelspace module."
>> +	depends on BR2_LINUX_KERNEL # openpowerlink-kernel-driver
>> +	help
>> +	  Compile openPOWERLINK application library which contains the
>> +	  interface to a Linux kernel space driver. It is used together
>> +	  with a Linux kernel module openPOWERLINK driver.
> 
>  It is used together with -> This will also build and install
> 
>  Also mention the name of that module.
> 
>> +
>> +comment "openpowerlink kernel stack needs a Linux kernel to be built"
>> +	depends on !BR2_LINUX_KERNEL
>> +
>> +endchoice
>> diff --git a/package/openpowerlink2/openpowerlink2-stack/openpowerlink2-stack.mk b/package/openpowerlink2/openpowerlink2-stack/openpowerlink2-stack.mk
>> new file mode 100644
>> index 0000000..9fa2d63
>> --- /dev/null
>> +++ b/package/openpowerlink2/openpowerlink2-stack/openpowerlink2-stack.mk
>> @@ -0,0 +1,69 @@
>> +################################################################################
>> +#
>> +# openpowerlink2-stack
>> +#
>> +################################################################################
>> +
>> +OPENPOWERLINK2_STACK_VERSION = $(OPENPOWERLINK2_VERSION)
>> +
>> +OPENPOWERLINK2_STACK_LICENSE = $(OPENPOWERLINK2_LICENSE)
>> +OPENPOWERLINK2_STACK_LICENSE_FILES = $(OPENPOWERLINK2_LICENSE_FILES)
>> +
>> +# We want to use the same archive for all sub-packages.
>> +OPENPOWERLINK2_STACK_SOURCE =
>> +OPENPOWERLINK2_STACK_DEPENDENCIES = openpowerlink2
>> +
>> +define OPENPOWERLINK2_STACK_SYMLINK_TO_SRC_HOOK
>> +	ln -s $(OPENPOWERLINK2_DIR) $(OPENPOWERLINK2_STACK_DIR)/src
>> +endef
> 
>  So here you can use the global _SOURCE and _SITE, and change this into
> 
> in the global .mk file:
> 
> define OPENPOWERLINK2_EXTRACT_CMDS
> 	$(INFLATE$(suffix $($(PKG)_SOURCE))) $(DL_DIR)/$($(PKG)_SOURCE) | \
> 		$(TAR) -C $($(PKG)_DIR) $(TAR_OPTIONS) - $($(PKG)_TAR_OPTIONS)
> endef
> 
> and in openpowerlink2-stack.mk:
> 
> OPENPOWERLINK2_STACK_EXTRACT_TAR_OPTIONS = src
> OPENPOWERLINK2_STACK_EXTRACT_CMDS = $(OPENPOWERLINK2_EXTRACT_CMDS)
> 
>> +
>> +OPENPOWERLINK2_STACK_POST_EXTRACT_HOOKS += OPENPOWERLINK2_STACK_SYMLINK_TO_SRC_HOOK
>> +
>> +OPENPOWERLINK2_STACK_SUBDIR = src/stack
>> +
>> +OPENPOWERLINK2_STACK_INSTALL_STAGING = YES
>> +
>> +OPENPOWERLINK2_STACK_CONF_OPTS = -DCFG_DEBUG_LVL=$(call qstrip,$(BR2_PACKAGE_OPENPOWERLINK2_DEBUG_LEVEL))
> 
>  Add "" around this. And split the long line.
> 
>> +
>> +# All option are ON by default
> 
>  So by default it would build the monolithic stack, the daemon, and the kernel
> interface?

Yes, all libraries are build since all options are ON in
stack/cmake/options-linux.cmake.
But you can use only one of them at run time.
Also disabling monolithic stack and daemon remove the pcap dependency and
disabling the kernel interface remove the linux dependency.
Actually, I only use the kernel interface.

> 
>> +ifeq ($(BR2_PACKAGE_OPENPOWERLINK2_STACK_MONOLITHIC_USER_STACK_LIB),y)
>> +OPENPOWERLINK2_STACK_DEPENDENCIES += libpcap
>> +OPENPOWERLINK2_STACK_CONF_OPTS += \
>> +	-DCFG_COMPILE_LIB_MN=$(if $(BR2_PACKAGE_OPENPOWERLINK2_MN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_MNAPP_USERINTF=OFF \
>> +	-DCFG_COMPILE_LIB_MNAPP_KERNELINTF=OFF \
>> +	-DCFG_COMPILE_LIB_MNDRV_PCAP=OFF \
>> +	-DCFG_COMPILE_LIB_CN=$(if $(BR2_PACKAGE_OPENPOWERLINK2_CN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_CNAPP_USERINTF=OFF \
>> +	-DCFG_COMPILE_LIB_CNAPP_KERNELINTF=OFF \
>> +	-DCFG_COMPILE_LIB_CNDRV_PCAP=OFF
>> +else ifeq ($(BR2_PACKAGE_OPENPOWERLINK2_STACK_USERSPACE_DAEMON_LIB),y)
>> +OPENPOWERLINK2_STACK_DEPENDENCIES += libpcap
>> +OPENPOWERLINK2_STACK_CONF_OPTS += \
>> +	-DCFG_COMPILE_LIB_MN=OFF \
>> +	-DCFG_COMPILE_LIB_MNAPP_USERINTF=$(if $(BR2_PACKAGE_OPENPOWERLINK2_MN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_MNAPP_KERNELINTF=OFF \
>> +	-DCFG_COMPILE_LIB_MNDRV_PCAP=$(if $(BR2_PACKAGE_OPENPOWERLINK2_MN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_CN=OFF \
>> +	-DCFG_COMPILE_LIB_CNAPP_USERINTF=$(if $(BR2_PACKAGE_OPENPOWERLINK2_CN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_CNAPP_KERNELINTF=OFF \
>> +	-DCFG_COMPILE_LIB_CNDRV_PCAP=$(if $(BR2_PACKAGE_OPENPOWERLINK2_CN),ON,OFF)
>> +else ifeq ($(BR2_PACKAGE_OPENPOWERLINK2_STACK_KERNEL_STACK_LIB),y)
>> +OPENPOWERLINK2_STACK_CONF_OPTS += \
>> +	-DCFG_COMPILE_LIB_MN=OFF \
>> +	-DCFG_COMPILE_LIB_MNAPP_USERINTF=OFF \
>> +	-DCFG_COMPILE_LIB_MNAPP_KERNELINTF=$(if $(BR2_PACKAGE_OPENPOWERLINK2_MN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_MNDRV_PCAP=OFF \
>> +	-DCFG_COMPILE_LIB_CN=OFF \
>> +	-DCFG_COMPILE_LIB_CNAPP_USERINTF=OFF \
>> +	-DCFG_COMPILE_LIB_CNAPP_KERNELINTF=$(if $(BR2_PACKAGE_OPENPOWERLINK2_CN),ON,OFF) \
>> +	-DCFG_COMPILE_LIB_CNDRV_PCAP=OFF
> 
>  There is nothing here that refers to $(LINUX_DIR), so how is the module built?
There is a separate cmake project to build the module. Here we only build a
library interface to the kernel module. So there is no need to add $(LINUX_DIR)
here.

> 
>> +endif
>> +
>> +ifeq ($(BR2_STATIC_LIBS),y)
>> +OPENPOWERLINK2_STACK_CONF_OPTS += -DCFG_COMPILE_SHARED_LIBRARY=NO
>> +else
>> +OPENPOWERLINK2_STACK_CONF_OPTS += -DCFG_COMPILE_SHARED_LIBRARY=YES
>> +endif
>> +
>> +$(eval $(cmake-package))
>> diff --git a/package/openpowerlink2/openpowerlink2.mk b/package/openpowerlink2/openpowerlink2.mk
>> new file mode 100644
>> index 0000000..82c4277
>> --- /dev/null
>> +++ b/package/openpowerlink2/openpowerlink2.mk
>> @@ -0,0 +1,17 @@
>> +################################################################################
>> +#
>> +# openpowerlink2
>> +#
>> +################################################################################
>> +
>> +OPENPOWERLINK2_VERSION = V2.1.1
>> +OPENPOWERLINK2_SITE = http://git.code.sf.net/p/openpowerlink/openPOWERLINK2
>> +OPENPOWERLINK2_SITE_METHOD = git
> 
>  What's wrong with the tarball [2] ? It has the same contents. It is missing a
> leading directory component but that's easily solved with custom extract commands.

In the past, I had an issue with the zip archive:
http://git.buildroot.net/buildroot/commit/?id=8dda4eb3bd65fdf9eff0422e0e974e0a1d012015
This is why the git repository was used...
Ok I'll try to use the tar.gz archive.

> 
>  In that case, of course, add a hash file.
Sure.

Thanks for your review.

Best regards,
Romain
> 
> 
>  Regards,
>  Arnout
> 
>> +OPENPOWERLINK2_LICENSE = BSD-2c, GPLv2
>> +OPENPOWERLINK2_LICENSE_FILES = license.md
>> +
>> +# Just extract the archive
>> +
>> +$(eval $(generic-package))
>> +
>> +include package/openpowerlink2/openpowerlink2-stack/openpowerlink2-stack.mk
>>
> 
> [1] http://www.cmake.org/cmake/help/v3.2/command/target_link_libraries.html
> [2]
> http://downloads.sourceforge.net/project/openpowerlink/openPOWERLINK/V2.1.1/openPOWERLINK-V2.1.1.tar.gz
> 




More information about the buildroot mailing list