Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Michael Biebl]

Am 12.04.2017 um 16:20 schrieb Alf Gaida:
>> Please use the actual path, not some non-existing abbreviation.
> will do so
> 
>> This is upgrade code from pre-jessie. This can be dropped.
> i'm fine with - but we need some kind of daemon restart

What do you mean? Remember that dh_installinit/dh_systemd_start will
already create code to restart the daemon after upgrade.

> 
>> We should probably check for both paths here, for upgrade safety, i.e
>> whether /etc/resolv.conf points to the old *or* new path.
> ok, better safe than sorry.
>>> diff --git a/debian/network-manager.preinst
>>> b/debian/network-manager.preinst
>>> index 9fff61d..b4c8a05 100644
>>> --- a/debian/network-manager.preinst
>>> +++ b/debian/network-manager.preinst
>>> @@ -45,6 +45,11 @@ case "$1" in
>>>   if dpkg --compare-versions "$2" lt "1.2.2-2"; then
>>>   deb-systemd-helper purge NetworkManager-wait-online.service
>>>   fi
>>> +
>>> +# Delete a symlink that points to /var/run
>>> +if [ "$(readlink /etc/resolv.conf)" =
>>> /var/run/NetworkManager/resolv.conf ] ; then
>>> +rm -f /etc/resolv.conf
>>> +fi
>> Why is that necessary? This appears to be a risky change with no clear
>> explanation why it's needed.
>>
> in case the symlink points to the old location just remove the symlink -
> an alternative would be the empty file - so NetworkManger can recreate
> the link to the new location at the next start - there is no risk in
> this. this is the same as in the other p-scripts - but you are right,
> the intention should be documented

It's still not clear to me why it's necessary to remove the file.
The file in /var/run won't magically be gone.
What problem exactly does this solve?

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Alf Gaida]

Great!


Am 12.04.2017 um 13:09 schrieb Michael Biebl:

Am 11.04.2017 um 21:59 schrieb Alf Gaida:

index 39402de..ae640de 100755
--- a/debian/rules
+++ b/debian/rules
@@ -47,7 +47,8 @@ override_dh_auto_configure:
--enable-json-validation \
--enable-bluez5-dun \
--with-selinux=yes \
-   --with-libaudit=yes
+   --with-libaudit=yes \
+   --runstatedir=/run

Fwiw, I've asked the debhelper maintainer to make /run the new default
for runstatedir in a future debhelper compat bump.

See
https://anonscm.debian.org/cgit/debhelper/debhelper.git/commit/?id=ea1b656f937c60f0d06d5272c149cb873b7d8ac6

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Alf Gaida]

> Please use the actual path, not some non-existing abbreviation.
will do so

> This is upgrade code from pre-jessie. This can be dropped.
i'm fine with - but we need some kind of daemon restart


> We should probably check for both paths here, for upgrade safety, i.e
> whether /etc/resolv.conf points to the old *or* new path.
ok, better safe than sorry.

diff --git a/debian/network-manager.preinst b/debian/network-manager.preinst
index 9fff61d..b4c8a05 100644
--- a/debian/network-manager.preinst
+++ b/debian/network-manager.preinst
@@ -45,6 +45,11 @@ case "$1" in
if dpkg --compare-versions "$2" lt "1.2.2-2"; then
deb-systemd-helper purge NetworkManager-wait-online.service
fi
+
+   # Delete a symlink that points to /var/run
+   if [ "$(readlink /etc/resolv.conf)" = 
/var/run/NetworkManager/resolv.conf ] ; then
+   rm -f /etc/resolv.conf
+   fi

Why is that necessary? This appears to be a risky change with no clear
explanation why it's needed.

in case the symlink points to the old location just remove the symlink - 
an alternative would be the empty file - so NetworkManger can recreate 
the link to the new location at the next start - there is no risk in 
this. this is the same as in the other p-scripts - but you are right, 
the intention should be documented


Will rework the patch

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Michael Biebl]

Am 11.04.2017 um 21:59 schrieb Alf Gaida:
> index 39402de..ae640de 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -47,7 +47,8 @@ override_dh_auto_configure:
>   --enable-json-validation \
>   --enable-bluez5-dun \
>   --with-selinux=yes \
> - --with-libaudit=yes
> + --with-libaudit=yes \
> + --runstatedir=/run

Fwiw, I've asked the debhelper maintainer to make /run the new default
for runstatedir in a future debhelper compat bump.

See
https://anonscm.debian.org/cgit/debhelper/debhelper.git/commit/?id=ea1b656f937c60f0d06d5272c149cb873b7d8ac6

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Michael Biebl]

Thanks for the patch. A few questions/comments

Am 11.04.2017 um 21:59 schrieb Alf Gaida:
> diff --git a/debian/changelog b/debian/changelog
> index e93e091..efc12e9 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,12 @@
> +network-manager (1.6.2-3.3) unstable; urgency=medium
> +
> +  [ Alf Gaida ]
> +  * run pid in /run/NM instead of /var/run/NM

Please use the actual path, not some non-existing abbreviation.

> +  * remove a symlink to /var/run in preinst
> +  * configure with runstatedir=/run (Closes: #860045)
> +
> + -- Alf Gaida   Tue, 11 Apr 2017 21:50:52 +0200
> +
>  network-manager (1.6.2-3) unstable; urgency=medium
>  
>* device/wifi: block autoconnect while scanning is in progress.
> diff --git a/debian/network-manager.init b/debian/network-manager.init
> index b21c40f..f7aecff 100644
> --- a/debian/network-manager.init
> +++ b/debian/network-manager.init
> @@ -18,7 +18,7 @@ NAME="NetworkManager"
>  
>  DAEMON=/usr/sbin/$NAME
>  
> -PIDFILE=/var/run/$NAME/$NAME.pid
> +PIDFILE=/run/$NAME/$NAME.pid
>  
>  SCRIPTNAME=/etc/init.d/network-manager
>  
> diff --git a/debian/network-manager.postinst b/debian/network-manager.postinst
> index bcb634d..a72383b 100644
> --- a/debian/network-manager.postinst
> +++ b/debian/network-manager.postinst
> @@ -28,7 +28,7 @@ case "$1" in
>   if [ -d /run/systemd/system ] && \
> [ -f /var/run/NetworkManager/NetworkManager.pid ]; then
>   start-stop-daemon --stop --retry 5 --quiet --exec 
> /usr/sbin/NetworkManager \
> -   --pidfile /var/run/NetworkManager/NetworkManager.pid || true
> +   --pidfile /run/NetworkManager/NetworkManager.pid || true
>   fi
>   fi

This is upgrade code from pre-jessie. This can be dropped.


>  
> diff --git a/debian/network-manager.postrm b/debian/network-manager.postrm
> index f0d9188..3d83170 100644
> --- a/debian/network-manager.postrm
> +++ b/debian/network-manager.postrm
> @@ -25,7 +25,7 @@ case "$1" in
>  # Replace /etc/resolv.conf symlink with an empty file on package
>  # removal to signal other packages that NetworkManager is no longer
>  # controlling the file. (See: #826366)
> -if [ "$(readlink /etc/resolv.conf)" = 
> /var/run/NetworkManager/resolv.conf ] ; then
> +if [ "$(readlink /etc/resolv.conf)" = 
> /run/NetworkManager/resolv.conf ] ; then

We should probably check for both paths here, for upgrade safety, i.e
whether /etc/resolv.conf points to the old *or* new path.

>  rm -f /etc/resolv.conf
>  touch /etc/resolv.conf
>  fi
> diff --git a/debian/network-manager.preinst b/debian/network-manager.preinst
> index 9fff61d..b4c8a05 100644
> --- a/debian/network-manager.preinst
> +++ b/debian/network-manager.preinst
> @@ -45,6 +45,11 @@ case "$1" in
>   if dpkg --compare-versions "$2" lt "1.2.2-2"; then
>   deb-systemd-helper purge NetworkManager-wait-online.service
>   fi
> +
> + # Delete a symlink that points to /var/run
> + if [ "$(readlink /etc/resolv.conf)" = 
> /var/run/NetworkManager/resolv.conf ] ; then
> + rm -f /etc/resolv.conf
> + fi

Why is that necessary? This appears to be a risky change with no clear
explanation why it's needed.

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Alf Gaida]

Patch attached

