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

Re: please allow mailman/1:2.1.11-5



* Thijs Kinkhorst [Tue, 11 Nov 2008 16:53:14 +0100]:

> Hi,

Hello,

> Please allow mailman/1:2.1.11-5 to fix a release critical bug. Adeodato
> already unblocked -4 for this bug, but another upload was necessary to
> handle a special case in the init script. Changelog below.

> mailman  (1:2.1.11-5) unstable; urgency=high

>    * Make init script also cope with non-specified site list.

>  -- Thijs Kinkhorst <thijs@debian.org>  Sun, 09 Nov 2008 11:26:46 +0100

I reviewed -5 the other day already, and had some concerns. Good thing
you wrote. Though I now see some of them are not a regression.

> SITE_LIST=$( grep '^MAILMAN_SITE_LIST' /etc/mailman/mm_cfg.py | cut -d"'" -f 2 )

That cut is smelly; what if double quotes are used? (It is a Python
file, right?) I'd go, in case you care, for:

  SITE_LIST=$( sed -rne "s/^[[:space:]]*MAILMAN_SITE_LIST[[:space:]]*=[[:space:]]*(['\"])([^'\"]+)\\1/\\2/p" /etc/mailman/mm_cfg.py )

(It breaks if the name contains a quote.)

> [ -z "$SITE_LIST" ] && SITE_LIST='mailman'

AFAIK that fails with `set -e`. You need:

  [ -n "$SITE_LIST" ] || SITE_LIST='mailman'

> if [ "$(/var/lib/mailman/bin/list_lists -b | grep ^${SITE_LIST}$ )" = "" ]; then

Just cosmetic, but while we're in the review business, again if you care
to change:

  if ! /var/lib/mailman/bin/list_lists -b | grep -q "^${SITE_LIST}$"; then


Anyway, only the `set -e` bit is important, please upload to fix at
least that one.

HTH,

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                            Listening to: James Blunt - Where Is My Mind


Reply to: