DragonFlyBSD / DPorts

The dedicated application build system for DragonFly BSD
Other
89 stars 44 forks source link

mail/spamassassin rc.d script needs command_interpreter=/usr/local/bin/perl #129

Closed pavalos closed 9 years ago

pavalos commented 9 years ago

mail/spamassassin installs an rc.d script (sa-spamd), but restart doesn't work since spamd is actually a perl script. Adding "command_interpreter=/usr/local/bin/perl" to rc.d/sa-spamd makes it work.

jrmarino commented 9 years ago

Thanks, I made the change at the ports level and fixed some other ports with interpreters of /usr/bin/perl at the same time.

jrmarino commented 9 years ago

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think? https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517

pavalos commented 9 years ago

Interesting that behavior on FreeBSD is the opposite (what we are trying to fix) of ours. Without command_interpreter, we can't stop or restart spamd. I'm about to hop on a flight, but I'll try to see where the incompatibility is (gotta be rc.subr) after i land.

On Jan 5, 2015, at 4:25 PM, jrmarino notifications@github.com wrote:

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think? https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517

— Reply to this email directly or view it on GitHub.

pavalos commented 9 years ago

There's no rush. John is going to back out the change in the meantime.

Adam

Adam Weinberger adamw@adamw.org http://www.adamw.org

On 5 Jan, 2015, at 17:39, Peter Avalos peter@theshell.com wrote:

Interesting that behavior on FreeBSD is the opposite (what we are trying to fix) of ours. Without command_interpreter, we can't stop or restart spamd. I'm about to hop on a flight, but I'll try to see where the incompatibility is (gotta be rc.subr) after i land.

On Jan 5, 2015, at 4:25 PM, jrmarino notifications@github.com wrote:

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think? https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517

— Reply to this email directly or view it on GitHub.

jrmarino commented 9 years ago

pavalos, have you had a chance to review both dragonfly and freebsd's handling of command_interpreter? I backed out your changes upon Adam's request, but the idea is that we figure out where the problem is and submit a change request if necessary. It seems dragonfly is using command_interpreter correctly, so it could be a bug in FreeBSD

pavalos commented 9 years ago

On Jan 20, 2015, at 2:21 AM, jrmarino notifications@github.com wrote:

pavalos, have you had a chance to review both dragonfly and freebsd's handling of command_interpreter? I backed out your changes upon Adam's request, but the idea is that we figure out where the problem is and submit a change request if necessary. It seems dragonfly is using command_interpreter correctly, so it could be a bug in FreeBSD

— Reply to this email directly or view it on GitHub https://github.com/DragonFlyBSD/DPorts/issues/129#issuecomment-70632701.

I’m convinced command_interpreter is correct, but I’ve narrowed this down to FreeBSD’s ps behavior.

DragonFly:

ps -o "pid,command" -p 782825

PID COMMAND 782825 perl: /usr/local/bin/spamd -l --socketpath=/var/run/spamd/spamd.sock -d -r /var/run/spamd/spamd.pid (perl)

FreeBSD: PID COMMAND 805 /usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid (perl)

(Note the lack of “perl: “ in FreeBSD’s output) So this is why FreeBSD is failing with command_interpreter set, but I haven’t figured out why FreeBSD does this…

—Peter

jrmarino commented 9 years ago

I thought you were on an email that explained the "perl" gets lost when spamd gets daemonized, at least on FreeBSD

jrmarino commented 9 years ago

(but obviously not)

pavalos commented 9 years ago

Well I just looked at the FreeBSD PR, and there’s some discussion there that I wasn't tracking on…So I agree that /usr/local/bin/spamd rewrites its command title in the daemonize routine on line 2929, but that still doesn’t explain why the interpreter (perl: ) part goes away.

That line in /usr/local/bin/spamd makes it go from:

/usr/local/bin/perl -T -w /usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid

to: /usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid

But note that FreeBSD doesn’t have the interpreter listed (as “perl: “), regardless of that line in spamd. But I don’t know why...=

jrmarino commented 9 years ago

You said that "command interpreter" is working on FreeBSD, but then you describe the "ps" mechanism that prevents it from working due to the behavior of ps.

This is why I was thinking there is actually a bug in FreeBSD. It's not working as advertised. It actually breaks if command_interpreter is specified.

pavalos commented 9 years ago

Looking at it from another point of view…Why does our ps include “perl: “?

Not all daemonized perl programs do that…Look at munin-node on DragonFly: /usr/local/bin/perl -wT /usr/local/sbin/munin-node

—Peter=

jrmarino commented 9 years ago

You tell me. :) All I know is that FreeBSD and DragonFly are not compatible in this aspect and DragonFly is the one that is operating as expected (I think). So we identified the problem but the analysis is ongoing. I'm just standing by for your recommendation and I'll try to help fix the problem based on that.

pavalos commented 9 years ago

After some consultation with Sascha Wildner, he pointed me to this gem in perl’s mg.c:

ifdef HAS_SETPROCTITLE

/\* The BSDs don't show the argv[] in ps(1) output, they
 \* show a string from the process struct and provide
 \* the setproctitle() routine to manipulate that. */
if (PL_origalen != 1) {
    s = SvPV_const(sv, len);

if __FreeBSD_version > 410001

    /* The leading "-" removes the "perl: " prefix,
     * but not the "(perl) suffix from the ps(1)
     * output, because that's what ps(1) shows if the
     * argv[] is modified. */
    setproctitle("-%s", s);

else /* old FreeBSDs, NetBSD, OpenBSD, anyBSD */

    /* This doesn't really work if you assume that
     * $0 = 'foobar'; will wipe out 'perl' from the $0
     * because in ps(1) output the result will be like
     * sprintf("perl: %s (perl)", s)
     * I guess this is a security feature:
     * one (a user process) cannot get rid of the original name.
     * --jhi */
    setproctitle("%s", s);

endif

DragonFly’s setproctitle() has the “-“ capability, so if we were to patch perl, then the ps output will not have perl in it, and command_interpreter will not be necessary. What do you think?

jrmarino commented 9 years ago

pro: it would make us more compatible with ports con 1: It would still break release 4.0 and 3.8 con 2: It would make DFLY different from netbsd and openbsd. Right now freebsd is the odd one out.

requirement if we don't do anything: multiple dragonfly-only patches to "fix" ports.

The answer is not clear cut.

pavalos commented 9 years ago

Well if DragonFly maintains setproctitle’s “-“ capability and perl wants to use it, then we should go in that direction (by patching perl source by adding a || defined(__DragonFly) to that section of mg.c.

As far as handling non-patched versions of perl, then we have command_interpreter…It’s not great, but I’m not sure what else to do…

jrmarino commented 9 years ago

Okay, I patched all 3 version of perl. I assume this resolves the issue, so I'm closing this ticket. Feel free to open it again if I'm wrong about that.