[__single_binary_service] Adapt bug fixes proposed by pedro

there are several typos, some style issues and now there is at most one service
restart in all cases.

Submitted by:   pedro <git2021@cas.cat>

gencode-remote

@@ -0,0 +1,21 @@
+#!/bin/sh -e
+
+STATE="$(cat "${__object}/parameter/state")"
+if [ "${STATE}" != "present" ]; then
+	exit
+fi
+
+ETC_DIR="/etc"
+SERVICE_NAME="${__object_id}"
+CONFIG_FILE_DEST="${ETC_DIR}/${SERVICE_NAME}.conf"
+
+BIN_DIR="/usr/local/bin"
+VERSION_FILE="${BIN_DIR}/.${SERVICE_NAME}.cdist.version"
+
+# We only restart here if there was a config change
+# but there was not a version change
+if grep -qE "^__file${CONFIG_FILE_DEST}" "${__messages_in}" && \
+	grep -qvE "^__file${VERSION_FILE}" "${__messages_in}"; then
+	echo "service ${SERVICE_NAME} restart"
+fi
+

man.rst

@@ -86,7 +86,9 @@ binary
     Otherwise, the contents of `--url` will be placed under this binary name.
 
 service-args
-    Any extra arguments to pass along with `--service-exec`.
+    Any extra arguments to pass along with `--service-exec`. Beware that any
+    service-args having the format `--config=/etc/foo.cfg` should be
+    represented in the following way `--service-exec='--config=/etc/foo.cfg'`
 
 service-exec
     The executable to use for this service.

manifest

@@ -1,5 +1,20 @@
 #!/bin/sh -e
 
+OS="$(cat "${__global}/explorer/os")"
+
+case "${OS}" in
+  debian)
+    SUPER_USER_GROUP=root
+  ;;
+  *bsd)
+    SUPER_USER_GROUP=wheel
+  ;;
+  *)
+    echo "Your OS '${OS}' is currently not supported." >&2
+    exit 1
+  ;;
+esac
+
 BIN_DIR="/usr/local/bin"
 ETC_DIR="/etc"
 
@@ -26,7 +41,7 @@ fi
 EXTRA_BINARIES="$(cat "${__object}/parameter/extra-binary" 2>/dev/null || true)"
 # This only makes sense for file archives
 if [ -n "${EXTRA_BINARIES}" ] && [ -f "${__object}/parameter/unpack" ]; then
-	cat >> /dev/stderr <<-EOF
+	cat >&2 <<-EOF
 	You cannot specify extra binaries without the --unpack argument.
 	Make sure that the --url argument points to a file archive.
 EOF
@@ -36,7 +51,8 @@ SERVICE_EXEC="$(cat "${__object}/parameter/service-exec" 2>/dev/null || true)"
 if [ -z "${SERVICE_EXEC}" ]; then
 	SERVICE_EXEC="${BIN_DIR}/${BINARY}"
 fi
-SERVICE_EXEC="${SERVICE_EXEC} $(cat "${__object}/parameter/service-args")"
+SERVICE_ARGS="$(cat "${__object}/parameter/service-args")"
+SERVICE_EXEC="${SERVICE_EXEC} ${SERVICE_ARGS}"
 
 SERVICE_DESCRIPTION="$(cat "${__object}/parameter/service-description" \
 	2>/dev/null || true)"
@@ -77,17 +93,19 @@ if [ -n "${CONFIG_FILE_SOURCE}" ] && [ "${STATE}" = "present" ]; then
 		--group "${GROUP}" \
 		--mode "0440" \
 		--source "${CONFIG_FILE_SOURCE}"
-	service_required="${service_required} __file${CONFIG_FILE_DEST}"
+	service_require="${service_require} __file${CONFIG_FILE_DEST}"
 fi
 
 
 
+INIT="$(cat "${__global}/explorer/init")"
 # TODO: Support non-systemd
