[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Freeze Exception For Ampache



On Sat, 2010-09-11 at 05:20 -0500, charlie wrote:
> I would like to request a freeze exception for ampache-3.5.4-8.
[...]
> I have also took this opportunity to fix bug #593759 which removes an
> unused variable in the postinst and postrm which causes the
> apache_install and apache_remove functions to fail.  This causes the
> initial web install interface not to be show after install.

I'm afraid this doesn't really make sense.  apache_{install,remove}
don't care what parameters are passed to them, and passing an extra
parameter won't cause them to fail.

I spent a little while this evening looking at the package, and noticed
that whilst the initial install did not set up the symlinks and
configure apache, it worked fine on a dpkg-reconfigure.

The problem seems to be that the config script explicitly calls db_set
to add each question to the debconf database with an answer of ""; I'm
not entirely sure why this was done, but removing the db_set calls makes
"apt-get install" do what I'd expect.

The problem with the postrm is that it retrieves the value of the
"configure and restart the webserver" question, stores it in $webserver,
and then iterates (the unset) $webservers to determine which servers to
restart.  It should instead retrieve the value of the "which webservers
to configure" question and iterate that; IMHO, it should also only do so
if the answer to the configuration question was "yes".

The attached debdiff (generated with -w to reduce the noise due to
indentation changes) implements both of these changes and modifies the
postinst to only configure the webserver when called with "configure",
not also with "upgrade".

fwiw, I also found the wording of the "configure and restart the web
server" question slightly confusing, as it suggests that the
configuration will be performed in any case and only the restarting will
not be done automatically.

Regards,

Adam
diff -Nru -w ampache-3.5.4/debian/ampache.config ampache-3.5.4/debian/ampache.config
--- ampache-3.5.4/debian/ampache.config	2010-08-02 05:06:51.000000000 +0100
+++ ampache-3.5.4/debian/ampache.config	2010-09-11 20:00:55.000000000 +0100
@@ -9,10 +9,8 @@
 . /usr/share/debconf/confmodule
 db_version 2.0 || [ 0 -lt 30 ]
 
-db_set ampache/webserver_type || true
 db_input critical ampache/webserver_type || true
 db_go || true
 
-db_set ampache/restart_webserver || true
 db_input critical ampache/restart_webserver || true
 db_go || true
diff -Nru -w ampache-3.5.4/debian/ampache.postinst ampache-3.5.4/debian/ampache.postinst
--- ampache-3.5.4/debian/ampache.postinst	2010-08-16 04:17:57.000000000 +0100
+++ ampache-3.5.4/debian/ampache.postinst	2010-09-11 19:06:01.000000000 +0100
@@ -34,7 +34,7 @@
     fi
 }
 
-if [ "$1" = "configure" ] || [ "$1" = "upgrade" ]; then 
+if [ "$1" = "configure" ]; then 
 
     db_get ampache/webserver_type || true
     webservers="$RET"
diff -Nru -w ampache-3.5.4/debian/ampache.postrm ampache-3.5.4/debian/ampache.postrm
--- ampache-3.5.4/debian/ampache.postrm	2010-08-02 05:06:51.000000000 +0100
+++ ampache-3.5.4/debian/ampache.postrm	2010-09-11 18:15:29.000000000 +0100
@@ -37,8 +37,9 @@
 if [ "$1" = "remove" ] || [ "$1" = "purge" ]; then
    #Ask the question and restart web server.
    db_get ampache/restart_webserver || true
-   
-   webserver="$RET"
+    if [ "$RET" = "true" ]; then
+        db_get ampache/webserver_type || true
+        webservers="$RET"
    for webserver in $webservers; do
        webserver=${webserver%,}
        if [ "$webserver" = "lighttpd" ] ; then
@@ -53,6 +54,7 @@
        fi
    done
 fi
+fi
 
 if [ "$1" = "purge" ]; then
     #This is needed to remove user added content.

Reply to: