Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 86099 - dspam-3.4.0-r1 and dspam-3.4.1 - more ebuild bugs than I can shake a stick at
Summary: dspam-3.4.0-r1 and dspam-3.4.1 - more ebuild bugs than I can shake a stick at
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All All
: High normal (vote)
Assignee: Lim Swee Tat (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-21 00:57 UTC by Mike Nerone
Modified: 2005-04-06 18:23 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Patch to dspam-3.4.0-r1.ebuild to fix above problems. (dspam-3.4.0-r1.ebuild.patch,7.04 KB, patch)
2005-03-21 01:00 UTC, Mike Nerone
Details | Diff
New patch to dspam-3.4.0-r1.ebuild (dspam-3.4.0-r1.ebuild.patch,7.45 KB, patch)
2005-03-21 01:37 UTC, Mike Nerone
Details | Diff
Yet another updated patch (dspam-3.4.0-r1.ebuild.patch,10.79 KB, patch)
2005-03-21 12:00 UTC, Mike Nerone
Details | Diff
1/Xth patch update, as X approaches zero (j/k) (dspam-3.4.0-r1.ebuild.patch,12.27 KB, patch)
2005-03-21 12:50 UTC, Mike Nerone
Details | Diff
Now for the new dspam-3.4.1.ebuild (dspam-3.4.1.ebuild.patch,11.64 KB, patch)
2005-03-23 11:52 UTC, Mike Nerone
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Nerone 2005-03-21 00:57:13 UTC
Tracked down and fixed many bugs in this ebuild (patch follows):

gentoo-provided docs overriding upstream docs (again - see bug #83779)
upstream has renamed many of the docs and put them into a new subdir (doc),
  which is not being included (and consequently, some of the dodocs are no-ops
  now)
system.log was being created in / instead of LOGDIR
an obvious case of search-and-replace-run-amok resulted in many places where
"--enable-virtual-users-users" was substituted for "--enable-virtual-users",
"virtual-users_users.sql" for "virtual_users.sql", and
"virtual-users users" for "virtual users". I think I got all of them. Note that
  this is also mentioned in bug #85939.
The mandir-related sed-fu didn't match anything, and are not needed anyway
  because the Makefile honors DESTDIR already (I can only assume that at one
  time they did not ;) )
--with-dspam-home is not set because it is skipped if virtual-users is in USE.
  dspam-home needs to be set either way, and it seems likely that this logic was
  actually intended to be applied to --enable-homedir, not --with-dspam-home
  (this is also what was resulting in the creation of the extraneous dir,
  "/usr/var/dspam"). This should fix bug #81596.
Needlessly pervasive keepdirs:
  HOMEDIR (keepdir's purpose is to protect empty directories from being deleted
    by autocleans. HOMEDIR is not empty, so keepdir is unnecessary and
    inappropriate here)
  HOMEDIR/txt (ditto)
  LOGDIR (ditto)
  cron.daily (ditto, plus dspam is not the logical "owner" of /etc/cron.daily,
    anyway - cronbase is, so it's cronbase's job to keepdir this directory
    (which it, in fact, does))
  logrotate.d (ditto cron.daily, but logrotate instead of cronbase)

One other thing I wanted to mention that this patch DOESN'T fix: with all of the storage drivers except mysql and postgres, virtual-users is being enabled regardless of the virtual-users USE flag. I don't know anything about these drivers and don't feel like researching it, but I thought this might actually be intentional, so I'm not touching it. Just wanted to point it out just in case.
Comment 1 Mike Nerone 2005-03-21 01:00:06 UTC
Created attachment 54028 [details, diff]
Patch to dspam-3.4.0-r1.ebuild to fix above problems.
Comment 2 Mike Nerone 2005-03-21 01:37:03 UTC
Created attachment 54029 [details, diff]
New patch to dspam-3.4.0-r1.ebuild

Found one more bug. dspam.conf was not properly being modified for the mysql
driver (the sed line was missing the "-i" so the output was going to the screen
instead of editing the file itself). This patch includes the fix along with
everything in the first patch.
Comment 3 Mike Nerone 2005-03-21 01:38:46 UTC
Oh and if anyone cares, the patched ebuild appears to work for dspam-3.4.1, as well (which I haven't tested in production).
Comment 4 steveb 2005-03-21 02:50:23 UTC
While we are editing this ebuild, could we please change this part as well?

from:
PGUSER=${DSPAM_PgSQL_USER} PGPASSWORD=${DSPAM_PgSQL_PWD} /usr/bin/psql -d ${DSPAM_PgSQL_DB} -U ${DSPAM_PgSQL_USER} -f ${HOMEDIR}/pgsql_virtual_users.sql 1>/dev/null 2>&1

To
if use neural ; then
	PGUSER=${DSPAM_PgSQL_USER} PGPASSWORD=${DSPAM_PgSQL_PWD} /usr/bin/psql -d ${DSPAM_PgSQL_DB} -U ${DSPAM_PgSQL_USER} -f ${HOMEDIR}/pgsql_virtual_users.sql 1>/dev/null 2>&1
fi

Comment 5 steveb 2005-03-21 02:51:57 UTC
AAGGGRAR!!! Copy & Paste error!!

should be:

from:
PGUSER=${DSPAM_PgSQL_USER} PGPASSWORD=${DSPAM_PgSQL_PWD} /usr/bin/psql -d ${DSPAM_PgSQL_DB} -U ${DSPAM_PgSQL_USER} -f ${HOMEDIR}/pgsql_virtual_users.sql 1>/dev/null 2>&1

to:
if use virtual-users ; then
        PGUSER=${DSPAM_PgSQL_USER} PGPASSWORD=${DSPAM_PgSQL_PWD} /usr/bin/psql -d ${DSPAM_PgSQL_DB} -U ${DSPAM_PgSQL_USER} -f ${HOMEDIR}/pgsql_virtual_users.sql 1>/dev/null 2>&1
fi
Comment 6 steveb 2005-03-21 02:58:40 UTC
Since using just one sed command is common in the ebuild, the initial configuration building could be as well simplified to:

	# build some initial configuration data
	cp src/dspam.conf ${T}/dspam.conf
	if use cyrus; then
		sed -e 's:^#*\(UntrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/lib/cyrus/deliver %u\":gI' \
			-e 's:^\(TrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/lib/cyrus/deliver\":gI' \
			-i ${T}/dspam.conf
	elif use exim; then
		sed -e 's:^#*\(UntrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/sbin/exim -oMr spam-scanned %u\":gI' \
			-e 's:^\(TrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/sbin/exim -oMr spam-scanned %u\":gI' \
			-i ${T}/dspam.conf
	elif use maildrop; then
		sed -e 's:^#*\(UntrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/bin/maildrop -d %u\":gI' \
			-e 's:^\(TrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/bin/maildrop\":gI' \
			-i ${T}/dspam.conf
	elif use procmail; then
		sed -e 's:^#*\(UntrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/bin/procmail -d %u\":gI' \
			-e 's:^\(TrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/bin/procmail":gI' \
			-i ${T}/dspam.conf
	else
		sed -e 's:^#*\(UntrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/sbin/sendmail\":gI' \
			-e 's:^\(TrustedDeliveryAgent\)[\t ]*.*:\1 \"/usr/sbin/sendmail\":gI' \
			-i ${T}/dspam.conf
	fi
Comment 7 steveb 2005-03-21 03:03:20 UTC
The Oracle and SQLite part should honor the new virtual-users USE flag as well:

	elif use oci8 ; then
		myconf="${myconf} --with-storage-driver=ora_drv"
		myconf="${myconf} --with-oracle-home=${ORACLE_HOME}"
		use virtual-users && myconf="${myconf} --enable-virtual-users"

		# I am in no way a Oracle specialist. If someone knows
		# how to query the version of Oracle, then let me know.
		if (expr ${ORACLE_HOME/*\/} : 10 1>/dev/null 2>&1); then
			myconf="${myconf} --with-oracle-version=10"
		fi
	elif use sqlite3 ; then
		myconf="${myconf} --with-storage-driver=sqlite3_drv"
		use virtual-users && myconf="${myconf} --enable-virtual-users"
	elif use sqlite ; then
		myconf="${myconf} --with-storage-driver=sqlite_drv"
		use virtual-users && myconf="${myconf} --enable-virtual-users"
	else
Comment 8 Mike Nerone 2005-03-21 12:00:42 UTC
Created attachment 54085 [details, diff]
Yet another updated patch

I've added in the changes from comment #6 and made similar changes in a couple
of other places as well (as long as I'm providing patches, I thought I'd help
out as much as possible).

Regarding comment #5 and comment #7, as I said originally, I didn't touch this
because I thought it might actually be intentional for some reason beyond my
ken. At this point, I'm pretty sure that's not the case, though, so this patch
rolls in those changes, as well.
Comment 9 Mike Nerone 2005-03-21 12:50:44 UTC
Created attachment 54090 [details, diff]
1/Xth patch update, as X approaches zero (j/k)

Sigh...found a still more. These are in pkg_config:

For mysql, postgres, and oracle, there were lines like:

  [[ -f ${HOMEDIR}/mysql.data ]] && mv -f ${HOMEDIR}/mysql.data ${HOMEDIR}

I'm not sure what was intended, but it definitely does nothing to move a file
from one directory into the same directory (running the ebuild config says as
much ;) ) I've removed these.

In the mysql section, there were array variables like $DSPAM_DB_DATA[2]. The
{}'s are required with this syntax. (i.e. "${DSPAM_DB_DATA[2]"). This was
causing ebuild config to fail completely.

As usual, this is a cumulative patch for all problems mentioned in this bug.
Comment 10 steveb 2005-03-21 12:55:18 UTC
Please do not optimize this part:
-	sed -i "s:^\(Purge.*\):###\1:g" ${T}/dspam.conf
-	sed -i "s:^#\(Purge.*\):\1:g" ${T}/dspam.conf
-	sed -i "s:^###\(Purge.*\):#\1:g" ${T}/dspam.conf
+	sed -e "s:^\(Purge.*\):###\1:g" \
+		-e "s:^#\(Purge.*\):\1:g" \
+		-e "s:^###\(Purge.*\):#\1:g" \
+		-i ${T}/dspam.conf

This MUST be runing in sequential order. Actualy this does disable the normal purge commands and switches to the one made for SQL backends. I think the above command will work the way it should work, but we need to test it, before rushing again and publishing it into CVS.

Comment 11 steveb 2005-03-21 13:04:06 UTC
Okay... I tested this:
-       sed -i "s:^\(Purge.*\):###\1:g" ${T}/dspam.conf
-       sed -i "s:^#\(Purge.*\):\1:g" ${T}/dspam.conf
-       sed -i "s:^###\(Purge.*\):#\1:g" ${T}/dspam.conf
+       sed -e "s:^\(Purge.*\):###\1:g" \
+               -e "s:^#\(Purge.*\):\1:g" \
+               -e "s:^###\(Purge.*\):#\1:g" \
+               -i ${T}/dspam.conf

It is okay. It can be summarized and does not to call 3 times the sed command. One allone is okay.
Comment 12 Mike Nerone 2005-03-21 13:11:46 UTC
I actually did think about that, but if I'm not mistaken, multiple -e options to sed *still* cause them to be executed in a deterministic sequential order (per-line in this case), so I was 95% (-ish ;D) sure it was safe. Thanks for confirming.
Comment 13 steveb 2005-03-21 13:18:37 UTC
Okay... now change please:

from:
einfo "Pleae read your dspam.conf, oracle.data and 

to:
einfo "Please read your dspam.conf, oracle.data and 


cheers

SteveB


btw: If you look at the ebuilds of Lim, then you will realize that he does not much like if statements :)

If you could change:

from:
		if has_version sys-kernel/linux26-headers; then
			myconf="${myconf} --enable-daemon"
		fi


to:
has_version sys-kernel/linux26-headers && myconf="${myconf} --enable-daemon"


then I think he would be very happy....
Comment 14 Mike Nerone 2005-03-21 13:26:53 UTC
Those fixes seem much more complicated and scare me. :P

But seriously, I might actually roll in a few spelling corrections if I do end up updating this patch due to another functional bug. :)
Comment 15 Manuel McLure 2005-03-21 16:10:23 UTC
Can we add a change to make the permissions of /usr/bin/dspam 6755 instead of 4755? Then you can add apache to the "dspam" group and don't have to do a bunch of suidexec stuff for it to access the dspam data when running dspam-web since everything will belong to the "dspam" group.
Comment 16 Mike Nerone 2005-03-21 22:59:53 UTC
The original thought for this bug was to help with some bug fixes and cleanup in the ebuild. I don't want to go changing actual functionality (I feel like I may have overstepped a bit already with the sed cleanups). Especially since the actual maintainer has not yet chimed in on this patch. You should probably create a new bug with the permissions suggestion, Manuel.
Comment 17 steveb 2005-03-21 23:06:50 UTC
No! Why a new bug report? It is fine to post/ask the permission problem to be fixed in this bug.
Comment 18 Mike Nerone 2005-03-23 11:52:07 UTC
Created attachment 54274 [details, diff]
Now for the new dspam-3.4.1.ebuild

I see that st_lim added a new ebuild for dspam-3.4.1 to the tree, with almost
none of these bugs fixed (two of the lines with the "virtual_users-users" typos
were fixed, but those were the only changed. Here's a new patch against
dspam-3.4.1.ebuild to fix the rest.

Lim Swee Tat!! Are you there? Is this thing on? :P
Comment 19 Haldir 2005-03-25 09:22:59 UTC
I'm not sure if it is related to missing if clauses (and if it is fixed in patch provided above), in the original Ebuild there is no way to get dspam compiled without all that sql stuff, just plain old libdb4.
Atleast offer the option to upgrade to sqlite etc.
If any old user with old libdb4 databases upgrades now, he can't access his old spam dbs anymore.

Most amavisd-new users with dspam functionality enabled will probably stil have the old libdb4 dbs, regardless if it's not "good" to use it anymore and all the others are  way faster.
Comment 20 Timo Maier 2005-03-26 05:24:30 UTC
I noticed that plain db support is missing, too. I would like to stay on db as well. Anyway, to convert the db to SQL you can use dspam_2sql.
Comment 21 steveb 2005-03-26 06:31:35 UTC
Yes. Only SQL based database storage is available in the current DSPAM ebuild. But internaly DSPAM still allows to use Barkley DB. We need to change one block inside the ebuild to still allow Barkley DB.


From:
	elif [ "${multiple_dbs}" -eq "0" ]; then
		echo
		ewarn "You need to select at least one database backend in your USE flags."
		ewarn "Please enable one of the following USE flags:"
		ewarn "  ${supported_dbs}"
		echo
		die "Database support missing"


To:
	elif [ "${multiple_dbs}" -eq "0" ]; then
		echo
		ewarn "You did not select any SQL based database backend. DSPAM will use"
		ewarn "Berkeley DB for storing data. If you don't want that, then enable"
		ewarn "one of the following USE flags:"
		ewarn "  ${supported_dbs}"
		echo
		ewarn "Waiting 30 seconds before starting..."
		ewarn "(Control-C to abort)..."
		epause 30




cheers

Steve
Comment 22 Lim Swee Tat (RETIRED) gentoo-dev 2005-03-28 06:32:46 UTC
Hi,
  This is just great. :)

Ciao
ST Lim
Comment 23 Mike Nerone 2005-03-28 10:35:34 UTC
Re: comment #21...wouldn't it be better to add support for the berkdb USE flag instead of it being implicit (and adding an 'if use berkdb...conf...' line in the appropriate spot)?
Comment 24 Lim Swee Tat (RETIRED) gentoo-dev 2005-04-06 18:23:23 UTC
This has been fixed for some time.  But the last comment just kinda caught me there.