Hello, Please find attached a new version of the patch. Here is what it does now: - Ship /etc/ola from package itself instead of postinst - Removed RUN_DAEMON check from init scripts as asked by Helmut - Moved DAEMON_ARGS definition before /etc/default/* sourcing in init scripts to allow overriding - Fully removed ola-rdm-tests postinst script: its only purpose after debconf stuff removal was to provide /etc/default/ola-rdm-tests. Not sure about shipping and empty file or not so I choose to remove it - /etc/default/ola is still removed by ola postrm - /etc/default/ola-rdm-tests is still removed by ola-rdm-tests postinst - debian/changelog has been updated, mentioning all 3 bugs that it closes, as well as all the other changes - Add debian/patches/include_lo_when_looking_for_network_interface.patch which close #769670 (simply changed false to true in common/network/InterfacePickerTest.cpp on line 66 so that network interface lookup include lo)
Speaking about patches, I discovered debian/patches/debian_changes in
source tree. I first thought I screwed up something, but it does not
seem so: apt-get source ola provide me with this patch, before applying
my nmu one. Problem: this patch is 132524 lines long :-/
Building from source is OK via pbuilder (including tests).
Testing with piuparts is OK as well (still with --no-upgrade-test options).
I did not pay that much attention to lintian results, but I thing we're
god here as well.
I still have a doubt about the opportunity to remove RUN_DAEMON checks
from init scripts since we're in a freeze period, but I'm the one who
validate at the end :)
At this point, I think that most of the major issues, if not all, are
now covered.
Please consider having a look on it and let me know if I can upload the
package on mentors.debian.net
Regards,
Jean Baptiste
On 20/11/2014 22:42, Helmut Grohne wrote:
> Hi Wouter and Jean Baptiste,
>
> On Wed, Nov 19, 2014 at 11:35:50PM +0100, Wouter Verhelst wrote:
>> On Tue, Nov 18, 2014 at 01:48:04PM +0100, Jean Baptiste Favre wrote:
>>> * Remove debconf calls from ola-rdm-tests postinst. (Closes: #767676)
>>> * Fix other seriouys issues:
>>> - Provides missing /etc/default/ola from ola postinst script to allow
>>> olad service control in the same way rdm_test_server is
>>
>> NAK. /etc/default/* to manage service status is a menace, and should be
>> forbidden.
>
> Wouter, please bear in mind, that it is not the proposed NMU that is
> adding the need for an /etc/default/ola file. The current init script
> (debian/ola.olad.init) sources /etc/default/ola and doesn't start at all
> when it does not exist. This puts you in a bad position to demand
> otherwise.
>
> Given the other things you said, I assume that it is not your intention
> for the init script to check for $RUN_DAEMON at all. (The issue is
> duplicated in the init script of ola-rdm-tests.)
>
> I suggest the following approach then:
>
> * Remove the check for $RUN_DAEMON from both init scripts (olad and
> rdm_test_server).
> * Do not ship the default files in either package.
> * Keep sourcing /etc/default/ola{,-rdm-tests} so users can change e.g.
> $DAEMON_ARGS.
>
> Optionally:
> * During upgrade, check whether the default files were unmodified (by
> comparing their md5sum to known values) and if they are unmodified,
> remove them.
>
> Wouter, do you agree with this approach?
>
> Jean Baptiste, would you like to invest further work into this?
>
> The "unowned file after purge issue" has bug number #769331 and should
> be mentioned in the changelog.
>
> Since I am building in pbuilder, #769670 will need fixing as well. What
> happens there is that pbuilder creates a new network namespace (using
> "unshare -n" from util-linux) and runs the build with just lo being
> available. The source code for the test is as follows.
>
> common/network/InterfacePickerTest.cpp:
> | auto_ptr<InterfacePicker> picker(InterfacePicker::NewPicker());
> | vector<Interface> interfaces = picker->GetInterfaces(false);
> | OLA_ASSERT_TRUE(interfaces.size() > 0);
>
> The false value in the second line is bound to a parameter called
> "include_loopback". So the test examines all interfaces, ignores "lo"
> and errors out, because it didn't find any (other than lo). The test
> simply doesn't make any sense. It verifies a property that does not hold
> in general.
>
> Given the zoo of issues in ola, I am not convinced that it is ready for
> being part of a stable release. It didn't build on any buildd either
> (see #755866). I am niether convinced that the "missing source" issue
> (#760414) really is non-RC, because the source for the minified
> jquery-ui library is not in the Debian archive. It is only available in
> a different version and we have previously declared "source is only
> available in a different version" as RC buggy.
>
> Helmut
>
> !DSPAM:546e606b17881612210777!
>
>
diff -Nru ola-0.9.1/debian/changelog ola-0.9.1/debian/changelog
--- ola-0.9.1/debian/changelog 2014-08-17 10:07:29.000000000 +0200
+++ ola-0.9.1/debian/changelog 2014-11-21 12:25:21.000000000 +0100
@@ -1,3 +1,21 @@
+ola (0.9.1-1.1) unstable; urgency=medium
+
+ * Non-maintainer upload
+ * Remove debconf calls from ola-rdm-tests postinst (Closes: #767676)
+ * Ship /etc/ola within ola & ola-rdm-tests packages (Closes: #769331)
+ * Add a patch which fix failing test when running in pbuilder
+ (Closes: #769670)
+ * Fix other important issues:
+ - Remove the check for $RUN_DAEMON from both init scripts olad and
+ rdm_test_server (Package maintainer disagree about this way of
+ controling service)
+ - Remove ola-rdm-tests postinst since its only purpose was to provide
+ /etc/default/ola-rdm-tests
+ - Add postrm scripts for packages ola & ola-rdm-tests to fully remove
+ configuration files so that piuparts tests can pass
+
+ -- Jean Baptiste Favre <deb...@jbfavre.org> Sun, 16 Nov 2014 17:44:18 +0100
+
ola (0.9.1-1) unstable; urgency=low
* New upstream release
diff -Nru ola-0.9.1/debian/ola.dirs ola-0.9.1/debian/ola.dirs
--- ola-0.9.1/debian/ola.dirs 2014-04-21 08:19:26.000000000 +0200
+++ ola-0.9.1/debian/ola.dirs 2014-11-21 12:02:25.000000000 +0100
@@ -1,3 +1,4 @@
+etc/ola
usr/bin
usr/lib
usr/share/olad/www
diff -Nru ola-0.9.1/debian/ola.olad.init ola-0.9.1/debian/ola.olad.init
--- ola-0.9.1/debian/ola.olad.init 2014-08-17 09:17:40.000000000 +0200
+++ ola-0.9.1/debian/ola.olad.init 2014-11-21 12:03:42.000000000 +0100
@@ -16,16 +16,11 @@
PIDFILE=/var/run/$NAME.pid
DESC="OLA daemon"
USER=olad
+DAEMON_ARGS="--syslog --log-level 3 --config-dir /etc/ola"
# Reads config file (will override defaults above)
[ -r /etc/default/ola ] && . /etc/default/ola
-if [ "$RUN_DAEMON" = "true" ] || [ "$RUN_DAEMON" = "yes" ] ; then
- DAEMON_ARGS="--syslog --log-level 3 --config-dir /etc/ola"
-elif [ "$1" = "start" ] || [ "$1" = "stop" ] ; then
- echo "The init script is currently inactive;\nuse \"dpkg-reconfigure ola\" to change this." >&2
-fi
-
[ -x "$DAEMON" ] || exit 0
. /lib/lsb/init-functions
diff -Nru ola-0.9.1/debian/ola.postinst ola-0.9.1/debian/ola.postinst
--- ola-0.9.1/debian/ola.postinst 2014-08-17 09:17:40.000000000 +0200
+++ ola-0.9.1/debian/ola.postinst 2014-11-21 12:04:43.000000000 +0100
@@ -8,13 +8,8 @@
groups olad | grep plugdev > /dev/null || adduser olad plugdev
# setup the config dir
-CONF_DIR=/etc/ola
-if [ ! -d ${CONF_DIR} ]; then
- mkdir -p ${CONF_DIR};
- chown -R olad:olad ${CONF_DIR};
- chmod g+s ${CONF_DIR};
-fi;
-
+chown -R olad:olad /etc/ola;
+chmod g+s /etc/ola;
# dh_installdeb will replace this with shell code automatically
# generated by other debhelper scripts.
diff -Nru ola-0.9.1/debian/ola.postrm ola-0.9.1/debian/ola.postrm
--- ola-0.9.1/debian/ola.postrm 1970-01-01 01:00:00.000000000 +0100
+++ ola-0.9.1/debian/ola.postrm 2014-11-21 12:02:25.000000000 +0100
@@ -0,0 +1,25 @@
+#!/bin/sh
+# postrm script for ola
+
+set -e
+
+case "$1" in
+ purge)
+ if [ -f /etc/default/ola ]; then
+ rm -f /etc/default/ola
+ fi
+ ;;
+
+ remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
+ ;;
+
+ *)
+ echo "postrm called with unknown argument \`$1'" >&2
+ exit 1
+ ;;
+
+esac
+
+#DEBHELPER#
+
+exit 0
diff -Nru ola-0.9.1/debian/ola-rdm-tests.dirs ola-0.9.1/debian/ola-rdm-tests.dirs
--- ola-0.9.1/debian/ola-rdm-tests.dirs 2014-04-21 08:19:26.000000000 +0200
+++ ola-0.9.1/debian/ola-rdm-tests.dirs 2014-11-21 12:02:25.000000000 +0100
@@ -1,3 +1,4 @@
+etc/ola
usr/bin
usr/lib
usr/share
diff -Nru ola-0.9.1/debian/ola-rdm-tests.postinst ola-0.9.1/debian/ola-rdm-tests.postinst
--- ola-0.9.1/debian/ola-rdm-tests.postinst 2014-04-21 08:19:26.000000000 +0200
+++ ola-0.9.1/debian/ola-rdm-tests.postinst 1970-01-01 01:00:00.000000000 +0100
@@ -1,64 +0,0 @@
-#!/bin/sh -e
-# postinst script for ola-rdm-tests
-
-conffile="/etc/default/ola-rdm-tests"
-
-update_config_file() {
- db_field=$1
- config_field=$2
-
- RET=false
- db_get $db_field
- if [ -n "$RET" ] ; then
- if grep -q "^$config_field" $conffile ; then
- # keep any admin changes, while replacing the variable content
- sed "s/^[ ]*$config_field=\".*\"/$config_field=\"$RET\"/" < $conffile > $conffile.new &&
- mv $conffile.new $conffile
- else
- echo "$config_field=\"$RET\"" >> $conffile
- fi
- fi
-}
-
-# Source debconf library -- we have a Depends line
-# to make sure it is there...
-. /usr/share/debconf/confmodule
-db_version 2.0
-
-case "$1" in
- configure)
- if [ -f $conffile ] ; then
- sed -i -e 's/^[ ]*DAEMON/RUN_DAEMON/g' $conffile
- else
- cat << EOF > $conffile
-# Defaults for ola-rdm-tests initscript (/etc/init.d/ola-rdm-tests)
-# This is a POSIX shell fragment
-
-# [automatically edited by postinst, do not change line format ]
-
-# ola-rdm-tests daemon switch. If set to true, rdm_test_server.py will run.
-RUN_DAEMON="true"
-EOF
- fi
-
- update_config_file ola-rdm-tests/daemon RUN_DAEMON
-
- db_stop
- ;;
-
- abort-upgrade|abort-remove|abort-deconfigure)
- ;;
-
- *)
- echo "postinst called with unknown argument \`$1'" >&2
- exit 1
- ;;
-esac
-
-
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
-
-exit 0
diff -Nru ola-0.9.1/debian/ola-rdm-tests.postrm ola-0.9.1/debian/ola-rdm-tests.postrm
--- ola-0.9.1/debian/ola-rdm-tests.postrm 1970-01-01 01:00:00.000000000 +0100
+++ ola-0.9.1/debian/ola-rdm-tests.postrm 2014-11-21 12:02:25.000000000 +0100
@@ -0,0 +1,25 @@
+#!/bin/sh
+# postrm script for ola-rdm-tests
+
+set -e
+
+case "$1" in
+ purge)
+ if [ -f /etc/default/ola-rdm-tests ]; then
+ rm -f /etc/default/ola-rdm-tests
+ fi
+ ;;
+
+ remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
+ ;;
+
+ *)
+ echo "postrm called with unknown argument \`$1'" >&2
+ exit 1
+ ;;
+
+esac
+
+#DEBHELPER#
+
+exit 0
diff -Nru ola-0.9.1/debian/ola-rdm-tests.rdm_test_server.init ola-0.9.1/debian/ola-rdm-tests.rdm_test_server.init
--- ola-0.9.1/debian/ola-rdm-tests.rdm_test_server.init 2014-08-17 09:17:40.000000000 +0200
+++ ola-0.9.1/debian/ola-rdm-tests.rdm_test_server.init 2014-11-21 12:04:03.000000000 +0100
@@ -17,16 +17,11 @@
PIDFILE=/var/run/$CMD.pid
DESC="OLA RDM Test Server"
USER=olad
+DAEMON_ARGS="--world-writeable"
# Reads config file (will override defaults above)
[ -r /etc/default/ola-rdm-tests ] && . /etc/default/ola-rdm-tests
-if [ "$RUN_DAEMON" = "true" ] || [ "$RUN_DAEMON" = "yes" ] ; then
- DAEMON_ARGS="--world-writeable"
-elif [ "$1" = "start" ] || [ "$1" = "stop" ] ; then
- echo "The init script is currently inactive;\nuse \"dpkg-reconfigure ola-rdm-tests\" to change this." >&2
-fi
-
[ -x "$DAEMON" ] || exit 0
. /lib/lsb/init-functions
diff -Nru ola-0.9.1/debian/patches/include_lo_when_looking_for_network_interface.patch ola-0.9.1/debian/patches/include_lo_when_looking_for_network_interface.patch
--- ola-0.9.1/debian/patches/include_lo_when_looking_for_network_interface.patch 1970-01-01 01:00:00.000000000 +0100
+++ ola-0.9.1/debian/patches/include_lo_when_looking_for_network_interface.patch 2014-11-21 12:22:39.000000000 +0100
@@ -0,0 +1,22 @@
+Description: Include lo interface lookup for InterfacePicker.
+ Package tests fails when running in pbuilder because InterfacePickerTest does not find
+ any suitable network interface.
+ pbuilder creates a new network namespace (using "unshare -n" from util-linux) and runs
+ the build with just lo being available.
+ Removing false flag passed to picker->GetInterfaces() allows to include lo in search,
+ thus allowing tests to complete.
+Author: Jean Baptiste Favre <deb...@jbfavre.org>
+Last-Update: 2014-11-21
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/common/network/InterfacePickerTest.cpp
++++ b/common/network/InterfacePickerTest.cpp
+@@ -63,7 +63,7 @@ CPPUNIT_TEST_SUITE_REGISTRATION(Interfac
+ */
+ void InterfacePickerTest::testGetInterfaces() {
+ auto_ptr<InterfacePicker> picker(InterfacePicker::NewPicker());
+- vector<Interface> interfaces = picker->GetInterfaces(false);
++ vector<Interface> interfaces = picker->GetInterfaces(true);
+ OLA_ASSERT_TRUE(interfaces.size() > 0);
+
+ vector<Interface>::iterator iter;
diff -Nru ola-0.9.1/debian/patches/series ola-0.9.1/debian/patches/series
--- ola-0.9.1/debian/patches/series 2014-04-21 11:13:06.000000000 +0200
+++ ola-0.9.1/debian/patches/series 2014-11-21 12:16:40.000000000 +0100
@@ -1 +1,2 @@
debian-changes
+include_lo_when_looking_for_network_interface.patch
signature.asc
Description: OpenPGP digital signature