Cheers Alf
diff --git a/debian/changelog b/debian/changelog
index e93e091..efc12e9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+network-manager (1.6.2-3.3) unstable; urgency=medium
+
+  [ Alf Gaida ]
+  * run pid in /run/NM instead of /var/run/NM
+  * remove a symlink to /var/run in preinst
+  * configure with runstatedir=/run (Closes: #860045)
+
+ -- Alf Gaida   Tue, 11 Apr 2017 21:50:52 +0200
+
 network-manager (1.6.2-3) unstable; urgency=medium
 
   * device/wifi: block autoconnect while scanning is in progress.
diff --git a/debian/network-manager.init b/debian/network-manager.init
index b21c40f..f7aecff 100644
--- a/debian/network-manager.init
+++ b/debian/network-manager.init
@@ -18,7 +18,7 @@ NAME="NetworkManager"
 
 DAEMON=/usr/sbin/$NAME
 
-PIDFILE=/var/run/$NAME/$NAME.pid
+PIDFILE=/run/$NAME/$NAME.pid
 
 SCRIPTNAME=/etc/init.d/network-manager
 
diff --git a/debian/network-manager.postinst b/debian/network-manager.postinst
index bcb634d..a72383b 100644
--- a/debian/network-manager.postinst
+++ b/debian/network-manager.postinst
@@ -28,7 +28,7 @@ case "$1" in
 	if [ -d /run/systemd/system ] && \
 	  [ -f /var/run/NetworkManager/NetworkManager.pid ]; then
 	start-stop-daemon --stop --retry 5 --quiet --exec /usr/sbin/NetworkManager \
-		  --pidfile /var/run/NetworkManager/NetworkManager.pid || true
+		  --pidfile /run/NetworkManager/NetworkManager.pid || true
 	fi
 	fi
 
diff --git a/debian/network-manager.postrm b/debian/network-manager.postrm
index f0d9188..3d83170 100644
--- a/debian/network-manager.postrm
+++ b/debian/network-manager.postrm
@@ -25,7 +25,7 @@ case "$1" in
 # Replace /etc/resolv.conf symlink with an empty file on package
 # removal to signal other packages that NetworkManager is no longer
 # controlling the file. (See: #826366)
-if [ "$(readlink /etc/resolv.conf)" = /var/run/NetworkManager/resolv.conf ] ; then
+if [ "$(readlink /etc/resolv.conf)" = /run/NetworkManager/resolv.conf ] ; then
 rm -f /etc/resolv.conf
 touch /etc/resolv.conf
 fi
diff --git a/debian/network-manager.preinst b/debian/network-manager.preinst
index 9fff61d..b4c8a05 100644
--- a/debian/network-manager.preinst
+++ b/debian/network-manager.preinst
@@ -45,6 +45,11 @@ case "$1" in
 	if dpkg --compare-versions "$2" lt "1.2.2-2"; then
 	deb-systemd-helper purge NetworkManager-wait-online.service
 	fi
+
+	# Delete a symlink that points to /var/run
+	if [ "$(readlink /etc/resolv.conf)" = /var/run/NetworkManager/resolv.conf ] ; then
+	rm -f /etc/resolv.conf
+	fi
 	;;
 
 abort-upgrade)
diff --git a/debian/rules b/debian/rules
index 39402de..ae640de 100755
--- a/debian/rules
+++ b/debian/rules
@@ -47,7 +47,8 @@ override_dh_auto_configure:
 		--enable-json-validation \
 		--enable-bluez5-dun \
 		--with-selinux=yes \
-		--with-libaudit=yes
+		--with-libaudit=yes \
+		--runstatedir=/run
 
 override_dh_install:
 	dh_install -X.la --list-missing


signature.asc
Description: OpenPGP digital signature

Bug#860045: [Pkg-utopia-maintainers] Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Michael Biebl]

Am 10.04.2017 um 19:01 schrieb Alf Gaida:
> Package: network-manager
> Severity: normal
> 
> We live in the 21. century - 
> https://wiki.debian.org/ReleaseGoals/RunDirectory : This is a release goal 
> for Wheezy. 
> 

feel free to send a patch.


-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#860045: Please don't use /var/run/$foo in network-manager anymore

[Alf Gaida]

Package: network-manager
Severity: normal

We live in the 21. century - https://wiki.debian.org/ReleaseGoals/RunDirectory 
: This is a release goal for Wheezy. 

Cheers Alf

-- System Information:
Debian Release: 9.0
  APT prefers buildd-unstable
  APT policy: (500, 'buildd-unstable'), (500, 'unstable'), (500, 'testing'), 
(500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.10.9-towo.1-siduction-amd64 (SMP w/8 CPU cores; PREEMPT)
Locale: LANG=de_DE.utf8, LC_CTYPE=de_DE.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)