[m-rev.] for review: add configuration option to enable internal file copying

Zoltan Somogyi zoltan.somogyi at runbox.com
Sun Jan 14 15:54:44 AEDT 2024


On 2024-01-14 15:45 +11:00 AEDT, "Julien Fischer" <jfischer at opturion.com> wrote:
> +AC_ARG_WITH([cp],
> +    AS_HELP_STRING([--with-cp],
> +       [Use the cp command for file copying by the Mercury compiler.
> +        The default is system dependent.]),
> +       [with_cp="$withval"],
> +       [with_cp="default"])

I would prefer if the explanation here spelled out which platforms
have which setting as default.

> +INSTALL_METHOD=
> +case "$with_cp" in
> +    yes)
> +        ;;
> +    no)
> +        INSTALL_METHOD="--install-method \"internal\""
> +        ;;
> +    default)
> +        if test "$USING_MICROSOFT_CL_COMPILER" = "yes"
> +        then
> +            INSTALL_METHOD="--install-method \"internal\""
> +        else
> +            case "$host" in
> +                *mingw*)
> +                    INSTALL_METHOD="--install-method \"internal\""
> +                    ;;
> +            esac
> +        fi
> +        ;;
> +    *)
> +        AC_MSG_ERROR([unexpected argument to --with-cp])
> +        ;;
> +
> +esac
> +
> +AC_SUBST(INSTALL_METHOD)

I would s/INSTALL_METHOD/INSTALL_METHOD_OPT/ here,
and below.

I would also explicitly set external as well as internal methods
explicitly, and assign to INSTALL_METHOD_OPT explicitly
on each possible path through the code above, not relying
on the initialization of that variable.

The rest is fine.

Zoltan.


More information about the reviews mailing list