Re: [systemd-devel] [PATCH v2 1/2] systemctl: drop hardcoded chkconfig invocation

2015-05-28 Thread Lukáš Nykrýn
Lennart Poettering píše v Čt 28. 05. 2015 v 13:05 +0200:
 On Wed, 27.05.15 18:08, Martin Pitt (martin.p...@ubuntu.com) wrote:
 
  Hello again,
  
  as discussed previously this second variant of un-hardcoding chkconfig
  now uses the proposed /usr/lib/systemd/systemd-sysv-install
  abstraction.
  
  I also added a reference implementation for chkconfig which gets
  installed with --enable-chkconfig, so on Fedora this should be no
  net change.
 
 Nah, please remove this part. We should not ship that upstream. THis
 is something that Fedora's initscripts.rpm should provide eventually,
 and should be neither shipped with systemd upstream nor systemd.rpm in
 Fedora.

+1
I will patch chkconfig to handle this similarly as the install_initd.

 
  This doesn't have a manpage yet (as it's not an user-callable
  program); where should this be documented? Just adding to README?
 
 Well, you could put it on some fdo wiki page maybe and link this up
 from the source and from NEWS.
 
 Doesn't have to be too formal...
 
 Patch looks fine otherwise: just remove any trace of chkconfig.
 
 Lennart
 


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2 1/2] systemctl: drop hardcoded chkconfig invocation

2015-05-28 Thread Lennart Poettering
On Wed, 27.05.15 18:08, Martin Pitt (martin.p...@ubuntu.com) wrote:

 Hello again,
 
 as discussed previously this second variant of un-hardcoding chkconfig
 now uses the proposed /usr/lib/systemd/systemd-sysv-install
 abstraction.
 
 I also added a reference implementation for chkconfig which gets
 installed with --enable-chkconfig, so on Fedora this should be no
 net change.

Nah, please remove this part. We should not ship that upstream. THis
is something that Fedora's initscripts.rpm should provide eventually,
and should be neither shipped with systemd upstream nor systemd.rpm in
Fedora.

 This doesn't have a manpage yet (as it's not an user-callable
 program); where should this be documented? Just adding to README?

Well, you could put it on some fdo wiki page maybe and link this up
from the source and from NEWS.

Doesn't have to be too formal...

Patch looks fine otherwise: just remove any trace of chkconfig.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH v2 1/2] systemctl: drop hardcoded chkconfig invocation

2015-05-27 Thread Martin Pitt
Hello again,

as discussed previously this second variant of un-hardcoding chkconfig
now uses the proposed /usr/lib/systemd/systemd-sysv-install
abstraction.

I also added a reference implementation for chkconfig which gets
installed with --enable-chkconfig, so on Fedora this should be no net
change.

I tested this with both --enable-chkconfig and --disable-chkconfig,
and with make dist (*cough*)

This doesn't have a manpage yet (as it's not an user-callable
program); where should this be documented? Just adding to README?

Thanks,

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From 77cb2227e2fbfbad4a48837b00f3d9196366a94d Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 27 May 2015 17:04:49 +0200
Subject: [PATCH 1/2] systemctl: drop hardcoded chkconfig invocation

Introduce /usr/lib/systemd/systemd-sysv-install [--root=] action name
abstraction instead of directly calling chkconfig. This allows distributions to
call their specific tools like update-rc.d without patching systemd.

Provide a reference implementation systemd-sysv-install.chkconfig and install
that with --enable-chkconfig.
---
 Makefile.am  | 12 +
 configure.ac |  4 +--
 src/systemctl/systemctl.c| 10 +++-
 src/systemctl/systemd-sysv-install.chkconfig | 37 
 4 files changed, 54 insertions(+), 9 deletions(-)
 create mode 100755 src/systemctl/systemd-sysv-install.chkconfig

diff --git a/Makefile.am b/Makefile.am
index d6010c5..e8abef0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -624,9 +624,21 @@ nodist_systemunit_DATA += \
 systemgenerator_PROGRAMS += \
 	systemd-sysv-generator \
 	systemd-rc-local-generator
+
+if HAVE_CHKCONFIG
+chkconfig-install-hook:
+	$(install_sh_PROGRAM) $(top_srcdir)/src/systemctl/systemd-sysv-install.chkconfig $(DESTDIR)/$(rootlibexecdir)/systemd-sysv-install
+
+chkconfig-uninstall-hook:
+	rm -f $(DESTDIR)/$(rootlibexecdir)/systemd-sysv-install
+
+INSTALL_EXEC_HOOKS += chkconfig-install-hook
+UNINSTALL_EXEC_HOOKS += chkconfig-uninstall-hook
+endif
 endif
 
 EXTRA_DIST += \
+	src/systemctl/systemd-sysv-install.chkconfig \
 	units/rc-local.service.in \
 	units/halt-local.service.in
 
diff --git a/configure.ac b/configure.ac
index 48cedb5..6cccbc7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -505,9 +505,7 @@ AC_ARG_ENABLE([chkconfig], AS_HELP_STRING([--disable-chkconfig],[Disable optiona
 have_chkconfig=yes
 fi])
 
-if test x${have_chkconfig} != xno ; then
-AC_DEFINE(HAVE_CHKCONFIG, 1, [Define if CHKCONFIG is available])
-fi
+AM_CONDITIONAL(HAVE_CHKCONFIG, test x${have_chkconfig} != xno)
 
 # --
 have_selinux=no
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index f8e10a4..2a81032 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -5098,7 +5098,7 @@ static int import_environment(sd_bus *bus, char **args) {
 static int enable_sysv_units(const char *verb, char **args) {
 int r = 0;
 
-#if defined(HAVE_SYSV_COMPAT)  defined(HAVE_CHKCONFIG)
+#if defined(HAVE_SYSV_COMPAT)
 unsigned f = 0;
 _cleanup_lookup_paths_free_ LookupPaths paths = {};
 
@@ -5123,7 +5123,7 @@ static int enable_sysv_units(const char *verb, char **args) {
 _cleanup_free_ char *p = NULL, *q = NULL, *l = NULL;
 bool found_native = false, found_sysv;
 unsigned c = 1;
-const char *argv[6] = { /sbin/chkconfig, NULL, NULL, NULL, NULL };
+const char *argv[6] = { ROOTLIBEXECDIR /systemd-sysv-install, NULL, NULL, NULL, NULL };
 char **k;
 int j;
 pid_t pid;
@@ -5161,15 +5161,13 @@ static int enable_sysv_units(const char *verb, char **args) {
 if (!found_sysv)
 continue;
 
-log_info(%s is not a native service, redirecting to /sbin/chkconfig., name);
+log_info(%s is not a native service, redirecting to systemd-sysv-install, name);
 
 if (!isempty(arg_root))
 argv[c++] = q = strappend(--root=, arg_root);
 
+argv[c++] = verb;
 argv[c++] = basename(p);
-argv[c++] =
-streq(verb, enable) ? on :
-streq(verb, disable) ? off : --level=5;
 argv[c] = NULL;
 
 l = strv_join((char**)argv,  );
diff --git a/src/systemctl/systemd-sysv-install.chkconfig b/src/systemctl/systemd-sysv-install.chkconfig
new file mode 100755
index 000..5256e26
--- /dev/null
+++ b/src/systemctl/systemd-sysv-install.chkconfig
@@ -0,0 +1,37 @@
+#!/bin/sh
+set -e
+
+usage() {
+echo Usage: $0 [--root=path]