Re: [PATCH] [cygwin]: Add cross-compile support to cwrapper

[Charles Wilson]

Peter Rosin wrote:
> It's been cooking in my head for a couple of days now, and I think it's
> time to throw out the old response I was writing and start over...
> 
> There are two "new" scenarios that I see (in addition to normal native
> builds and normal crosses).
> 
> (1) $host=$build, but scripts are not running on $build (lying)
> (2) $host!=$build, but we are able the execute $host code.
> 
> Your patch is about making the cwrapper work. However, in order to
> fully support (1), more changes are needed. E.g. When you feed paths
> to the linker in a command file, MSYS does not get to see those
> files, so you have to make the conversion manually.

Well, sorta. This is only "lying" if you consider MSYS to be a habitual
liar.  Otherwise, this is "normal".  However, your argument remains
unchanged for both that case, and for the *really* lying case, where
$host is cygwin but you claim it is mingw/msys. Either way, we (libtool)
needs to know about it (somehow), and do (something) about it, in
several different contexts.

> This conversion will be the same as the one needed for cwrapper but
> still different. However, the older MSVC support had a similar
> concept in the "fix_srcfile_path", but that is very limited in
> scope compared to this patch. Further down the road (as evident by
> the patches in the "Status of the MSVC/MSYS thread") I would like
> to reuse your conversion functions for all path conversion needs.
> I.e. add func_to_tool_path, that translates paths for easy consumption
> by the toolchain, instead of for the cwrapper which is the target of
> func_to_host_path.

I'm not sure how the cwrapper is the "target" of the func_to_host_path.
That function (in whatever underlying implementation) converts one path
from $build to $host format, and puts the answer into a result variable.
How it is used, or who calls it, is up to the caller.

You're just trying to draw a distinction between the needs of the
compiler itself (which is running on $build), the needs of the $host
environment, and the capabilities of the $build environment.

The present patch only draws a distinction between the needs of the
$host environment and the capabilities of the $build environment (as
does this existing func_to_host_path() code in ToT).  Without additional
changes elsewhere such as those you propose, the net effect is an
implicit assumption that the needs of the compiler itself (running on
$build) are the same as the capabilities of $build.

For MSVC-on-cygwin, MSVC-on-msys, and even mingw/gcc-on-msys, that
assumption is not entirely accurate in all cases. But it is a
pre-existing (dubious) assumption, not a brand new one introduced by you
or me.

> When on MSYS ($host=mingw, $build=mingw, but scripts running on
> MSYS), set both func_to_tool_path and func_to_host_path to
> func_msys_to_mingw_path_convert. I.e. scenario (1).

Right...

> When on Cygwin with a cross compiler ($host=mingw, $build=cygwin,
> scripts running on Cygwin and it's possible to execute the output),
> set func_to_tool_path to func_noop_path_convert and func_to_host_path
> to func_msys_to_mingw_path_convert. I.e. scenario (2).

I think you mean func_cygwin_to_mingw_path_convert here.

And if you're REALLY lying ($build is cygwin but you claim
--build=mingw), you explicitly set
 lt_cv_to_host_path_cmd=func_cygwin_to_mingw_path_convert
 lt_cv_to_tool_path_cmd=func_cygwin_to_mingw_path_convert

Note that this indirection is not possible -- at least, not with some
additional fiddling -- with the "extra credit" approach described below.

> Ultimately I would like to eliminate fix_srcfile_path, but
> fix_srcfile_path is used in func_mode_compile. So, I would like to
> move these functions earlier in the script right from the start. But
> that will perhaps come with a performance penalty, so perhaps the
> list oriented function should remain further down? I don't know
> what's best really, I'm just pointing out a future need.

I think we can wait for a simple mechanical patch to move functions
around in the file until we actually need to do so.  But in principle, I
agree: consolidation of similar code to one place improves
maintainability. So func_mode_compile should eventually use these new
functions (indirected thru $to_tool_path, etc).

And we should then do some timing analysis on a platform unaffected by
the need to move those functions earlier -- say native cygwin -- or
linux -- which wouldn't care about fix_src_file_path (or its
replacement). Before, after, how long does compiling 100 .c's and
linking them take?

For extra credit, suppose someone dreams up a mechanism whereby we can
emit custom functions into the libtool script *at particular locations*
(e.g. "before func_mode_compile" or "before func_mode_link") via .m4.
For instance, by using different markers in ltmain.m4sh: currently the
XSI_SHELL_FNS are inserted where 'Generated shell functions inserted
here' appears in ltmain.m4sh.  What if there were other markers, like
'Generated shell functions needed by func_mode_compile inserted here'
and 'Generated shell functions needed by func_mode_link inserted here'?
This would complicate _LT_CONFIG(TAG) [C], but isn't that what m4 is for?

Anyway, then we might not have to worry that the path conversion
functions are "early" -- because only the ones our particular
configuration needs will actually be there, and not all the rest of the
conversion functions.  And they will be located exactly as early in the
libtool script as they must be, and no earlier.

However, care must be taken here to allow some mechanism for
knowledgable users to explicitly control which function bodies are
emitted, so that those who "lie" about $build can still operate as they
are used to doing. function/variable indirection won't be possible.

But, this extra credit approach is an optimization, and this "get the
dadgum paths correct" support is still in its early stages.  You know
what Donald Knuth says about premature optimization.

> Also, I don't think the patch forwards the new cache variables to
> configure runs in the testsuite, isn't that necessary? But now I
> looked back and saw that you exported them before you invoked
> configure. I guess that works, but it's not as stable as adding
> them to $configure_options at the top of tests/testsuite.at.
> I really like to be able to add them as arguments to configure
> and have them remembered for me for future config.status --recheck
> invocations.

Okay, I can do that easily.

