On 2022/12/09 09:17, Martin Ziemer wrote: > Am Tue, Dec 06, 2022 at 04:27:53PM +0000 schrieb Stuart Henderson: > > On 2022/12/06 16:50, Martin Ziemer wrote: > > > Am Tue, Dec 06, 2022 at 03:42:10PM +0000 schrieb Stuart Henderson: > > > > On 2022/12/06 16:38, Martin Ziemer wrote: > > > > > Am Tue, Dec 06, 2022 at 03:24:51PM +0000 schrieb Stuart Henderson: > > > > > > On 2022/12/06 15:22, Stuart Henderson wrote: > > > > > > > I have left the getmails patch alone for now as I can't test it > > > > > > > but the > > > > > > > pgrep invocation is wrong, it should probably search for > > > > > > > something like > > > > > > > "^/bin/sh /usr/local/bin/getmails$" and then I expect the set -e > > > > > > > will > > > > > > > work. > > > > > > ...or here's an (untested) version with that proposed change. > > > > > > (sorry for the spam!) > > > > > > +-PID_GETMAILS=$(pgrep -U $UID_BY_ID '^getmails$') > > > > > > ++PID_GETMAILS=$(pgrep -U $UID_BY_ID '^/bin/sh > > > > > > /usr/local/bin/getmails$') > > > > > Just tested the getmails change: it still exits at the pgrep line. > > > > Try pgrep -f [...] > > > Does not work either. > > > The problem is a premature end of the whole script, of pgrep finds > > > noting, instead of just filling the variable to empty. > > Have a poke around with pgrep while the script is running and see > > what's needed to get it to match then; in particular this part of the > > diff breaks the whole reason they're using pgrep: > > > > +-if [ "x$PID_GETMAILS" != "x$$" ]; then > > ++if [ "x${PID_GETMAILS}x" != "xx" ]; then > > Today i got my hands on a Debian and a FreeBSD system. > > On Debian getmails is not distributed with the package. If i use > getmails from git there, i get the same error. > > On FreeBSD getmails is distributed unpatched. It shows the same error > (and tries to start getmail from /usr/bin/getmail) > > So i see 3 Options for us: > > 1. We do not distribute the script in the package > 2. We patch getmails in a way like the diff below and install it (This > version i use on my systems) > 3. We ship a patched version as example
or 4. Actually fix it I must say I don't really understand going to the trouble of looking at several OS rather than just figuring out what's needed to fix. > > I tend to say for the moment not shipping it would be the safest way, > until the version in upstream is better. > The original getmail had no script for multiple configuration files, > so we will get no problems with compatibility. > > --- /usr/obj/ports/getmail-6.18.10/getmail6-6.18.10/getmails Sun Sep 18 > 19:56:20 2022 > +++ /usr/local/bin/getmails Fri Dec 9 08:15:44 2022 > @@ -28,8 +28,8 @@ BASE1=${1##*/} > [ "$BASE1" != "${BASE1#$2}" ] && return 0 || return 1 > } > UID_BY_ID=$(id -u) > -PID_GETMAILS=$(pgrep -U $UID_BY_ID '^getmails$') > -if [ "x$PID_GETMAILS" != "x$$" ]; then > +PID_GETMAILS=$(pgrep -fU $UID_BY_ID '/usr/local/bin/getmails' | sed > "s/$$//" | tr -d '\n' ) > +if [ "x${PID_GETMAILS}x" != "xx" ]; then > echo "The getmails script is already running as PID=\"$PID_GETMAILS\" > ." >&2 This patched test is completely broken. It would be better to remove the "are we already running" check completely than patch it in a way that might at first glance look like a fix, but really isn't. Think about what it's doing. pgrep should *always* return at least one running instance here (the one which is currently running), or more than one if another instance is running. Then the 'if [ "x$PID_GETMAILS" != "x$$" ]' is checking whether the string returned from pgrep is equal to the script's current pid. If so, it's ok. If not (because it is "<current pid> <other pid>" then it reports the duplicate. > exit 1 > fi > @@ -44,7 +44,7 @@ if [ -f $getmailrcdir/stop ]; then > echo "Do not run getmail ... (if not, remove $getmailrcdir/stop)" >&2 > exit 1 > fi > -rcfiles="/usr/bin/getmail" > +rcfiles="/usr/local/bin/getmail" > # Address concerns raised by #863856 > # emacs backup files: foo~ foo# > # vim backup files: foo~ foo.swp > @@ -57,7 +57,8 @@ if $para ; then > ! endwith "$file" '#' && \ > ! startswith "$file" 'oldmail-' && \ > ! endwith "$file" '.swp' && \ > - ! endwith "$file" '.bak' ; then > + ! endwith "$file" '.bak' && \ > + [ -f "$file" ]; then > $rcfiles --rcfile "$file" "$@" & > pids="$pids $!" > fi > @@ -79,7 +80,8 @@ else > ! endwith "$file" '#' && \ > ! startswith "$file" 'oldmail-' && \ > ! endwith "$file" '.swp' && \ > - ! endwith "$file" '.bak' ; then > + ! endwith "$file" '.bak' && \ > + [ -f "$file" ]; then > rcfiles="$rcfiles --rcfile \"$file\"" > fi > done