Re: [Bug-mcron] [PATCH] configure: Add '--with-sendmail' for (mcron config config-sendmail)

[Mathieu Lirzin]

Hello 宋文武, 

address@hidden (宋文武) writes:

> this patch add '--with-sendmail' option to the configure script.
>
> With it, I can set the sendmail command (config-sendmail) directly.
>
> Without it, the configure script fails to get a working sendmail command
> due to missing 'ac_cv_prog_WHICH'...

Indeed there is an issue here.

> From f59066b5cc474fa5d531a1ee44baa46b728f4fbd Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <address@hidden>
> Date: Mon, 1 Oct 2018 14:07:48 +0800
> Subject: [PATCH] configure: Add '--with-sendmail' for (mcron config
>  config-sendmail).
>
> * build-aux/guix.scm: Remove 'which' from native-inputs of the package.
> * configure.ac: Add '--with-sendmail' option to set the command of sendmail
> directly.
> * src/mcron/redirect.scm (with-mail-out): Don't add USER to the command of
> sendmail.

I am not familiar with sendmail, so can you explain why USER has been
removed from ‘with-mail-out’?

Additionally an entry in the NEWS files announcing the new configuration
option and how/why to use it would be welcome.

> ---
>  build-aux/guix.scm     |  3 +--
>  configure.ac           | 29 +++++++++--------------------
>  src/mcron/redirect.scm |  5 +----
>  3 files changed, 11 insertions(+), 26 deletions(-)
[...]
> diff --git a/configure.ac b/configure.ac
> index 0bb9262..e8a135a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -55,26 +55,15 @@ GUILE_PROGS
>  
>  AM_MISSING_PROG(HELP2MAN, help2man, $missing_dir)
>  
> -# Now find a sendmail or equivalent.
> -
> -AC_CHECK_PROGS(SENDMAIL, sendmail)
> -if test "x$ac_cv_prog_SENDMAIL" != "x"; then
> -   AC_MSG_CHECKING(sendmail path and arguments)
> -   ac_cv_prog_SENDMAIL="`$ac_cv_prog_WHICH sendmail` -FCronDaemon -odi -oem "
> -dnl  -or0s"
> -   AC_MSG_RESULT($ac_cv_prog_SENDMAIL)
> -
> -else
> -   AC_CHECK_PROGS(SENDMAIL, mail)
> -   if test "x$ac_cv_prog_SENDMAIL" != "x"; then
> -      AC_MSG_CHECKING(mail path)
> -      ac_cv_prog_SENDMAIL="`$ac_cv_prog_WHICH mail` -d "
> -      AC_MSG_RESULT($ac_cv_prog_SENDMAIL)
> -   else
> -      AC_MSG_RESULT(No mail program found)
> -   fi
> -fi
> -SENDMAIL=$ac_cv_prog_SENDMAIL
> +AC_MSG_CHECKING([sendmail command])
> +AC_ARG_WITH(sendmail,
> +            AC_HELP_STRING([--with-sendmail=COMMAND],
> +                           [command to read an email message from
> +                           standard input, and send it]),
> +              SENDMAIL=$withval,
> +              SENDMAIL=["sendmail -t"])

I guess you choose to remove the ‘AC_CHECK_PROGS’ to make ‘sendmail’ a
loose dependency, meaning forcing ‘mcron’ to look at runtime in the PATH
instead of having absolute file name set at configure time.  Am I
correct?  If that's the case I think a comment regarding that fact would
help.

Wouldn't the ‘-t’ option better located on the caller side (meaning in
“src/mcron/redirect.scm”)?

Nitpick: ‘AC_HELP_STRING’ is an obsolete macro which has been
superseeded by ‘AS_HELP_STRING’.

Thank you for your patch.  Sorry for the delay.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37