Re: [new port] getmail6

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