> Oh, and one more thing, can the to_host_pathlist_cmd variable be
> eliminated, and the list conversion somehow abstracted to just
> use whatever to_host_path_cmd is currently active? That would be
> a nice improvement, methinks. But perhaps it's hard to optimize
> out looping in some of the cases?

That's the problem. In general, converting pathlists is a strikingly
different operation than converting a single path:
  (1) analyze leading and trailing pathseps (which differ depending on
$build), and make a modified copy of input arg as appropriate.
  (2) variant: use a utility program (or sometimes two) that is/are
capable of operating on an entire pathlist, OR split the input arg on
build-pathsep and loop, delegating to to_host_path_cmd.  For each return
value, concatenate using host-pathsep.  However, as a (slight)
optimization in this latter case, call directly the underlying
path_convert impl for your $build/$host combination, thus avoiding two
unneccesary levels of function calls and an eval.
  (3) analyze result; maybe implement fallback conversion
  (4) modify result by adding back leading/trailing pathseps in $host
format, if original had them in $build format

In three of the four steps, you behave differently based on $build/$host
(is build-pathsep/host-pathsep ;/:, :/;, :/: ? How to handle in each
case?) And, even step #3 is different when $host is mingw, because we
have a dirt-stupid fallback 's/:/;/g' that doesn't apply for other
$hosts.  Well, MAYBE you could do
   sed -e "s/${build_pathsep}/${host_pathsep}/g"
which would be a useless fork for all but *-to-mingw. Perhaps:
  if "x$build_pathsep" != "x$host_pathsep"; then
    result=`echo "$input" |\
      sed -e "s/${build_pathsep}/${host_pathsep}/g"`
  else
    result="$input"
  fi
But this presupposes that we have libtool variables $build_pathsep and
$host_pathsep (and, possibly, $tool_pathsep?) I'm not doing a good job
of eliminating libtool variables, here, am I?

But, continuing on in that vein: you *might* be able to abstract it by
  (1) make build-pathsep and host-pathsep (+ tool-pathsep?) into libtool
variables
  (2) for each step of the pathlist conversion algorithm, modify
appropriately to use those vars instead of literals

However, as currently implemented, the "top level" implementations are
all pretty simple
  (1) func_stripname : : "$1" && tmpvar=$func_stripname_result

  (2) NOT abstracted: call a specific impl function which
      : does cygpath -p for paths instead of regular cygpath
      : msys_to_mingw transparently operates on both paths and pathlists
      : nix_to_cygwin needs to do winepath pathlist (which
          manually loops), followed by cygpath -p
      : etc.
      All of these are encapsulated into a single function call (or
      possibly two). They could be made into libtool variables
      themselves, but that doesn't help eliminate
      to_host_pathlist_cmd -- it only adds MORE!
      Basically, the problem here is we do NOT want to always
        split-on-dirsep and call func_to_host_path

  (3) NOT currently abstracted - but see code snippet above

  (4) func_pathlist_front_back_pathsep ":*" "*:" ";" "$1"

Thus, each _pathlist_convert() function itself already represents, at
the coarsest level, exactly how the overall algorithm differs for the
various $build/$host combinations. I don't think it is much of a win to
add even more libtool variables, and indirected code:
   func_pathlist_front_back_pathsep "${build_pathsep}*"
      "*${build_pathsep}" "${host_pathsep}" "$1"
just to eliminate one libtool variable.  We'd also have to be careful to
pass the from_pathsep and to_pathsep variables as arguments to all
helper functions, so that they could be used in both build_to_host and
build_to_tool scenarios.

===============
I guess I can see some consolidation possible:
  (1) make all "low-level" helper functions accept
      from_pathsep and to_pathsep as fn args, like
      func_pathlist_front_back_pathsep already does
  (2) Modify func_pathlist_convert_check with the
      snippet above, thus eliminating func_pathlist_convert_check_mingw.
  (3) Modify the toplevel functions to always pass explicit
      ':'/';' characters to those helper functions, at least for now.
But this really doesn't help simplify step #2 of the pathlist conversion
algorithm, or eliminate the to_host_pathlist_cmd libtool variable. But
the following does, at the cost of requiring a specific naming
convention for the conversion functions:

to_host_pathlist_cmd=
func_init_to_host_pathlist_cmd ()
{
  $opt_debug
  if test -z "$to_host_pathlist_cmd"; then
    # with appropriate modifications if we ever decide to
    # allow to_host_path_cmd to be empty
    func_stripname '' '_path_convert' "$to_host_path_cmd"
    to_host_pathlist_cmd="${func_stripname_result}_pathlist_convert"
  fi
}

func_to_host_pathlist ()
{
  $opt_debug
  func_init_to_host_pathlist_cmd
  # ditto mods if we need to guard against empty
  eval '$to_host_pathlist_cmd "$1"'
}

Later, if we find we MUST violate the naming convention for some reason,
we could special case for that inside the if statement, by switching on
the value of $to_host_path_cmd (NOT $build or $host!).

How does that sound, as an intermediate plan?
===============

Now, if somebody attacked the extra credit problem, then maybe all of
this concern would be moot. Rather than have

... definitions of all helper functions ...
... needed by any of the fns below ...

... definitions of all possible build_to_host_path_convert fns ...

func_to_host_path()
{
  ... indirect thru libtool variable ...
}

You'd have

... definitions of only those helper fns specifically ...
... needed by the particular implementation below ...

func_to_host_path()
{
   ... the actual implementation you really need ...
   ... for this build/host combination ...
}

and no indirection variables at all.  But I *really*, *really*, *really*
don't want the perfect, sometime in the indefinite future, to be the
enemy of the good, ready and working right now. (Plus, the indirection
variable allows an easy override for those who lie about $build).

--
Chuck