[m-rev.] for review: add configuration option to enable internal file copying
Julien Fischer
jfischer at opturion.com
Sun Jan 14 16:04:24 AEDT 2024
On Sun, 14 Jan 2024, Zoltan Somogyi wrote:
> 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.
I have changed it to say that cp is the default except when using MSVC or
MinGW.
>> +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.
Done.
> 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.
Done.
> The rest is fine.
Thanks for the review.
Julien.
>
> Zoltan.
More information about the reviews
mailing list