| Summary: | dspam-3.4.0-r1 and dspam-3.4.1 - more ebuild bugs than I can shake a stick at | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Mike Nerone <mike> |
| Component: | Current packages | Assignee: | Lim Swee Tat (RETIRED) <st_lim> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gentoobugzilla, manuel, steeeeeveee |
| Priority: | High | ||
| Version: | 2004.3 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Attachments: |
Patch to dspam-3.4.0-r1.ebuild to fix above problems.
New patch to dspam-3.4.0-r1.ebuild Yet another updated patch 1/Xth patch update, as X approaches zero (j/k) Now for the new dspam-3.4.1.ebuild |
||
|
Description
Mike Nerone
2005-03-21 00:57:13 UTC
Created attachment 54028 [details, diff]
Patch to dspam-3.4.0-r1.ebuild to fix above problems.
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.
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). 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
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
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
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
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. 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.
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.
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.
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. 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....
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. :) 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. 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. No! Why a new bug report? It is fine to post/ask the permission problem to be fixed in this bug. 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
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. 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. 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
Hi, This is just great. :) Ciao ST Lim 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)? This has been fixed for some time. But the last comment just kinda caught me there. |
