Re: [PATCH 2/4] AC_SITE_LOAD: add OS/2-specific initialization

Eric Blake <[email protected]> Mon, 22 Sep 2014 09:38:51 -0600

Newsgroups	gmane.comp.sysutils.autoconf.patches
Organization	Red Hat, Inc.
Message-ID	<[email protected]>

On 09/22/2014 12:59 AM, KO Myung-Hun wrote:
> \ may be recognized as an escape character on some shells such as
> pdksh. And the executables on OS/2 have .exe as an extension.

Umm, \ is an escape character on ALL sh-related shells.  And .exe
handling on OS/2 should behave as it does on mingw.

> 
> * lib/autoconf/general.m4 (AC_SITE_LOAD): Convert \ in PATH to /.
> Add .exe to ac_executable_extensions.

This says what you changed, but not why.  A good commit message gives
rationale on WHY the change is important, such as a demonstration of
what goes wrong without the patch.

> ---
>  lib/autoconf/general.m4 |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
> index 77f71d2..5a87d5e 100644
> --- a/lib/autoconf/general.m4
> +++ b/lib/autoconf/general.m4
> @@ -1951,6 +1951,34 @@ do
>        || AC_MSG_FAILURE([failed to load site script $ac_site_file])
>    fi
>  done
> +
> +if test -n "$OS2_SHELL"; then
> +  # Backslashes into forward slashes:
> +  # The following OS/2 specific code is performed AFTER config.site
> +  # has been loaded to allow users to change their environment there.
> +  # This strange code is necessary to deal with handling of backslashes by
> +  # ksh.
> +  ac_save_IFS="$IFS"
> +  IFS="\\"
> +  ac_TEMP_PATH=
> +  for ac_dir in $PATH; do
> +    IFS=$ac_save_IFS
> +    if test -z "$ac_TEMP_PATH"; then
> +      ac_TEMP_PATH="$ac_dir"
> +    else
> +      ac_TEMP_PATH="$ac_TEMP_PATH/$ac_dir"
> +    fi
> +  done
> +  export PATH="$ac_TEMP_PATH"
> +  unset ac_TEMP_PATH

It looks like this is an (overly-complex) way of converting all \ in
$PATH into / before proceeding.  But why is it necessary?

> +
> +  # add .exe to ac_executable_extensions
> +  if test -z "$ac_executable_extensions"; then
> +    AC_MSG_WARN([ac_executable_extensions not set, assuming .exe])
> +  fi
> +  ac_executable_extensions="$ac_executable_extensions .exe"
> +  export ac_executable_extensions

Why is the existing code that sets ac_executable_extensions not
sufficient?  And why do you have to export it into the environment of
child processes?  This might be better as two separate patches, since it
is doing two unrelated changes.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc (application/pgp-signature, 539 B)