After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737139 - Networkmanager-0.9.10.0: no way to set rundir to /run
Networkmanager-0.9.10.0: no way to set rundir to /run
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Pavel Simerda
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-09-22 19:10 UTC by Pacho Ramos
Modified: 2015-05-28 10:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
incomplete patch (4.40 KB, patch)
2014-12-29 22:58 UTC, Pavel Simerda
none Details | Review
WIP patch (4.79 KB, patch)
2014-12-30 08:49 UTC, Pavel Simerda
none Details | Review
build: support runstatedir configure option (3.27 KB, patch)
2015-05-21 15:15 UTC, Thomas Haller
none Details | Review

Description Pacho Ramos 2014-09-22 19:10:07 UTC
We see no way for setting this paths (due we wanting to deprecate /var/run in favor of /run)
  nmstatedir: /var/lib/NetworkManager
  nmrundir: /run/NetworkManager

For achieving that we need to sed configure because, otherwise, /var from localstatedir setting is appended due configure.ac having:
# NetworkManager paths
AC_SUBST(nmbinary, "$sbindir/$PACKAGE", [NetworkManager binary executable])
AC_SUBST(nmconfdir, "$sysconfdir/$PACKAGE", [NetworkManager configuration directory])
AC_SUBST(nmdatadir, "$datadir/$PACKAGE", [NetworkManager shared data directory])
AC_SUBST(nmstatedir, "$localstatedir/lib/$PACKAGE", [NetworkManager persistent state directory])
AC_SUBST(nmrundir, "$localstatedir/run/$PACKAGE", [NetworkManager runtime state directory])


If we set localstatedir to "/" nmstatedir will be in /lib, if we set it to /var, rundir will be /var/run instead of /run

Thanks
Comment 1 Pavel Simerda 2014-12-26 23:46:36 UTC
What's the most common way to handle it in other autotools based packages?
Comment 2 Pacho Ramos 2014-12-28 19:05:39 UTC
I am not sure if I am understanding this correctly:
https://www.gnu.org/prep/standards/html_node/Directory-Variables.html

But seems that relying on "runstatedir" would allow us to be able to set that stuff to be put in /run
Comment 3 Pavel Simerda 2014-12-29 22:58:27 UTC
Created attachment 293456 [details] [review]
incomplete patch

This is a work in progress patch tested with autoconf 2.69. It doesn't yet support setting the runstatedir. I'll have to experiment a bit.
Comment 4 Pavel Simerda 2014-12-30 08:49:35 UTC
Created attachment 293467 [details] [review]
WIP patch

This patch is ready for first rounds of review. It still has potential issues, as it uses /var/run as the default value for $runstatedir but attempts to use the variable also for things that have been assumed to be part of /run for some time now. This needs a bit of consideration.

My first thought is that $runstatedir should be supplied by the distribution and should be /run on distributions that opted to use /run. Therefore it should be safe to use it for everything as in the patch and ./configure should just let you override specific paths for specific tools like ConsoleKit or systemd.
Comment 5 Thomas Haller 2015-01-09 18:54:07 UTC
I think you should not touch the files under src/dhcp-manager/systemd-dhcp/ unless you have a very good reason to do so. We aim them to stay as similar as possible as their systemd origin.
Comment 6 Thomas Haller 2015-05-21 15:15:43 UTC
Created attachment 303766 [details] [review]
build: support runstatedir configure option

https://bugzilla.gnome.org/show_bug.cgi?id=737139

[thaller@redhat.com: modified original patch]
Comment 7 Thomas Haller 2015-05-21 15:17:32 UTC
I took patch from attachment 293467 [details] [review] and modified it.

Especially, I dropped the following changes:



 if test "$use_consolekit" = "yes"; then
     AC_DEFINE([SESSION_TRACKING_CONSOLEKIT], 1, [Define to 1 if ConsoleKit is available])
-    AC_DEFINE([CKDB_PATH], "/var/run/ConsoleKit/database", [Path to ConsoleKit database])
+    AC_DEFINE([CKDB_PATH], "$runstatedir/ConsoleKit/database", [Path to ConsoleKit database])
     session_tracking="$session_tracking, consolekit"
 fi


 bool plymouth_running(void) {
-        return access("/run/plymouth/pid", F_OK) >= 0;
+        return access(RUNSTATEDIR "/plymouth/pid", F_OK) >= 0;
 }

