Add TCF Agent package
PATCH 1/1 Add TCF Agent package
--- package/Config.in | 1 + ...add-CMake-install-target-for-agent-binary.patch | 35 ++++++++++++++++ package/tcfagent/Config.in | 16 ++++++++ package/tcfagent/S55tcfagent | 47 ++++++++++++++++++++++ package/tcfagent/tcfagent.hash | 4 ++ package/tcfagent/tcfagent.mk | 38 +++++++++++++++++ package/tcfagent/tcfagent.service | 10 +++++ 7 files changed, 151 insertions(+) create mode 100644 package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch create mode 100644 package/tcfagent/Config.in create mode 100755 package/tcfagent/S55tcfagent create mode 100644 package/tcfagent/tcfagent.hash create mode 100644 package/tcfagent/tcfagent.mk create mode 100644 package/tcfagent/tcfagent.service
package/Config.in +1 -0
@@ -124,6 +124,7 @@ menu "Debugging, profiling and benchmark" source "package/stress-ng/Config.in" source "package/sysdig/Config.in" source "package/sysprof/Config.in" + source "package/tcfagent/Config.in" source "package/tinymembench/Config.in" source "package/trace-cmd/Config.in" source "package/trinity/Config.in"
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch new file +35 -0
@@ -0,0 +1,35 @@ +From 9140c630085833acfe565f27195a876c6656f068 Mon Sep 17 00:00:00 2001 +From: Norbert Lange <[email protected]> +Date: Mon, 30 Oct 2017 16:22:40 +0100 +Subject: [PATCH] add CMake install target for agent binary + +allows use in automated buildsystems like Buildroot +--- + agent/CMakeLists.txt | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/agent/CMakeLists.txt b/agent/CMakeLists.txt +index aef15b96..7868987a 100644 +--- a/agent/CMakeLists.txt ++++ b/agent/CMakeLists.txt +@@ -1,6 +1,7 @@ + # -*- cmake -*- + + cmake_minimum_required(VERSION 2.8) ++include(GNUInstallDirs) + + set(CMAKE_COLOR_MAKEFILE OFF) + +@@ -43,3 +44,9 @@ message(STATUS "machine:" ${TCF_MACHINE}) + + add_executable(agent tcf/main/main.c) + target_link_libraries(agent ${TCF_LIB_NAME}) ++ ++install(TARGETS agent ${TCF_LIB_NAME} ++ RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ++ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} ++ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} ++) +-- +2.14.2 +
package/tcfagent/Config.in new file +16 -0
@@ -0,0 +1,16 @@ +config BR2_PACKAGE_TCFAGENT + bool "tcfagent" + depends on BR2_TOOLCHAIN_HAS_THREADS # snmp++ + select BR2_PACKAGE_UTIL_LINUX_LIBUUID + help + Target Communication Framework Agent is an example application + using the Target Communication Framework Library. + + Target Communication Framework is universal, extensible, simple, + lightweight, vendor agnostic framework for tools and targets to + communicate for purpose of debugging, profiling, code patching and + other device software development needs. tcf-agent is a daemon, + which provides TCF services that can be used by local and remote clients. + +comment "tcfagent needs a toolchain w/ threads, C++, dynamic library" + depends on !BR2_TOOLCHAIN_HAS_THREADS
package/tcfagent/S55tcfagent new file +47 -0
47 lines not on this page · raw message
package/tcfagent/tcfagent.hash new file +4 -0
@@ -0,0 +1,4 @@ +# Locally computed: +sha256 47d34c0778aa8b9e2c26132c9bb03d643bfb8e44d03ce862d06f2f93edcb63ae org.eclipse.tcf.agent-1.3.0.tar.gz +sha256 34188fd2daeadf6574071f5004f9a7a55b3ac73efe9db203a01559ee1013b2db org.eclipse.tcf.agent-1.4_neon.tar.gz +sha256 4b6c757e2bed92a0a791d0687425d462c974abe4c79f80e27e362fdaa59107f5 org.eclipse.tcf.agent-1.5_oxygen.tar.gz
package/tcfagent/tcfagent.mk new file +38 -0
@@ -0,0 +1,38 @@ +################################################################################ +# +# TCFAGENT +# +################################################################################ + +TCFAGENT_VERSION = 1.5_oxygen +# the tar.xz link was broken the time this file got authored +TCFAGENT_SOURCE = org.eclipse.tcf.agent-$(TCFAGENT_VERSION).tar.gz +TCFAGENT_SITE = http://git.eclipse.org/c/tcf/org.eclipse.tcf.agent.git/snapshot +# see https://wiki.spdx.org/view/Legal_Team/License_List/Licenses_Under_Consideration +TCFAGENT_LICENSE = BSD-3-Clause +TCFAGENT_LICENSE_FILES = agent/edl-v10.html + +TCFAGENT_DEPENDENCIES = util-linux +TCFAGENT_SUBDIR = agent +TCFAGENT_CONF_OPTS = -DBUILD_SHARED_LIBS=Off + +define TCFAGENT_RENAME_AGENT + mv $(TARGET_DIR)/usr/bin/agent $(TARGET_DIR)/usr/sbin/tcf-agent +endef + +TCFAGENT_POST_INSTALL_TARGET_HOOKS += TCFAGENT_RENAME_AGENT + +define TCFAGENT_INSTALL_INIT_SYSTEMD + $(INSTALL) -D -m 644 package/tcfagent/tcfagent.service \ + $(TARGET_DIR)/usr/lib/systemd/system/tcfagent.service + mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants + ln -fs ../../../../usr/lib/systemd/system/tcfagent.service \ + $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/tcfagent.service +endef + +define TCFAGENT_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 package/tcfagent/S55tcfagent \ + $(TARGET_DIR)/etc/init.d/S55tcfagent +endef + +$(eval $(cmake-package))
package/tcfagent/tcfagent.service new file +10 -0
@@ -0,0 +1,10 @@ +[Unit] +Description=Target Communication Framework Agent +After=network.target + +[Service] +Type=forking +ExecStart=@SBINDIR@/tcf-agent -L- -l0 + +[Install] +WantedBy=multi-user.target
Signature · 1 line
2.15.0
PATCH 1/1 Add TCF Agent package
Hello,
Quoted 1 line · Norbert Lange
Signed-off-by Norbert Lange <[email protected]>
Thanks a lot for this contribution! This looks pretty good, there are just a few things that should be improved. See below for my comments.
First a pedantic comment: the commit title should be "tcf-agent: new package".
Quoted 8 lines
--- package/Config.in | 1 + ...add-CMake-install-target-for-agent-binary.patch | 35 ++++++++++++++++ package/tcfagent/Config.in | 16 ++++++++ package/tcfagent/S55tcfagent | 47 ++++++++++++++++++++++ package/tcfagent/tcfagent.hash | 4 ++ package/tcfagent/tcfagent.mk | 38 +++++++++++++++++ package/tcfagent/tcfagent.service | 10 +++++
Please add yourself to the DEVELOPERS file. This way, we will know who to contact when there are issues with this package :-)
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch line 6
Quoted 12 lines
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch new file +6
-0@@ -0,0 +1,35 @@ +From 9140c630085833acfe565f27195a876c6656f068 Mon Sep 17 00:00:00 2001 +From: Norbert Lange <[email protected]> +Date: Mon, 30 Oct 2017 16:22:40 +0100 +Subject: [PATCH] add CMake install target for agent binary + +allows use in automated buildsystems like Buildroot
Buildroot could technically do without it, but it would be annoying indeed. Perhaps a better phrasing would be: "It is common for CMake packages to make sure that 'make install' works properly, and that's what most users expect. More specifically, build systems such as Buildroot also expect 'make install' to do the right thing for CMake-based packages".
Also, please add your Signed-off-by line to this patch, and submit it upstream for inclusion.
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch line 19
Quoted 12 lines
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch new file +12
-0+ agent/CMakeLists.txt | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/agent/CMakeLists.txt b/agent/CMakeLists.txt +index aef15b96..7868987a 100644 +--- a/agent/CMakeLists.txt ++++ b/agent/CMakeLists.txt +@@ -1,6 +1,7 @@ + # -*- cmake -*- + + cmake_minimum_required(VERSION 2.8) ++include(GNUInstallDirs)
I'm not super familiar with CMake, but why is this needed?
package/tcfagent/Config.in line 3
Quoted 9 lines
package/tcfagent/Config.in new file +3
-0@@ -0,0 +1,16 @@ +config BR2_PACKAGE_TCFAGENT + bool "tcfagent" + depends on BR2_TOOLCHAIN_HAS_THREADS # snmp++
You're not using snmp++, so the comment here doesn't make much sense.
package/tcfagent/Config.in line 4
Quoted 1 line
package/tcfagent/Config.in new file +1
-0+ select BR2_PACKAGE_UTIL_LINUX_LIBUUID
You should also select BR2_PACKAGE_UTIL_LINUX then.
package/tcfagent/Config.in line 13
Quoted 9 lines
package/tcfagent/Config.in new file +9
-0+ help + Target Communication Framework Agent is an example application + using the Target Communication Framework Library. + + Target Communication Framework is universal, extensible, simple, + lightweight, vendor agnostic framework for tools and targets to + communicate for purpose of debugging, profiling, code patching and + other device software development needs. tcf-agent is a daemon, + which provides TCF services that can be used by local and remote clients.
Please wrap lines at 72 characters (where the first tab counts for 8 characters).
package/tcfagent/Config.in line 16
Quoted 2 lines
package/tcfagent/Config.in new file +2
-0+comment "tcfagent needs a toolchain w/ threads, C++, dynamic library" + depends on !BR2_TOOLCHAIN_HAS_THREADS
So, the message says that you need threads, C++ and dynamic library... but the dependency is only about thread. Which one is correct ? :-)
package/tcfagent/S55tcfagent line 3
Quoted 9 lines
package/tcfagent/S55tcfagent new file +3
-0@@ -0,0 +1,47 @@ +#!/bin/sh +# +# Start tcf-agent....
Not very useful comment.
package/tcfagent/S55tcfagent line 11
Quoted 8 lines
package/tcfagent/S55tcfagent new file +8
-0+# + +DAEMON_PATH=/usr/sbin/tcf-agent +DAEMON_NAME=tcf-agent +DAEMON_ARGS="-d -L- -l0" + +PIDFILE=/var/run/$DAEMON_NAME.pid +[ ! -r /etc/default/$DAEMON_NAME ] || . /etc/default/$DAEMON_NAME
Positive logic here would be better:
[ -r /etc/default/$DAEMON_NAME ] && . /etc/default/$DAEMON_NAME
package/tcfagent/S55tcfagent line 18
Quoted 6 lines
package/tcfagent/S55tcfagent new file +6
-0+start() { + printf "Starting $DAEMON_NAME: " + start-stop-daemon -S -q -o \ + -x $DAEMON_PATH -- $DAEMON_ARGS && + PPID=$(pidof $(basename $DAEMON_PATH)) && + echo $PPID > $PIDFILE
What you're doing here for the pidfile is quite weird. Why don't you let start-stop-daemon create the PID file in the first place ? Is it because tcf-agent daemonizes itself ? If so, I see two options:
- Ask tcf-agent to not daemonize itself (sometimes there's a -f or --foreground option)
- Ask tcf-agent to create the PID file if it supports that.
package/tcfagent/S55tcfagent line 25
Quoted 6 lines
package/tcfagent/S55tcfagent new file +6
-0+ [ $? = 0 ] && echo "OK" || echo "FAIL" +} + +stop() { + printf "Stopping $DAEMON_NAME: " + start-stop-daemon -K -o -s HUP -q -p $PIDFILE \
Why the HUP signal ?
package/tcfagent/S55tcfagent line 27
Quoted 2 lines
package/tcfagent/S55tcfagent new file +2
-0+ -x $DAEMON_PATH && + rm -f $PIDFILE
We generally don't remove the PID file.
Quoted 10 lines
package/tcfagent/S55tcfagent new file +10
-0+ [ $? = 0 ] && echo "OK" || echo "FAIL" +} + +case "$1" in + start) + start + ;; + stop) + stop + ;;
package/tcfagent/S55tcfagent line 47
Quoted 10 lines
package/tcfagent/S55tcfagent new file +10
-0+ restart|reload) + stop + start + ;; + *) + echo "Usage: $0 {start|stop|restart}" + exit 1 +esac + +exit $?
This last "exit" is not needed.
package/tcfagent/tcfagent.hash line 4
Quoted 10 lines
package/tcfagent/tcfagent.hash new file +4
-0@@ -0,0 +1,4 @@ +# Locally computed: +sha256 47d34c0778aa8b9e2c26132c9bb03d643bfb8e44d03ce862d06f2f93edcb63ae org.eclipse.tcf.agent-1.3.0.tar.gz +sha256 34188fd2daeadf6574071f5004f9a7a55b3ac73efe9db203a01559ee1013b2db org.eclipse.tcf.agent-1.4_neon.tar.gz +sha256 4b6c757e2bed92a0a791d0687425d462c974abe4c79f80e27e362fdaa59107f5 org.eclipse.tcf.agent-1.5_oxygen.tar.gz
Please keep the hash only for the current version being used.
However, you could add the hash for the license file.
Quoted 7 lines
package/tcfagent/tcfagent.mk new file +1
-0@@ -0,0 +1,38 @@ +################################################################################
package/tcfagent/tcfagent.mk line 11
Quoted 10 lines
package/tcfagent/tcfagent.mk new file +10
-0+# +# TCFAGENT +# +################################################################################ + +TCFAGENT_VERSION = 1.5_oxygen +# the tar.xz link was broken the time this file got authored +TCFAGENT_SOURCE = org.eclipse.tcf.agent-$(TCFAGENT_VERSION).tar.gz +TCFAGENT_SITE = http://git.eclipse.org/c/tcf/org.eclipse.tcf.agent.git/snapshot +# see https://wiki.spdx.org/view/Legal_Team/License_List/Licenses_Under_Consideration
Thanks for adding this explanation, definitely useful!
package/tcfagent/tcfagent.mk line 17
Quoted 6 lines
package/tcfagent/tcfagent.mk new file +6
-0+TCFAGENT_LICENSE = BSD-3-Clause +TCFAGENT_LICENSE_FILES = agent/edl-v10.html + +TCFAGENT_DEPENDENCIES = util-linux +TCFAGENT_SUBDIR = agent +TCFAGENT_CONF_OPTS = -DBUILD_SHARED_LIBS=Off
Why do you disable shared library build ?
package/tcfagent/tcfagent.mk line 23
Quoted 6 lines
package/tcfagent/tcfagent.mk new file +6
-0+ +define TCFAGENT_RENAME_AGENT + mv $(TARGET_DIR)/usr/bin/agent $(TARGET_DIR)/usr/sbin/tcf-agent +endef + +TCFAGENT_POST_INSTALL_TARGET_HOOKS += TCFAGENT_RENAME_AGENT
This is a bit annoying because you have already overwritten the "agent" file if there was one in $(TARGET_DIR)/usr/bin/agent. So perhaps it should be patched in the upstream CMake build logic instead ?
Quoted 21 lines
package/tcfagent/tcfagent.mk new file +15
-0+ +define TCFAGENT_INSTALL_INIT_SYSTEMD + $(INSTALL) -D -m 644 package/tcfagent/tcfagent.service \ + $(TARGET_DIR)/usr/lib/systemd/system/tcfagent.service + mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants + ln -fs ../../../../usr/lib/systemd/system/tcfagent.service \ + $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/tcfagent.service +endef + +define TCFAGENT_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 package/tcfagent/S55tcfagent \ + $(TARGET_DIR)/etc/init.d/S55tcfagent +endef + +$(eval $(cmake-package))package/tcfagent/tcfagent.service new file +0
-0
package/tcfagent/tcfagent.service line 7
Quoted 7 lines
package/tcfagent/tcfagent.service new file +7
-0@@ -0,0 +1,10 @@ +[Unit] +Description=Target Communication Framework Agent +After=network.target + +[Service] +Type=forking +ExecStart=@SBINDIR@/tcf-agent -L- -l0
What is replacing @SBINDIR@ by the correct value? Note: I'm not super familiar with systemd, so maybe it's an obvious functionality of systemd. But I don't think I've already seen that in systemd units.
Could you look at the different issues I pointed out and submit an updated version of your patch ?
Thanks a lot!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 Add TCF Agent package
Hi,
- [ ! -r /etc/default/$DAEMON_NAME ] || . /etc/default/$DAEMON_NAME
This scheme is used to be able to execute that script with 'sh -e' [ -r /etc/default/$DAEMON_NAME ] && . /etc/default/$DAEMON_NAME would fail if there is no such file (which is no error)
Please tell me if I should still use the later line.
- sysv init file
I am using HUP because TERM does not end the process, and their own init script (agent/tcf/main/tcf-agent.init) uses HUP.
Fetching the PID this way is also taken from that file, the daemon does do a fork. I tried running in foreground but that had some other issues
which I dont recall right now (will test this some more). Would be cleanest to patch in creating a pid file I think.
- systemd unit file
Sorry, I really should've tested this, copied some boilerplate code with placeholders.
- GNUInstallDirs
allows common variable overrides for installation dirs, which is often important for Windows/Mac to allow derivation from the unix-sheme
- DBUILD_SHARED_LIBS=Off
Without this, a seperate library will installed, which has no use other than the tcf-agent binary.
This library is an example implementation, its unlikely you would use it unmodified, if it gets stable at some time I would create a separate package libtcfagent
including the headers needed for development.
(Its also what you get when you use the plain Makefile)
- drop CMake....
I am considering dropping the use of CMake (there s alot deduced automatically, by changing the name you got to redo all of that manually)
and using the plain Makefile, this seems like less hassle in the end, but that means some additional stuff will be installed.
It there any common way to do this install step in an temp dir and then (re)moving files before copying this into the final target fs?
The other points are clear.
Norbert
2017-11-29 22:45 GMT+01:00 Thomas Petazzoni
<[email protected]>:
Quoted 272 lines
Hello,
Quoted 1 line · Norbert Lange
Signed-off-by Norbert Lange <[email protected]>Thanks a lot for this contribution! This looks pretty good, there are just a few things that should be improved. See below for my comments.
First a pedantic comment: the commit title should be "tcf-agent: new package".
Quoted 8 lines
--- package/Config.in | 1 + ...add-CMake-install-target-for-agent-binary.patch | 35 ++++++++++++++++ package/tcfagent/Config.in | 16 ++++++++ package/tcfagent/S55tcfagent | 47 ++++++++++++++++++++++ package/tcfagent/tcfagent.hash | 4 ++ package/tcfagent/tcfagent.mk | 38 +++++++++++++++++ package/tcfagent/tcfagent.service | 10 +++++Please add yourself to the DEVELOPERS file. This way, we will know who to contact when there are issues with this package :-)
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch line 6
Quoted 12 lines
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch new file +6
-0@@ -0,0 +1,35 @@ +From 9140c630085833acfe565f27195a876c6656f068 Mon Sep 17 00:00:00 2001 +From: Norbert Lange <[email protected]> +Date: Mon, 30 Oct 2017 16:22:40 +0100 +Subject: [PATCH] add CMake install target for agent binary + +allows use in automated buildsystems like BuildrootBuildroot could technically do without it, but it would be annoying indeed. Perhaps a better phrasing would be: "It is common for CMake packages to make sure that 'make install' works properly, and that's what most users expect. More specifically, build systems such as Buildroot also expect 'make install' to do the right thing for CMake-based packages".
Also, please add your Signed-off-by line to this patch, and submit it upstream for inclusion.
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch line 19
Quoted 12 lines
package/tcfagent/0001-add-CMake-install-target-for-agent-binary.patch new file +12
-0+ agent/CMakeLists.txt | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/agent/CMakeLists.txt b/agent/CMakeLists.txt +index aef15b96..7868987a 100644 +--- a/agent/CMakeLists.txt ++++ b/agent/CMakeLists.txt +@@ -1,6 +1,7 @@ + # -*- cmake -*- + + cmake_minimum_required(VERSION 2.8) ++include(GNUInstallDirs)I'm not super familiar with CMake, but why is this needed?
package/tcfagent/Config.in line 3
Quoted 9 lines
package/tcfagent/Config.in new file +3
-0@@ -0,0 +1,16 @@ +config BR2_PACKAGE_TCFAGENT + bool "tcfagent" + depends on BR2_TOOLCHAIN_HAS_THREADS # snmp++You're not using snmp++, so the comment here doesn't make much sense.
package/tcfagent/Config.in line 4
Quoted 1 line
package/tcfagent/Config.in new file +1
-0+ select BR2_PACKAGE_UTIL_LINUX_LIBUUIDYou should also select
BR2_PACKAGE_UTIL_LINUXthen.package/tcfagent/Config.in line 13
Quoted 9 lines
package/tcfagent/Config.in new file +9
-0+ help + Target Communication Framework Agent is an example application + using the Target Communication Framework Library. + + Target Communication Framework is universal, extensible, simple, + lightweight, vendor agnostic framework for tools and targets to + communicate for purpose of debugging, profiling, code patching and + other device software development needs. tcf-agent is a daemon, + which provides TCF services that can be used by local and remote clients.Please wrap lines at 72 characters (where the first tab counts for 8 characters).
package/tcfagent/Config.in line 16
Quoted 2 lines
package/tcfagent/Config.in new file +2
-0+comment "tcfagent needs a toolchain w/ threads, C++, dynamic library" + depends on !BR2_TOOLCHAIN_HAS_THREADSSo, the message says that you need threads, C++ and dynamic library... but the dependency is only about thread. Which one is correct ? :-)
package/tcfagent/S55tcfagent line 3
Quoted 9 lines
package/tcfagent/S55tcfagent new file +3
-0@@ -0,0 +1,47 @@ +#!/bin/sh +# +# Start tcf-agent....Not very useful comment.
package/tcfagent/S55tcfagent line 11
Quoted 8 lines
package/tcfagent/S55tcfagent new file +8
-0+# + +DAEMON_PATH=/usr/sbin/tcf-agent +DAEMON_NAME=tcf-agent +DAEMON_ARGS="-d -L- -l0" + +PIDFILE=/var/run/$DAEMON_NAME.pid +[ ! -r /etc/default/$DAEMON_NAME ] || . /etc/default/$DAEMON_NAMEPositive logic here would be better:
[ -r
/etc/default/$DAEMON_NAME] && ./etc/default/$DAEMON_NAMEpackage/tcfagent/S55tcfagent line 18
Quoted 6 lines
package/tcfagent/S55tcfagent new file +6
-0+start() { + printf "Starting $DAEMON_NAME: " + start-stop-daemon -S -q -o \ + -x $DAEMON_PATH -- $DAEMON_ARGS && + PPID=$(pidof $(basename $DAEMON_PATH)) && + echo $PPID > $PIDFILEWhat you're doing here for the pidfile is quite weird. Why don't you let start-stop-daemon create the PID file in the first place ? Is it because tcf-agent daemonizes itself ? If so, I see two options:
- Ask tcf-agent to not daemonize itself (sometimes there's a -f or --foreground option)
- Ask tcf-agent to create the PID file if it supports that.
package/tcfagent/S55tcfagent line 25
Quoted 6 lines
package/tcfagent/S55tcfagent new file +6
-0+ [ $? = 0 ] && echo "OK" || echo "FAIL" +} + +stop() { + printf "Stopping $DAEMON_NAME: " + start-stop-daemon -K -o -s HUP -q -p $PIDFILE \Why the HUP signal ?
package/tcfagent/S55tcfagent line 27
Quoted 2 lines
package/tcfagent/S55tcfagent new file +2
-0+ -x $DAEMON_PATH && + rm -f $PIDFILEWe generally don't remove the PID file.
Quoted 10 lines
package/tcfagent/S55tcfagent new file +10
-0+ [ $? = 0 ] && echo "OK" || echo "FAIL" +} + +case "$1" in + start) + start + ;; + stop) + stop + ;;package/tcfagent/S55tcfagent line 47
Quoted 10 lines
package/tcfagent/S55tcfagent new file +10
-0+ restart|reload) + stop + start + ;; + *) + echo "Usage: $0 {start|stop|restart}" + exit 1 +esac + +exit $?This last "exit" is not needed.
package/tcfagent/tcfagent.hash line 4
Quoted 10 lines
package/tcfagent/tcfagent.hash new file +4
-0@@ -0,0 +1,4 @@ +# Locally computed: +sha256 47d34c0778aa8b9e2c26132c9bb03d643bfb8e44d03ce862d06f2f93edcb63ae org.eclipse.tcf.agent-1.3.0.tar.gz +sha256 34188fd2daeadf6574071f5004f9a7a55b3ac73efe9db203a01559ee1013b2db org.eclipse.tcf.agent-1.4_neon.tar.gz +sha256 4b6c757e2bed92a0a791d0687425d462c974abe4c79f80e27e362fdaa59107f5 org.eclipse.tcf.agent-1.5_oxygen.tar.gzPlease keep the hash only for the current version being used.
However, you could add the hash for the license file.
Quoted 7 lines
package/tcfagent/tcfagent.mk new file +1
-0@@ -0,0 +1,38 @@ +################################################################################package/tcfagent/tcfagent.mk line 11
Quoted 10 lines
package/tcfagent/tcfagent.mk new file +10
-0+# +# TCFAGENT +# +################################################################################ + +TCFAGENT_VERSION = 1.5_oxygen +# the tar.xz link was broken the time this file got authored +TCFAGENT_SOURCE = org.eclipse.tcf.agent-$(TCFAGENT_VERSION).tar.gz +TCFAGENT_SITE = http://git.eclipse.org/c/tcf/org.eclipse.tcf.agent.git/snapshot +# see https://wiki.spdx.org/view/Legal_Team/License_List/Licenses_Under_ConsiderationThanks for adding this explanation, definitely useful!
package/tcfagent/tcfagent.mk line 17
Quoted 6 lines
package/tcfagent/tcfagent.mk new file +6
-0+TCFAGENT_LICENSE = BSD-3-Clause +TCFAGENT_LICENSE_FILES = agent/edl-v10.html + +TCFAGENT_DEPENDENCIES = util-linux +TCFAGENT_SUBDIR = agent +TCFAGENT_CONF_OPTS = -DBUILD_SHARED_LIBS=OffWhy do you disable shared library build ?
package/tcfagent/tcfagent.mk line 23
Quoted 6 lines
package/tcfagent/tcfagent.mk new file +6
-0+ +define TCFAGENT_RENAME_AGENT + mv $(TARGET_DIR)/usr/bin/agent $(TARGET_DIR)/usr/sbin/tcf-agent +endef + +TCFAGENT_POST_INSTALL_TARGET_HOOKS += TCFAGENT_RENAME_AGENTThis is a bit annoying because you have already overwritten the "agent" file if there was one in $(TARGET_DIR)/usr/bin/agent. So perhaps it should be patched in the upstream CMake build logic instead ?
Quoted 21 lines
package/tcfagent/tcfagent.mk new file +15
-0+ +define TCFAGENT_INSTALL_INIT_SYSTEMD + $(INSTALL) -D -m 644 package/tcfagent/tcfagent.service \ + $(TARGET_DIR)/usr/lib/systemd/system/tcfagent.service + mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants + ln -fs ../../../../usr/lib/systemd/system/tcfagent.service \ + $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/tcfagent.service +endef + +define TCFAGENT_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 package/tcfagent/S55tcfagent \ + $(TARGET_DIR)/etc/init.d/S55tcfagent +endef + +$(eval $(cmake-package))package/tcfagent/tcfagent.service new file +0
-0package/tcfagent/tcfagent.service line 7
Quoted 7 lines
package/tcfagent/tcfagent.service new file +7
-0@@ -0,0 +1,10 @@ +[Unit] +Description=Target Communication Framework Agent +After=network.target + +[Service] +Type=forking +ExecStart=@SBINDIR@/tcf-agent -L- -l0What is replacing @SBINDIR@ by the correct value? Note: I'm not super familiar with systemd, so maybe it's an obvious functionality of systemd. But I don't think I've already seen that in systemd units.
Could you look at the different issues I pointed out and submit an updated version of your patch ?
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
PATCH 1/1 Add TCF Agent package
Hello,
It would be nice if you could avoid top-posting. Replying below each part of the e-mail you're answering is much more readable, and is the common practice on the mailing lists of open-source projects.
Quoted 8 lines · Norbert Lange
Hi,
- [ ! -r
/etc/default/$DAEMON_NAME] || ./etc/default/$DAEMON_NAME
This scheme is used to be able to execute that script with 'sh -e' [ -r/etc/default/$DAEMON_NAME] && ./etc/default/$DAEMON_NAMEwould fail if there is no such file (which is no error)Please tell me if I should still use the later line.
Interesting reason. Please keep your original proposal then. The ejabberd package is already using this construct. However, all other packages are using the && construct.
Unless Yann has a better suggestion for this ?
Quoted 3 lines
- sysv init file
I am using HUP because TERM does not end the process, and their own init script (agent/tcf/main/tcf-agent.init) uses HUP.
OK. A comment in the shell script about why you're sending the HUP signal would be nice.
Quoted 4 lines
Fetching the PID this way is also taken from that file, the daemon does do a fork. I tried running in foreground but that had some other issues which I dont recall right now (will test this some more). Would be cleanest to patch in creating a pid file I think.
Yes, if you could test some more running the daemon in the background, it would be nice.
Quoted 3 lines
- systemd unit file
Sorry, I really should've tested this, copied some boilerplate code with placeholders.
No problem :)
Quoted 4 lines
- GNUInstallDirs
allows common variable overrides for installation dirs, which is often important for Windows/Mac to allow derivation from the unix-sheme
Quoted 8 lines
- DBUILD_SHARED_LIBS=Off
Without this, a seperate library will installed, which has no use other than the tcf-agent binary.
This library is an example implementation, its unlikely you would use it unmodified, if it gets stable at some time I would create a separate package libtcfagent
including the headers needed for development.
(Its also what you get when you use the plainMakefile)
Please add a comment in the .mk file about this, like:
# tcf-agent builds a shared library, but it really isn't useful for # anything but tcf-agent itself, so don't bother building it as a # shared library
Quoted 6 lines
- drop CMake....
I am considering dropping the use of CMake (there s alot deduced automatically, by changing the name you got to redo all of that manually)
and using the plainMakefile, this seems like less hassle in the end, but that means some additional stuff will be installed.
Hum, I don't think I suggested dropping CMake. I'm all in favor of using standardized build systems such as CMake. What about just patching the CMakeLists.txt to build a binary named tcf-agent instead of agent? With a proper explanation, this should be acceptable by the upstream tcf-agent developers.
Quoted 2 lines
It there any common way to do this install step in an temp dir and then (re)moving files before copying this into the final target fs?
Nope, we don't have such a possibility currently I'm afraid.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 Add TCF Agent package
2017-11-30 13:50 GMT+01:00 Thomas Petazzoni
<[email protected]>:
Quoted 5 lines
Hello,
It would be nice if you could avoid top-posting. Replying below each part of the e-mail you're answering is much more readable, and is the common practice on the mailing lists of open-source projects.
Ok noted.
Quoted 7 lines
Quoted 6 lines
- drop CMake....
I am considering dropping the use of CMake (there s alot deduced automatically, by changing the name you got to redo all of that manually)
and using the plainMakefile, this seems like less hassle in the end, but that means some additional stuff will be installed.
Quoted 5 lines
Hum, I don't think I suggested dropping CMake. I'm all in favor of using standardized build systems such as CMake. What about just patching the CMakeLists.txt to build a binary named tcf-agent instead of agent? With a proper explanation, this should be acceptable by the upstream tcf-agent developers.
You did not suggest it, I am thinking about the best way to build it. The package comes both with CMake and GNUMake files, both with their issues, GNUMake is more complete as it installs headers and init script.
I cant easily rename the executable target, as there is already a target named "tcf-agent" for the library. I dont want to work around that limitation, as in my experience, as soon as you start circumventing the CMake automechanics you never stop.
"tcf-agent" is the commonly referred executable name btw.
Quoted 5 lines
Quoted 3 lines
It there any common way to do this install step in an temp dir and then (re)moving files before copying this into the final target fs?
Nope, we don't have such a possibility currently I'm afraid.
Ok, gonna need some tinkering time, especially after looking more closely into the (C)Make files. I found some goodies like
if(NOT TCF_MACHINE)
set(TCF_MACHINE "x86_64")
endif()
which means currently other architectures will have some funny disassembler and debugging-services...
Possible values are "a64 arm i386 i686 powerpc ppc64 x86_64", gonna map this from the Buildroot variables somehow.
Also, if the package *optionally* links to eg. libssl if available, then I would add this only inTCFAGENT_DEPENDENCIES += openssl ?
Norbert
PATCH 1/1 Add TCF Agent package
In reply to Thomas PetazzoniNorbert, Thomas, All,
On 2017-11-30 13:50 +0100, Thomas Petazzoni spake thusly:
Quoted 6 lines · Norbert Lange
Quoted 5 lines
- [ ! -r
/etc/default/$DAEMON_NAME] || ./etc/default/$DAEMON_NAME
This scheme is used to be able to execute that script with 'sh -e' [ -r/etc/default/$DAEMON_NAME] && ./etc/default/$DAEMON_NAMEwould fail if there is no such file (which is no error)
Quoted 7 lines
Quoted 1 line
Please tell me if I should still use the later line.
Interesting reason. Please keep your original proposal then. The ejabberd package is already using this construct. However, all other packages are using the && construct.
Unless Yann has a better suggestion for this ?
I think we should write down those rules about startup scripts and their configuration files.
But back on topic for Norbert's use-case. I usually do like he did, but I find it hackish nonetheless. I really prefer positive logic anyway, like so:
if [ -r /etc/default/foo.config ]; then
. /etc/default/foo.config
fi
Yes, that's three lines instead of one, but who cares, really?
However, Norbert's excuse only stands for script that are 'set -e' (i.e. they exit as soon as a command exits with a non-zero return code.
But this is not the case of the startup script. So if we would really want to make it a single line, then positive logic would still to be preferred:
[ -r /etc/default/foo.config ] && . /etc/default/foo.config
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 Add TCF Agent package
2017-11-30 22:24 GMT+01:00 Yann E. MORIN <[email protected]>:
Quoted 11 lines
Norbert, Thomas, All,
On 2017-11-30 13:50 +0100, Thomas Petazzoni spake thusly:
Quoted 8 lines · Norbert Lange
Quoted 6 lines
- [ ! -r
/etc/default/$DAEMON_NAME] || ./etc/default/$DAEMON_NAME
This scheme is used to be able to execute that script with 'sh -e' [ -r/etc/default/$DAEMON_NAME] && ./etc/default/$DAEMON_NAMEwould fail if there is no such file (which is no error)Please tell me if I should still use the later line.
Quoted 8 lines
Quoted 5 lines
Interesting reason. Please keep your original proposal then. The ejabberd package is already using this construct. However, all other packages are using the && construct.
Unless Yann has a better suggestion for this ?
I think we should write down those rules about startup scripts and their configuration files.
Yes, maybe some helper scripts like https://packages.debian.org/sid/lsb-base
would be a good resource for templates and cut down most of the repetition aswell.
eg.
#!/bin/sh
DAEMON_PATH=/usr/sbin/tcf-agent
DAEMON_NAME=tcf-agent
DAEMON_ARGS="-d -L- -l0"
. /lib/lsb/init-functions
# compute default name for pidfile DAEMON_USE_PIDFILE=default
# mostly the same for many, if not copy then and adopt . $DEFAULT_START_STOP
Quoted 5 lines
But back on topic for Norbert's use-case. I usually do like he did, but I find it hackish nonetheless. I really prefer positive logic anyway, like so:
Quoted 8 lines
if [ -r
/etc/default/foo.config]; then
./etc/default/foo.config
fiYes, that's three lines instead of one, but who cares, really?
However, Norbert's excuse only stands for script that are 'set -e' (i.e. they exit as soon as a command exits with a non-zero return code.
There is no excuse (I have no problem using your solution), but old experiences, preferences and habits like running scripts with set -e and the "shortcut logic" required - try until one of the statements is true.
The if will disable exit-on-error, so that's different to both lines.
Btw. I was under the impression that buildroot tries to be as small as possible, and therefore uses its own init scripts instead the ones from upstream packages.
Quoted 6 lines
But this is not the case of the startup script. So if we would really want to make it a single line, then positive logic would still to be preferred:
[ -r /etc/default/foo.config ] && . /etc/default/foo.config
Ok.
Norbert
Quoted 11 lines
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
PATCH 1/1 Add TCF Agent package
Norbet, All,
On 2017-12-01 09:40 +0100, Norbert Lange spake thusly:
Quoted 14 lines
2017-11-30 22:24 GMT+01:00 Yann E. MORIN <[email protected]>:
Quoted 13 lines
On 2017-11-30 13:50 +0100, Thomas Petazzoni spake thusly:
Quoted 12 lines · Norbert Lange
Quoted 6 lines
- [ ! -r
/etc/default/$DAEMON_NAME] || ./etc/default/$DAEMON_NAME
This scheme is used to be able to execute that script with 'sh -e' [ -r/etc/default/$DAEMON_NAME] && ./etc/default/$DAEMON_NAMEwould fail if there is no such file (which is no error)Please tell me if I should still use the later line.
Interesting reason. Please keep your original proposal then. The ejabberd package is already using this construct. However, all other packages are using the && construct.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Unless Yann has a better suggestion for this ?
I think we should write down those rules about startup scripts and their configuration files.
Yes, maybe some helper scripts like https://packages.debian.org/sid/lsb-base
would be a good resource for templates and cut down most of the repetition aswell.
I think a simple template in the manual would be enough.
Quoted 8 lines
Quoted 8 lines
But back on topic for Norbert's use-case. I usually do like he did, but I find it hackish nonetheless. I really prefer positive logic anyway, like so:
if [ -r
/etc/default/foo.config]; then
./etc/default/foo.config
fi
Quoted 6 lines
Quoted 4 lines
Yes, that's three lines instead of one, but who cares, really?
However, Norbert's excuse only stands for script that are 'set -e' (i.e. they exit as soon as a command exits with a non-zero return code.
There is no excuse (I have no problem using your solution),
Sorry, I did not meant 'excuse' as something that was an bad reason, but in a more neutral sense...
Quoted 7 lines
but old experiences, preferences and habits like running scripts with set -e and the "shortcut logic" required - try until one of the statements is true.
The if will disable exit-on-error, so that's different to both lines.Btw. I was under the impression that buildroot tries to be as small as possible, and therefore uses its own init scripts instead the ones from upstream packages.
There is no causality; only correlation. We usually provide our own startup scripts not because of size, but because the upstream ones make strong assumptions about running on a desktop-like or server-like system, and do not work with Busybox init for example.
And startup script are relatively small, when compared to the rest of the system. Also, they comress very easily. So it is usually not worth optimising them for size that much.
(and besides, using three lines instead of one is only in fact using only just a few more bytes, which is what counts in the end).
Regards,
Yann E. MORIN.
Quoted 19 lines
Quoted 5 lines
But this is not the case of the startup script. So if we would really want to make it a single line, then positive logic would still to be preferred:
[ -r /etc/default/foo.config ] && . /etc/default/foo.configOk.
NorbertQuoted 11 lines
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'