libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] [cygwin|mingw]: Add cross-compile support to cwrapper (take


From: Ralf Wildenhues
Subject: Re: [PATCH] [cygwin|mingw]: Add cross-compile support to cwrapper (take 6)
Date: Thu, 26 Aug 2010 22:18:43 +0200
User-agent: Mutt/1.5.20 (2010-04-22)

Hi Charles,

first off, a few general comments: thanks for doing this work,
thanks for being so persistent.

Then, please just move the new functions where Peter needs them,
if they really need moving, that is.

Then, since the next few days might see a number of commits: in case
your redone patch does not apply or rebase cleanly against master any
more, you can easily leave that rebasing work to me, or better, just
apply it to the tree you tested against and let's merge that from
master (again, I can do that for you).

* Charles Wilson wrote on Mon, Jul 19, 2010 at 03:07:01AM CEST:
> * libltdl/m4/libtool.m4 (_LT_PATH_CONVERSION_FUNCTIONS): New
> function sets libtool variable $to_host_path_cmd, and employs
> cache. AC_SUBSTs $to_host_path_cmd, as well.
> (_LT_SETUP): Require it.
> * tests/testsuite.at: Ensure to_host_path_cmd is passed as a
> variable setting on the configure line for (new testsuite) tests.
> * Makefile.am: Ensure to_host_path_cmd is included in
> TEST_ENVIRONMENT so that it is passed to (old testsuite) tests.

Typo TESTS_ENVIRONMENT.

> * libltdl/config/ltmain.m4sh (func_cygpath): New function.
> (func_init_to_host_pathlist_cmd): New function.
> (func_to_host_path): Refactored to... (now uses eval
> $to_host_path_cmd).
> (func_wine_to_win32_path): Here. New function.
> (func_msys_to_win32): Here. New function.
> (func_path_convert_check): Here. New function.
> (func_noop_path_convert): Here. New function.
> (func_msys_to_mingw_path_convert): Here. New function.
> (func_cygwin_to_mingw_path_convert): Here. New function.
> (func_nix_to_mingw_path_convert): Here. New function.
> (func_msys_to_cygwin_path_convert): New function.
> (func_nix_to_cygwin_path_convert): New function.
> (func_to_host_pathlist): Refactored to... (now uses eval
> $to_host_pathlist_cmd and func_init_to_host_pathlist_cmd).
> (func_pathlist_convert_check): Here. New function.
> (func_pathlist_front_back_pathsep): Here. New function.
> (func_wine_to_win32_pathlist): Here. New function.
> (func_noop_pathlist_convert): Here. New function.
> (func_msys_to_mingw_pathlist_convert): Here. New function.
> (func_cygwin_to_mingw_pathlist_convert): Here. New function.
> (func_nix_to_mingw_pathlist_convert): Here. New function.
> (func_msys_to_cygwin_pathlist_convert): New function.
> (func_nix_to_cygwin_pathlist_convert): New function.

I'm not sure I've asked before, but will state again: coding up X-to-Y
for N choices of X and M choices of Y has complexity N*M, while coding
it up as from-X and to-Y has complexity N+M.  Quadratic algorithms don't
scale.  Why is the latter not done?  The answer should be in a comment
in the code, if it cannot be done.  If it can be done, then I am OK with
making it a TODO item to be addressed after 2.2.12, rationale being that
that's just an optimization so QoI issue, whereas your patch brings new
features.

[...]
> The original motivation was to enable correct cwrapper *execution*
> when cross-compiling to a cygwin target (from linux, primarily) --
> where the $build machine was (a) x86 (b) and had wine (c) and had
> a cygwin "installation" that could be executed under wine.
[...]

> mingw-native (e.g. msys->mingw)
> ============
> mingw-native (old tests): 2 FAIL, 120 PASS, 2 SKIP
>   mdemo-exec.test (after mdemo-conf)
>   mdemo2-exec.text
>   I don't believe these are related to this patch, because they
>   have been around a long time. I do NOT see these failures in the
>   cygwin->mingw configuration (where the -exec test is, correctly,
>   skipped), nor in the linux->mingw cross configurations (where these
>   two -exec tests are NOT skipped (!!) but do pass [I have binfmt
>   enabled]).

This is interesting.  I don't see these mdemo*exec failures in my
mingw-native setup.  I wonder why that is.  When parallel-tests is
merged, we should look at test-suite.log.

> cygwin->mingw cross (faked)
> ===================
> This is where you do:
>    $ export PATH="/c/MinGW/bin:${PATH}"
>    $ configure --build=i686-pc-cygwin --host=mingw32 \
>       NM=/c/MinGW/bin/nm.exe
> That is, you use cygwin as $build, but use the native MinGW
> compiler (instead of a cygwin-hosted cross compiler).
> 
> I haven't tested this yet (I forgot about it) but I really should.
> It is a common use case.

Yes, this would be interesting to see.

> cygwin->mingw cross (lie)
> ===================
> This is where you do
>    $ configure --build=i686-pc-mingw32 --host=i686-pc-mingw32 \
>       --target=i686-pc-mingw32
> even though you are actually running under *cygwin* and not
> mingw (e.g. msys in it's normal "pretend I'm mingw" personality).

I understand Danny used this a lot, but I think it is broken, and while
it's ok to make it work if that's not complicated, I don't think we
should have to complicate things much for it.

> See:
> http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00193.html

> To make this work, with this patch and when you lie to your build
> system this way, you'll have to do the following:
> 
> cygwin$ export lt_cv_to_host_path_cmd=func_cygwin_to_mingw_path_convert
> cygwin$ path/to/configure \
>       --build=i686-pc-mingw32 \
>       --host=i686-pc-mingw32 \
>       --target=i686-pc-mingw32 \
>       --disable-dependency-tracking
> 
> because if you don't, libtool will (mistakenly) use the
> func_msys_to_mingw_path_convert function, since you did tell it that
> $build was mingw (e.g. msys in it's pretend I'm mingw" personality).
> disable-dependency-tracking is necessary because otherwise MinGW gcc
> will put pure win32 (C:/foo) paths into the .deps/*.P files, and
> cygwin make will be quite confused.

All these limitations seem acceptable to me for this particular case.

> I haven't tested this recently, but I need to. I just forgot.


> linux->mingw cross
> ==================
> linux->mingw (old tests): 2 of 100 FAIL, 6 SKIP
>   FAIL: tests/demo-hardcode.test
>   FAIL: tests/depdemo-relink.test
>   Don't know if these are regressions or not; will recheck without
>   this patch.

Whether or not, it'd be interesting to see verbose logs of these at some
point.

> linux->cygwin cross (LT_CYGPATH not set)
> ===================
> linux->cygwin (old tests): 1 of 114 FAIL, 10 SKIP
>   FAIL: tests/demo-hardcode.test
>   Ditto: don't know if this is a regression or not; will recheck
>   without this patch.

Ditto.

> linux->cygwin (new tests): AWFUL.
>                          23 unexpected failures
>                          32 skip
> I don't expect any difference WITH LT_CYGPATH set, because
> cygpath-1.7 (and, indeed, all cygwin-1.7 programs) seems to crash under
> wine anyway.

OK, so we have to ignore results from this configuration as long as that
issue is not fixed.

> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -528,7 +528,8 @@ TESTS_ENVIRONMENT = MAKE="$(MAKE)" CC="$(CC)" 
> CFLAGS="$(CFLAGS)" \
>       CXX="$(CXX)" CXXFLAGS="$(CXXFLAGS)" CXXCPP="$(CXXCPP)" \
>       F77="$(F77)" FFLAGS="$(FFLAGS)" \
>       FC="$(FC)" FCFLAGS="$(FCFLAGS)" \
> -     GCJ="$(GCJ)" GCJFLAGS="$(GCJFLAGS)"
> +     GCJ="$(GCJ)" GCJFLAGS="$(GCJFLAGS)" \
> +     lt_cv_to_host_path_cmd="$(to_host_path_cmd)"

I don't understand why this change should be necessary.  In your
testing, you describe that most setups set a correct to_host_path_cmd
themselves.  For the other(s), you already describe
  export lt_cv_to_host_path_cmd=override
  ./configure ...
  make all check

That should be sufficient without the above patch, no?

> --- a/libltdl/config/ltmain.m4sh
> +++ b/libltdl/config/ltmain.m4sh
> @@ -2775,166 +2775,595 @@ fi\
>  "
>  }
>  
> +####################################
> +# PATH CONVERSION HELPER FUNCTIONS #
> +####################################

Believe it or not, the GNU Coding Standards ask us to name things "file
name" that which are files, and "path" those which are colon-separated
(or $PATH_SEPARATOR-separated).  I'll close my eyes now on this issue,
because I see hundreds of instances of it in this patch, so that'd be a
TODO item.

> +# func_wine_to_win32_path ARG
> +# Helper function used by path conversion functions
> +# when $build is *nix, and $host is mingw, cygwin,
> +# or some other win32 environment. Relies on a
> +# correctly configured wine environment available,
> +# with the winepath program in $build's $PATH.
> +#
> +# ARG is the $build path to be converted to win32 format.
> +# result is available in $func_wine_to_win32_path_result
> +# result is empty on error (or when arg is empty)
> +func_wine_to_win32_path ()
> +{
> +  $opt_debug
> +  lt_sed_naive_backslashify='s|\\\\*|\\|g;s|/|\\|g;s|\\|\\\\|g'

Might want to move this variable outside to just initialize it once?
It's global anyway.  Maybe even general.m4sh.

> +  func_wine_to_win32_path_result="$1"
> +  if test -n "$1"; then
> +    # Unfortunately, winepath does not exit with a non-zero
> +    # error code, so we are forced to check the contents of
> +    # stdout. On the other hand, if the command is not
> +    # found, the shell will set an exit code of 127 and print
> +    # *an error message* to stdout. So we must check for both
> +    # error code of zero AND non-empty stdout, which explains
> +    # the odd construction:

Peter, are you reading this?  Looks like a TODO item for
automake/lib/compile.  ;-)

> +    func_wine_to_win32_path_tmp=`winepath -w "$1" 2>/dev/null`
> +    if test "$?" -eq 0 && test -n "${func_wine_to_win32_path_tmp}"; then

I'll just note that some shells ((d?)ash 0.2) fail to propagate the exit
status of an assignment.  No need to change the code, but users should
have a decent shell for this.

> +      func_wine_to_win32_path_result=`$ECHO "$func_wine_to_win32_path_tmp" |
> +        $SED -e "$lt_sed_naive_backslashify"`
> +    else
> +      func_wine_to_win32_path_result=

The way this is coded, correctness relies on the fact that all code
paths that invoke this function do eventually check for non-emptiness
of the result.

> +    fi
> +  fi
> +}
> +# end: func_wine_to_win32_path
> +
> +
> +# func_wine_to_win32_pathlist ARG
> +# Helper function used by path conversion functions
> +# when $build is *nix, and $host is mingw, cygwin,
> +# or some other win32 environment. Relies on a
> +# correctly configured wine environment available,
> +# with the winepath program in $build's $PATH.
> +# Assumes ARG has no leading or trailing path separator
> +# characters.
> +#
> +# ARG is pathlist to be converted from $build format to win32.
> +# Result is available in $func_wine_to_win32_pathlist_result
> +# Unconvertible paths in pathlist are skipped; if no paths
> +# are convertible, result may be empty.
> +func_wine_to_win32_pathlist ()
> +{
> +  $opt_debug
> +  # unfortunately, winepath doesn't convert pathlists
> +  func_wine_to_win32_pathlist_result=""
> +  if test -n "$1"; then
> +    func_wine_to_win32_pathlist_oldIFS=$IFS

IFS is saved and restored always to the same value in libtool, AFAIK,
so a short variable name should suffice completely.

> +    IFS=:
> +    for func_wine_to_win32_pathlist_f in $1; do
> +      IFS=$func_wine_to_win32_pathlist_oldIFS
> +      if test -n "$func_wine_to_win32_pathlist_f" ; then

This if is unnecessary, the for list does not expand $1 to empty tokens.

> +        func_wine_to_win32_path "$func_wine_to_win32_pathlist_f"
> +        if test -n "$func_wine_to_win32_path_result" ; then
> +          if test -z "$func_wine_to_win32_pathlist_result"; then
> +            
> func_wine_to_win32_pathlist_result="$func_wine_to_win32_path_result"
> +          else
> +            func_append func_wine_to_win32_pathlist_result 
> ";$func_wine_to_win32_path_result"
> +          fi
> +        fi
> +      fi
> +    done
> +    IFS=$func_wine_to_win32_pathlist_oldIFS
> +  fi
> +}
> +# end: func_wine_to_win32_pathlist
> +
> +
> +# func_cygpath ARGS...
> +# a wrapper around calling the cygpath program via
> +# LT_CYGPATH, when $host is *nix and cygwin is
> +# hosted via a wine environment (or, rarely, when
> +# host is mingw -- that is, msys).
> +#
> +# Result is available in func_cygpath_result, which
> +# may be empty on error. Can accomodate both paths
> +# and pathlists (with appropriate options).
> +#
> +# ARGS are the typical arguments and options for
> +# the cygpath program. Usually, the last argument
> +# is the path or pathlist to be converted.
> +#
> +# The full *nix (or msys) path to the cygpath program must be
> +# specified in the LT_CYGPATH environment variable. This
> +# is because (a) the cygpath program shouldn't be in $PATH,
> +# because it usually lives in cygwin's bin/ directory --
> +# along with *cygwin* versions of sed, id, cp. If the *nix (or
> +# msys) host environment had those programs in its $PATH, many
> +# bad things could happen. (b) especially in cygwin-1.7, multiple
> +# installations (with separate "mount tables" in
> +# <CYGROOT-N>/etc/fstab) can coexist on the same Win32
> +# instance. The cygpath.exe for cygwin installation #N in
> +# <CYGROOT-N>/bin automatically deduces the appropriate
> +# ../etc/fstab file. Therefore, it matters which cygpath.exe
> +# is used. LT_CYGPATH may be replaced or supplemented by an
> +# LT_INIT-activated configure option in the future.

Wow.  That belongs in the documenation, not the code.  The code should
have a concise text, how about:

  # func_cygpath ARGS...
  # Wrapper around calling the cygpath program via LT_CYGPATH, when $host
  # is *nix and Cygwin is hosted via a wine environment or MSYS.
  # Returns the Cygwin file name or path in func_cygpath_result, or an
  # empty string on error.
  #
  # ARGS are passed to cygpath, with the last one being the file name
  # or path to be converted.
  #
  # Specify the absolute *nix (or MSYS) name to cygpath in the LT_CYGPATH
  # environment variable, do not put it in $PATH.

> +func_cygpath ()
> +{
> +  $opt_debug
> +  if test -n "$LT_CYGPATH" && test -f "$LT_CYGPATH"; then
> +    func_cygpath_result=`$LT_CYGPATH "$@" 2>/dev/null`
> +    if test "$?" -ne 0; then
> +      # on failure, ensure result is empty
> +      func_cygpath_result=
> +    fi
> +  else
> +    func_cygpath_result=
> +    func_error "LT_CYGPATH is empty or specifies non-existant file: 
> \`$LT_CYGPATH'"

existent

> +  fi
> +}
> +#end: func_cygpath

> +# func_msys_to_win32 ARG
> +# Converts ARG from msys (unix-ish) format to
> +# win32 format. Can accomodate both paths and pathlists.
> +# Result is available in func_msys_to_win32_result.

How about
# Convert file or path ARG from MSYS format to w32 format.  Return
# result in func_msys_to_win32_result.
?

> +func_msys_to_win32 ()
> +{
> +  $opt_debug
> +  lt_sed_naive_backslashify='s|\\\\*|\\|g;s|/|\\|g;s|\\|\\\\|g'

See above.

> +  # awkward: cmd appends spaces to result
> +  func_msys_to_win32_result=`( cmd //c echo "$1" ) 2>/dev/null |
> +    $SED -e 's/[ ]*$//' -e "$lt_sed_naive_backslashify"`
> +}
> +#end: func_msys_to_win32
> +
> +
> +# func_path_convert_check ARG1 ARG2

> +# Verify that ARG1 (a path in $build format) was
> +# converted to $host format in ARG2. Otherwise, emit
> +# an error message, but continue (resetting
> +# func_to_host_path_result to ARG1).

Please reflow to 80 columns (or 78 or 72, but not 53, please).

> +func_path_convert_check ()
> +{
> +  $opt_debug
> +  if test -z "$2" && test -n "$1" ; then
> +    func_error "Could not determine host path corresponding to"
> +    func_error "  \`$1'"
> +    func_error "Continuing, but uninstalled executables may not work."
> +    # Fallback:
> +    func_to_host_path_result="$1"
> +  fi
> +}
> +# end func_path_convert_check
> +
> +
> +# func_pathlist_convert_check FROM_PATHSEP TO_PATHSEP FROM_PATHLIST 
> TO_PATHLIST
> +# Verify that FROM_PATHLIST (a path in $build format) was converted
> +# $host format in TO_PATHLIST. Otherwise, emit an error message, but
> +# continue, resetting func_to_host_path_result to a simplistic
> +# fallback value (see below).
> +func_pathlist_convert_check ()
> +{
> +  $opt_debug
> +  if test -z "$4" && test -n "$3"; then
> +    func_error "Could not determine the host path(s) corresponding to"
> +    func_error "  \`$3'"
> +    func_error "Continuing, but uninstalled executables may not work."
> +    # Fallback. If even this fallback fails, the fix is not to
> +    # complicate the expression below, but for the user to provide,
> +    # in that situation, whatever elements are missing from the
> +    # environment so that the actual pathlist conversion functions
> +    # work properly (for instance, a working wine installation
> +    # with winepath so that path translation in the cross-to-mingw
> +    # case works).

This belongs in the documentation, no?

> +    if test "x$1" != "x$2"; then
> +      lt_replace_pathsep_chars="s|$1|$2|g"
> +      func_to_host_pathlist_result=`echo "$3" |\

unneeded trailing backslash.

> +        $SED -e "$lt_replace_pathsep_chars"`
> +    else
> +      func_to_host_pathlist_result="$3"
> +    fi
> +  fi
> +}
> +# end func_pathlist_convert_check
> +
> +
> +# func_pathlist_front_back_pathsep FRONTPAT BACKPAT REPL ORIG
> +# Modifies func_to_host_pathlist_result by prepending REPL
> +# if ORIG matches FRONTPAT and appending REPL if ORIG matches
> +# BACKPAT.
> +func_pathlist_front_back_pathsep ()
> +{
> +  $opt_debug
> +  case "$4" in

unneeded quotes

> +  $1 ) func_to_host_pathlist_result="$3$func_to_host_pathlist_result"
> +    ;;
> +  esac
> +  case "$4" in
> +  $2 ) func_append func_to_host_pathlist_result "$3"
> +    ;;
> +  esac
> +}
> +# end func_pathlist_front_back_pathsep
>  
> -# func_to_host_path arg
> +
> +#############################################
> +# $build to $host PATH CONVERSION FUNCTIONS #
> +#############################################
> +# invoked via `eval $to_host_path_cmd ARG'

eval without at least double-quoting following variables is almost
always wrong.


> +# At present, the following path conversions are supported:
>  #    $build          $host
>  #    mingw (msys)    mingw  [e.g. native]
>  #    cygwin          mingw
>  #    *nix + wine     mingw
> +#    mingw (msys)    cygwin [*] [**]
> +#    *nix + wine     cygwin [**]

Again, all of this belongs in documentation.  But I don't want to get
too picky now, if it helps you developing then ok.

> +# [*] available, but not officially supported. See comments with
> +#     func_msys_to_cygwin_path_convert.
> +# [**] requires environment variable $LT_CYGPATH. See comments
> +#      with func_cygpath.
> +# In each case, ARG is the path to be converted from $build
> +# to $host format. the result will be available in
> +# $func_to_host_path_result.


> +# func_to_host_path ARG

> +# converts the path ARG from $build format to $host
> +# format.

Please reflow.

>  func_to_host_path ()
>  {
> +  $opt_debug
> +  eval '$to_host_path_cmd "$1"'
> +}
> +# end func_to_host_path
> +
> +
> +# func_noop_path_convert ARG
> +# A no-op path conversion function for use when $build == $host.

s/==/=/  for shell syntax.

> +# or when there is no required (or known) conversion function
> +# between $build and $host.

Or even better:
  # func_noop_path_convert ARG
  # Copy ARG to func_to_host_path_result.

> +func_noop_path_convert ()
> +{
> +  $opt_debug

You really want to turn on debugging here?  ;-)

> +  func_to_host_path_result="$1"
> +}
> +# end func_noop_path_convert

> +# func_msys_to_mingw_path_convert ARG
> +# A path conversion function for use with "native" mingw
> +# builds -- that is, when $host is *mingw*, and $build
> +# is *mingw* (which is to say, msys).

Please, globally s/(which is to say, msys)//.  IMVHO this will just
confuse any user reading it besides Peter and you.  ;-)

>  In this case, the
> +# msys shell automatically converts paths for any non-msys
> +# applications it launches, but that facility isn't available
> +# from inside the cwrapper.

No need to describe what this function does not (and need not) do.

One reason I'm harping so much on all the comments is that, if they were
concise, one could just see the actual code within one screenful and
*see* what is different and immediately be able to guess why.

> +# ARG is the path to be converted; the result is available
> +# in func_to_host_path_result.
> +func_msys_to_mingw_path_convert ()
> +{
> +  $opt_debug
>    func_to_host_path_result="$1"
>    if test -n "$1"; then

> +    func_msys_to_win32 "$1"
> +    func_to_host_path_result="$func_msys_to_win32_result"
>    fi
> +  func_path_convert_check "$1" "$func_to_host_path_result"
>  }
> -# end: func_to_host_path
> +# end func_msys_to_mingw_path_convert
>  
> -# func_to_host_pathlist arg
> +
> +# func_cygwin_to_mingw_path_convert ARG
> +# A path conversion function for use when $host is *mingw*
> +# but $build is *cygwin*.

This is enough.

> In this case, the cygpath program
> +# provided by the $build environment is sufficient for all
> +# conversions.

This is not needed.

> +# ARG is the path to be converted; the result is available
> +# in func_to_host_path_result.
> +func_cygwin_to_mingw_path_convert ()
> +{
> +  $opt_debug
> +  func_to_host_path_result="$1"
> +  if test -n "$1"; then
> +    # because $build is cygwin, we call "the" cygpath
> +    # in $PATH; no need to use LT_CYGPATH in this case.
> +    func_to_host_path_result=`cygpath -m "$1"`
> +  fi
> +  func_path_convert_check "$1" "$func_to_host_path_result"
> +}
> +# end func_cygwin_to_mingw_path_convert

[...]

> +# func_msys_to_cygwin_path_convert ARG
> +# A path conversion function for use when $host is *cygwin*
> +# but $build is *mingw* (that is, msys). This implies running
> +# a cross build from msys to cygwin -- but msys has notorious
> +# problems executing cygwin apps, because of conflicts between
> +# cygwin1.dll and msys-1.0.dll. However, we'll try it. First,
> +# convert from msys to win32, then use func_cygpath to convert
> +# from win32 to cygwin. Requires LT_CYGPATH.

I suggest:

  # Convert from MSYS to Cygwin.  Requires LT_CYGPATH set.

But who needs or wants this?  You didn't mention it in your testing
AFAICS, and I don't understand why you wouldn't just use Cygwin if you
wanted to have it anyway.  I'd just drop it, together with the
associated case below.

> +# ARG is the path to be converted; the result is available
> +# in func_to_host_path_result.
> +func_msys_to_cygwin_path_convert ()
> +{
> +  $opt_debug
> +  func_to_host_path_result="$1"
> +  if test -n "$1"; then
> +    func_msys_to_win32 "$1"
> +    func_cygpath -u "$func_msys_to_win32_result"
> +    func_to_host_path_result="$func_cygpath_result"
> +  fi
> +  func_path_convert_check "$1" "$func_to_host_path_result"
> +}
> +# end func_msys_to_cygwin_path_convert
> +
> +# func_nix_to_cygwin_path_convert ARG
> +# A path conversion function for use when $host is *cygwin*
> +# but $build is some *nix variant. In this case, we assume
> +# that a wine environment with a working winepath executable
> +# is available in $build's $PATH, and that cygwin is installed
> +# within that wine environment. Requires LT_CYGPATH (see
> +# func_cygpath).

  # Convert file name ARG from *nix to Cygwin.  Requires Cygwin
  # installed in a wine environment, working winepath, LT_CYGPATH.

> +# ARG is the path to be converted; the result is available
> +# in func_to_host_path_result.
> +func_nix_to_cygwin_path_convert ()
> +{
> +  $opt_debug
> +  func_to_host_path_result="$1"
> +  if test -n "$1"; then
> +    # convert from *nix to win32, then use cygpath to
> +    # convert from win32 to cygwin.
> +    func_wine_to_win32_path "$1"
> +    func_cygpath -u "$func_wine_to_win32_path_result"
> +    func_to_host_path_result="$func_cygpath_result"
> +  fi
> +  func_path_convert_check "$1" "$func_to_host_path_result"
> +}
> +# end func_nix_to_cygwin_path_convert

> +#################################################
> +# $build to $host PATHLIST CONVERSION FUNCTIONS #
> +#################################################
> +# invoked via `eval $to_host_pathlist_cmd ARG'

All comments above apply to this section as well.  Below I'm only
annotating diffs.

> +# ASSUMPTIONS: all such conversion functions are
> +# named using the following convention:
> +#   path conversion function    : xxxxxx_path_convert ()
> +#   pathlist conversion function: xxxxxx_pathlist_convert ()
> +# where, for any given $build/$host combination the 'xxxxxx'
> +# value is the same.

What do you mean with ASSUMPTIONS?  Either the code does that, or it
doesn't.  The comment is either tautological with the code (and thus can
be removed), or it is wrong, in which case the code should be fixed.
Thanks.

> +to_host_pathlist_cmd=
> +func_init_to_host_pathlist_cmd ()
> +{
> +  $opt_debug
> +  if test -z "$to_host_pathlist_cmd"; then
> +    func_stripname '' '_path_convert' "$to_host_path_cmd"
> +    to_host_pathlist_cmd="${func_stripname_result}_pathlist_convert"
> +  fi
> +}
> +
> +
> +# func_to_host_pathlist ARG
> +# converts the pathlist ARG from $build format to $host
> +# format.
>  func_to_host_pathlist ()
>  {
> +  $opt_debug
> +  func_init_to_host_pathlist_cmd
> +  eval '$to_host_pathlist_cmd "$1"'

This should work:
  $to_host_pathlist_cmd "$1"

> +}
> +# end func_to_host_pathlist

> +# func_noop_pathlist_convert ARG
> +# A no-op pathlist conversion function for use when $build == $host,
> +# or when there is no required (or known) conversion function
> +# between $build and $host.

See above (other noop function).

> +func_noop_pathlist_convert ()
> +{
> +  $opt_debug
> +  func_to_host_pathlist_result="$1"
> +}
> +# end func_noop_pathlist_convert

> +func_msys_to_mingw_pathlist_convert ()
> +{
[...]

> +    # Remove leading and trailing path separator characters from
> +    # ARG. msys behavior is inconsistent here, cygpath turns them
> +    # into '.;' and ';.', and winepath ignores them completely.

This comment is good, because it describes unusual behavior!

> +    func_stripname : : "$1"
> +    func_to_host_pathlist_tmp1=$func_stripname_result
> +    func_msys_to_win32 "$func_to_host_pathlist_tmp1"
> +    func_to_host_pathlist_result="$func_msys_to_win32_result"
> +    func_pathlist_convert_check ":" ";" \
> +      "$func_to_host_pathlist_tmp1" "$func_to_host_pathlist_result"
> +    func_pathlist_front_back_pathsep ":*" "*:" ";" "$1"
> +  fi
> +}
> +# end func_msys_to_mingw_pathlist_convert

> +# func_cygwin_to_mingw_pathlist_convert ARG
> +# A pathlist conversion function for use when $host is *mingw*
> +# but $build is *cygwin*. In this case, the cygpath program
> +# provided by the $build environment is sufficient for all
> +# conversions.

The second sentence is not needed.

> +# ARG is the pathlist to be converted; the result is available
> +# in func_to_host_pathlist_result.
> +func_cygwin_to_mingw_pathlist_convert ()
> +{
> +  $opt_debug
> +  func_to_host_pathlist_result="$1"
> +  if test -n "$1"; then
> +    # Remove leading and trailing path separator characters from
> +    # ARG. msys behavior is inconsistent here, cygpath turns them
> +    # into '.;' and ';.', and winepath ignores them completely.

This comment can just be
       # See func_msys_to_mingw_pathlist_convert.

Repetition always leads to maintenance problems.

> +    func_stripname : : "$1"
> +    func_to_host_pathlist_tmp1=$func_stripname_result
> +    func_to_host_pathlist_result=`cygpath -m -p 
> "$func_to_host_pathlist_tmp1"`
> +    func_pathlist_convert_check ":" ";" \
> +      "$func_to_host_pathlist_tmp1" "$func_to_host_pathlist_result"
> +    func_pathlist_front_back_pathsep ":*" "*:" ";" "$1"
>    fi
>  }
> -# end: func_to_host_pathlist
> +# end func_cygwin_to_mingw_pathlist_convert
> +
> +
> +# func_nix_to_mingw_pathlist_convert ARG
> +# A pathlist conversion function for use when $host is *mingw*
> +# but $build is some *nix variant. In this case, we assume
> +# that a wine environment with a working winepath executable
> +# is available in $build's $PATH.

see above.

> +# ARG is the pathlist to be converted; the result is available
> +# in func_to_host_pathlist_result.
> +func_nix_to_mingw_pathlist_convert ()
> +{
> +  $opt_debug
> +  func_to_host_pathlist_result="$1"
> +  if test -n "$1"; then
> +    # Remove leading and trailing path separator characters from
> +    # ARG. msys behavior is inconsistent here, cygpath turns them
> +    # into '.;' and ';.', and winepath ignores them completely.

see above.

> +    func_stripname : : "$1"
> +    func_to_host_pathlist_tmp1=$func_stripname_result
> +    func_wine_to_win32_pathlist "$func_to_host_pathlist_tmp1"
> +    func_to_host_pathlist_result="$func_wine_to_win32_pathlist_result"
> +    func_pathlist_convert_check ":" ";" \
> +      "$func_to_host_pathlist_tmp1" "$func_to_host_pathlist_result"
> +    func_pathlist_front_back_pathsep ":*" "*:" ";" "$1"
> +  fi
> +}
> +# end func_nix_to_mingw_pathlist_convert

> +# func_msys_to_cygwin_pathlist_convert ARG
> +# A pathlist conversion function for use when $host is *cygwin*
> +# but $build is *mingw* (that is, msys). This implies running
> +# a cross build from msys to cygwin -- but msys has notorious
> +# problems executing cygwin apps, because of conflicts between
> +# cygwin1.dll and msys-1.0.dll. However, we'll try it. First,
> +# convert from msys to win32, then use func_cygpath to convert
> +# from win32 to cygwin. Requires LT_CYGPATH.

See above and elsewhere above.

> +# ARG is the pathlist to be converted; the result is available
> +# in func_to_host_pathlist_result.
> +func_msys_to_cygwin_pathlist_convert ()
> +{
> +  $opt_debug
> +  func_to_host_pathlist_result="$1"
> +  if test -n "$1"; then
> +    # Remove leading and trailing path separator characters from
> +    # ARG. msys behavior is inconsistent here, cygpath turns them
> +    # into '.;' and ';.', and winepath ignores them completely.

See above.

> +    func_stripname : : "$1"
> +    func_to_host_pathlist_tmp1=$func_stripname_result
> +    func_msys_to_win32 "$func_to_host_pathlist_tmp1"
> +    func_cygpath -u -p "$func_msys_to_win32_result"
> +    func_to_host_pathlist_result="$func_cygpath_result"
> +    func_pathlist_convert_check ":" ":" \

No need to quote :

> +      "$func_to_host_pathlist_tmp1" "$func_to_host_pathlist_result"
> +    func_pathlist_front_back_pathsep ":*" "*:" ":" "$1"
> +  fi
> +}
> +# end func_msys_to_cygwin_pathlist_convert
> +
> +
> +# func_nix_to_cygwin_pathlist_convert ARG
> +# A pathlist conversion function for use when $host is *cygwin*
> +# but $build is some *nix variant. In this case, we assume
> +# that a wine environment with a working winepath executable
> +# is available in $build's $PATH, and that cygwin is installed
> +# within that wine environment. Requires LT_CYGPATH (see
> +# func_cygpath).
> +#
> +# ARG is the pathlist to be converted; the result is available
> +# in func_to_host_pathlist_result.
> +func_nix_to_cygwin_pathlist_convert ()
> +{
> +  $opt_debug
> +  func_to_host_pathlist_result="$1"
> +  if test -n "$1"; then
> +    # Remove leading and trailing path separator characters from
> +    # ARG. msys behavior is inconsistent here, cygpath turns them
> +    # into '.;' and ';.', and winepath ignores them completely.

See above

> +    func_stripname : : "$1"
> +    func_to_host_pathlist_tmp1=$func_stripname_result
> +    func_wine_to_win32_pathlist "$func_to_host_pathlist_tmp1"
> +    func_cygpath -u -p "$func_wine_to_win32_pathlist_result"
> +    func_to_host_pathlist_result="$func_cygpath_result"
> +    func_pathlist_convert_check ":" ":" \

No need to quote :

> +      "$func_to_host_pathlist_tmp1" "$func_to_host_pathlist_result"
> +    func_pathlist_front_back_pathsep ":*" "*:" ":" "$1"
> +  fi
> +}
> +# end func_nix_to_cygwin_pathlist_convert

I still fail to understand why the file name and path variants of the
functions cannot be collapsed pairwise.  Oh well.

> --- a/libltdl/m4/libtool.m4
> +++ b/libltdl/m4/libtool.m4

> +# _LT_PATH_CONVERSION_FUNCTIONS
> +# -----------------------------
> +# Determine which path conversion functions should be
> +# used by func_to_host_path (and, implicitly, by
> +# func_to_host_pathlist).  These are needed for certain
> +# cross-compile configurations and "native" mingw (which
> +# is actually an msys->mingw cross).

Please reflow.  Please don't explain *here* what "native" MinGW is,
define it once in the manual instead.

> +m4_defun([_LT_PATH_CONVERSION_FUNCTIONS],
> +[AC_REQUIRE([AC_CANONICAL_HOST])dnl
> +AC_REQUIRE([AC_CANONICAL_BUILD])dnl
> +AC_MSG_CHECKING([how to convert $build paths to $host format])
> +AC_CACHE_VAL(lt_cv_to_host_path_cmd,
> +[case $host in
> +  *mingw* )

The usual matching is either
  case $host_os in
    mingw*) ... ;;
    cygwin*) ... ;;

or
  case $host in
    *-*-mingw*) ... ;;
    *-*-cygwin*) ... ;;

so that you actually match the OS part of the triple.  Several
instances.

> +    case $build in
> +      *mingw* ) # actually msys
> +        lt_cv_to_host_path_cmd=func_msys_to_mingw_path_convert
> +        ;;
> +      *cygwin* )
> +        lt_cv_to_host_path_cmd=func_cygwin_to_mingw_path_convert
> +        ;;
> +      * ) # otherwise, assume *nix
> +        lt_cv_to_host_path_cmd=func_nix_to_mingw_path_convert
> +        ;;
> +    esac
> +    ;;
> +  *cygwin* )
> +    case $build in
> +      *mingw* ) # actually msys
> +        lt_cv_to_host_path_cmd=func_msys_to_cygwin_path_convert
> +        ;;
> +      *cygwin* )
> +        lt_cv_to_host_path_cmd=func_noop_path_convert
> +        ;;
> +      * ) # otherwise, assume *nix
> +        lt_cv_to_host_path_cmd=func_nix_to_cygwin_path_convert
> +        ;;
> +    esac
> +    ;;
> +  * ) # unhandled hosts (and "normal" native builds)
> +    lt_cv_to_host_path_cmd=func_noop_path_convert
> +    ;;
> +esac
> +])
> +to_host_path_cmd=$lt_cv_to_host_path_cmd
> +AC_MSG_RESULT([$lt_cv_to_host_path_cmd])
> +_LT_DECL([to_host_path_cmd], [lt_cv_to_host_path_cmd],
> +         [0], [convert $build paths to $host format])dnl
> +AC_SUBST([to_host_path_cmd])dnl

This AC_SUBST seems wrong.  We don't want
  to_host_path_cmd = func_nix_to_cygwin_path_convert

to appear in user packages' Makefile files.  You want the AC_SUBST
in Libtool toplevel's configure.ac instead, so that it appears in
Libtool's toplevel Makefile.

> +])# _LT_PATH_CONVERSION_FUNCTIONS

> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -37,7 +37,7 @@ for tool in ACLOCAL AUTOHEADER AUTOCONF AUTOMAKE 
> AUTORECONF; do
>  done
>  export ACLOCAL AUTOHEADER AUTOCONF AUTOMAKE AUTORECONF
>  eval `$LIBTOOL --config | grep '^EGREP='`
> -eval `$LIBTOOL --config | $EGREP 
> '^(host|host_os|host_alias|build|build_alias)='`
> +eval `$LIBTOOL --config | $EGREP 
> '^(host|host_os|host_alias|build|build_alias|to_host_path_cmd)='`
>  configure_options=--prefix=/nonexistent
>  if test -n "$host_alias"; then
>    configure_options="$configure_options --host $host_alias"
> @@ -45,6 +45,9 @@ fi
>  if test -n "$build_alias"; then
>    configure_options="$configure_options --build $build_alias"
>  fi
> +if test -n "$to_host_path_cmd"; then
> +  configure_options="$configure_options 
> lt_cv_to_host_path_cmd=$to_host_path_cmd"
> +fi
>  if (FOO=bar; unset FOO) >/dev/null 2>&1; then
>    unset=unset
>  else

I don't see why this change should be necessary (see my other comment
regarding 'export lt_cv_to_host_path_cmd' above).

I haven't tested the patch.  I suppose it's more productive to test the
end result.

To let you not feel that I am making this another neverending story:
please, after revising, if you don't have open questions, do commit the
patch, judging yourself whether it is suitable for master yet or, if you
deem it not, just push it to some new branch, let's call it
file-conversion for lack of a better name.

Thanks,
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]