@@ -6362,7 +6362,7 @@ int container_get_leader(const char *machine, pid_t *pid) {
         assert(machine);
         assert(pid);
 
-        p = strjoina("/run/systemd/machines/", machine);
+        p = strjoina(RUNSTATEDIR "/run/systemd/machines/", machine);
         r = parse_env_file(p, NEWLINE, "LEADER", &s, "CLASS", &class, NULL);


 static inline bool logind_running(void) {
-        return access("/run/systemd/seats/", F_OK) >= 0;
+        return access(RUNSTATEDIR "/systemd/seats/", F_OK) >= 0;
 }
Comment 8 Thomas Haller 2015-05-21 15:17:56 UTC
Comment on attachment 293467 [details] [review]
WIP patch

>From d1df19dc0016ccfe94a4e506cc15739ab26cf549 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Pavel=20=C5=A0imerda?= <psimerda@redhat.com>
>Date: Mon, 29 Dec 2014 23:56:46 +0100
>Subject: [PATCH] WIP: support runstatedir
>
>---
> configure.ac                                    | 7 +++++--
> src/Makefile.am                                 | 1 +
> src/dhcp-manager/nm-dhcp-dhclient.c             | 2 +-
> src/dhcp-manager/systemd-dhcp/src/shared/util.c | 4 ++--
> src/dhcp-manager/systemd-dhcp/src/shared/util.h | 2 +-
> src/dnsmasq-manager/nm-dnsmasq-manager.c        | 2 +-
> 6 files changed, 11 insertions(+), 7 deletions(-)
>
>diff --git a/configure.ac b/configure.ac
>index 6739b23..6fee243 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -83,12 +83,15 @@ dnl Make sha1.c happy on big endian systems
> dnl
> AC_C_BIGENDIAN
> 
>+# Add runstatedir if not specified manually in autoconf < 2.70
>+AS_IF([test -z "$runstatedir"], runstatedir="$localstatedir/run")
>+
> # NetworkManager paths
> AC_SUBST(nmbinary, "$sbindir/$PACKAGE", [NetworkManager binary executable])
> AC_SUBST(nmconfdir, "$sysconfdir/$PACKAGE", [NetworkManager configuration directory])
> AC_SUBST(nmdatadir, "$datadir/$PACKAGE", [NetworkManager shared data directory])
> AC_SUBST(nmstatedir, "$localstatedir/lib/$PACKAGE", [NetworkManager persistent state directory])
>-AC_SUBST(nmrundir, "$localstatedir/run/$PACKAGE", [NetworkManager runtime state directory])
>+AC_SUBST(nmrundir, "$runstatedir/$PACKAGE", [NetworkManager runtime state directory])
> 
> # Alternative configuration plugins
> AC_ARG_ENABLE(config-plugin-ibft, AS_HELP_STRING([--enable-config-plugin-ibft], [enable ibft configuration plugin]))
>@@ -353,7 +356,7 @@ if test "$with_session_tracking" = "systemd"; then
> 	AC_SUBST(SYSTEMD_LOGIN_LIBS)
> fi
> if test "$with_session_tracking" = "consolekit"; then
>-	AC_SUBST(CKDB_PATH, /var/run/ConsoleKit/database)
>+	AC_SUBST(CKDB_PATH, $runstatedir/ConsoleKit/database)
> fi
> AC_MSG_RESULT($with_session_tracking)
> 
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 808e59e..506d207 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -417,6 +417,7 @@ AM_CPPFLAGS += \
> 	-DDATADIR=\"$(datadir)\" \
> 	-DLIBEXECDIR=\"$(libexecdir)\" \
> 	-DLOCALSTATEDIR=\"$(localstatedir)\" \
>+	-DRUNSTATEDIR=\"$(runstatedir)\" \
> 	-DSBINDIR=\"$(sbindir)\" \
> 	-DSYSCONFDIR=\"$(sysconfdir)\" \
> 	-DRUNDIR=\"$(rundir)\" \
>diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c b/src/dhcp-manager/nm-dhcp-dhclient.c
>index 504e8d5..07a470c 100644
>--- a/src/dhcp-manager/nm-dhcp-dhclient.c
>+++ b/src/dhcp-manager/nm-dhcp-dhclient.c
>@@ -346,7 +346,7 @@ dhclient_start (NMDhcpClient *client,
> 		return FALSE;
> 	}
> 
>-	pid_file = g_strdup_printf (LOCALSTATEDIR "/run/dhclient%s-%s.pid",
>+	pid_file = g_strdup_printf (RUNSTATEDIR "/dhclient%s-%s.pid",
> 		                        ipv6 ? "6" : "",
> 		                        iface);
> 
>diff --git a/src/dhcp-manager/systemd-dhcp/src/shared/util.c b/src/dhcp-manager/systemd-dhcp/src/shared/util.c
>index 05d4eb7..b8b4c68 100644
>--- a/src/dhcp-manager/systemd-dhcp/src/shared/util.c
>+++ b/src/dhcp-manager/systemd-dhcp/src/shared/util.c
>@@ -4160,7 +4160,7 @@ bool nulstr_contains(const char*nulstr, const char *needle) {
> }
> 
> bool plymouth_running(void) {
>-        return access("/run/plymouth/pid", F_OK) >= 0;
>+        return access(RUNSTATEDIR "/plymouth/pid", F_OK) >= 0;
> }
> 
> char* strshorten(char *s, size_t l) {
>@@ -6336,7 +6336,7 @@ int container_get_leader(const char *machine, pid_t *pid) {
>         assert(machine);
>         assert(pid);
> 
>-        p = strappenda("/run/systemd/machines/", machine);
>+        p = strappenda(RUNSTATEDIR "/systemd/machines/", machine);
>         r = parse_env_file(p, NEWLINE, "LEADER", &s, "CLASS", &class, NULL);
>         if (r == -ENOENT)
>                 return -EHOSTDOWN;
>diff --git a/src/dhcp-manager/systemd-dhcp/src/shared/util.h b/src/dhcp-manager/systemd-dhcp/src/shared/util.h
>index a769065..74dcb1c 100644
>--- a/src/dhcp-manager/systemd-dhcp/src/shared/util.h
>+++ b/src/dhcp-manager/systemd-dhcp/src/shared/util.h
>@@ -863,7 +863,7 @@ static inline unsigned log2u_round_up(unsigned x) {
> }
> 
> static inline bool logind_running(void) {
>-        return access("/run/systemd/seats/", F_OK) >= 0;
>+        return access(RUNSTATEDIR "/systemd/seats/", F_OK) >= 0;
> }
> 
> #define DECIMAL_STR_WIDTH(x)                            \
>diff --git a/src/dnsmasq-manager/nm-dnsmasq-manager.c b/src/dnsmasq-manager/nm-dnsmasq-manager.c
>index 2f38ea7..02e6d4f 100644
>--- a/src/dnsmasq-manager/nm-dnsmasq-manager.c
>+++ b/src/dnsmasq-manager/nm-dnsmasq-manager.c
>@@ -104,7 +104,7 @@ nm_dnsmasq_manager_new (const char *iface)
> 
> 	priv = NM_DNSMASQ_MANAGER_GET_PRIVATE (manager);
> 	priv->iface = g_strdup (iface);
>-	priv->pidfile = g_strdup_printf (LOCALSTATEDIR "/run/nm-dnsmasq-%s.pid", iface);
>+	priv->pidfile = g_strdup_printf (RUNSTATEDIR "/nm-dnsmasq-%s.pid", iface);
> 
> 	return manager;
> }
>-- 
>1.8.5.5
>
Comment 9 Thomas Haller 2015-05-21 15:19:42 UTC
(In reply to Thomas Haller from comment #8)

Ups. Sorry for the spam.
Comment 10 Thomas Haller 2015-05-21 15:20:44 UTC
also, this the following in src/Makefile.am seems not working as expected:

 rundir=$(runstatedir)/NetworkManager
 statedir=$(localstatedir)/lib/NetworkManager
 install-data-hook:
     $(mkinstalldirs) -m 0700 $(DESTDIR)$(rundir)
     $(mkinstalldirs) -m 0700 $(DESTDIR)$(statedir)
     $(mkinstalldirs) -m 0755 $(DESTDIR)$(pkglibdir)

anybody know why?
Comment 11 Beniamino Galvani 2015-05-27 16:18:18 UTC
(In reply to Thomas Haller from comment #10)
> also, this the following in src/Makefile.am seems not working as expected:
> 
>  rundir=$(runstatedir)/NetworkManager
>  statedir=$(localstatedir)/lib/NetworkManager
>  install-data-hook:
>      $(mkinstalldirs) -m 0700 $(DESTDIR)$(rundir)
>      $(mkinstalldirs) -m 0700 $(DESTDIR)$(statedir)
>      $(mkinstalldirs) -m 0755 $(DESTDIR)$(pkglibdir)
> 
> anybody know why?

The reason could be that $(runstatedir) is not a global variable in autoconf < 2.70 and thus is not exported to generated Makefiles as other variables (localstatedir for example). Maybe we should add an AC_SUBST statement in configure.ac to define it when autoconf < 2.70?
Comment 12 Beniamino Galvani 2015-05-28 07:13:06 UTC
This should be enough:

 # Add runstatedir if not specified manually in autoconf < 2.70
 AS_IF([test -z "$runstatedir"], runstatedir="$localstatedir/run")
+AC_SUBST(runstatedir)

The rest seems OK to me.
Comment 13 Thomas Haller 2015-05-28 10:00:21 UTC
(In reply to Beniamino Galvani from comment #12)
> This should be enough:
> 
>  # Add runstatedir if not specified manually in autoconf < 2.70
>  AS_IF([test -z "$runstatedir"], runstatedir="$localstatedir/run")
> +AC_SUBST(runstatedir)
> 
> The rest seems OK to me.

thanks. This works.


merged to master as 

http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=cd3c52a24da9b374776e448aa038c3ea8dc3ac51

Closing this bug as resolved. Please reopen if there are issues (or open a new BZ).