-__systemd_unit "${SERVICE_NAME}.service" \
+case "${INIT}" in
-	--source "-" \
+	systemd)
-	--state "${STATE}" \
+		__systemd_unit "${SERVICE_NAME}.service" \
-	--restart \
+			--source "-" \
-	--enablement-state "enabled" <<EOF
+			--state "${STATE}" \
+			--enablement-state "enabled" <<EOF
 [Unit]
 Description=${SERVICE_DESCRIPTION}
 After=network.target
@@ -103,18 +121,18 @@ Restart=always
 [Install]
 WantedBy=multi-user.target
 EOF
-service_require="${service_require} __systemd_unit/${SERVICE_NAME}.service"
+	service_require="${service_require} __systemd_unit/${SERVICE_NAME}.service"
+
+  ;;
+  *)
+    echo "Init system ${INIT}' is currently not supported." >&2
+    exit 1
+  ;;
+esac
 
 # Proceed after user and service description have been prepared
 export require="${require} ${service_require}"
 
-# Perform a service restart if config has changed
-if [ "${STATE}" = "present" ]; then
-	__check_messages "${SERVICE_NAME}_config" \
-		--pattern "^__file${CONFIG_FILE_DEST}" \
-		--execute "service ${SERVICE_NAME} restart"
-fi
-
 VERSION_FILE="${BIN_DIR}/.${SERVICE_NAME}.cdist.version"
 IS_VERSION="$(cat "${__object}/explorer/explorer-version")"
 
@@ -130,8 +148,7 @@ if [ "${SHOULD_VERSION}" != "${IS_VERSION}" ] && \
 service ${SERVICE_NAME} stop || true
 for bin_file in ${BINARY} ${EXTRA_BINARIES}; do
 	bin_path="${TMP_PATH}/\${bin_file}"
-	# TODO: on the BSDs, the super user group is wheel
+	chown root:${SUPER_USER_GROUP} "\${bin_path}"
-	chown root:root "\${bin_path}"
 	chmod 0555 "\${bin_path}"
 	cp -af "\${bin_path}" "${BIN_DIR}/\${bin_file}"
 done
@@ -154,39 +171,34 @@ EOF
 		require="__download${TMP_PATH}.tar.gz" \
 			__unpack "${TMP_PATH}.tar.gz" \
 			${UNPACK_ARGS} \
-			--destination "${TMP_PATH}" \
+			--destination "${TMP_PATH}"
-			--onchange "$(cat <<EOF
-${perform_service_upgrade}
-EOF
-)"
 		version_bump_require="__unpack${TMP_PATH}.tar.gz"
 	else
 		# Create temp directory
 		__directory "${TMP_PATH}"
 		# Download binary directoy to the temp directory with the
 		# specified binary name
-		# And also perform service upgrade
 		require="__directory${TMP_PATH}" __download \
 			"${TMP_PATH}/${BINARY}" \
 			--url "${DOWNLOAD_URL}" \
 			--download remote \
-			--sum "${CHECKSUM}" \
+			--sum "${CHECKSUM}"
-			--onchange "${perform_service_upgrade}"
 		version_bump_require="__download${TMP_PATH}/${BINARY}"
 	fi
 
 	# Perform update of cdist-managed version file
-	# only after binaries have been upgraded
+	# And also perform service upgrade
 	printf "%s" "${SHOULD_VERSION}" | \
 		require="${version_bump_require}" __file \
 			"${VERSION_FILE}" \
+			--onchange "${perform_service_upgrade}" \
 			--source "-"
 fi
 
 if [ "${STATE}" = "absent" ]; then
 	# Perform cleanup of generated files
 	for bin_file in ${BINARY} ${EXTRA_BINARIES}; do
-		__file "${BINARY_PREFIX}/${bin_file}" --state "absent"
+		__file "${BIN_DIR}/${bin_file}" --state "absent"
 	done
 	__file "${VERSION_FILE}" --state "absent"
 	__file "${CONFIG_FILE_DEST}" --state "absent"