Re: pr-msvc-support merge

[Peter Rosin]

Hi Ralf!

Den 2010-06-16 20:57 skrev Ralf Wildenhues:
> let's cut the libtool list from replies, ok?

Sure.

> * Peter Rosin wrote on Wed, Jun 16, 2010 at 02:30:47PM CEST:
> > Den 2010-06-14 22:40 skrev Ralf Wildenhues:
> > > * Peter Rosin wrote on Mon, Jun 14, 2010 at 09:35:45AM CEST:
>
> > Running the tests are still an outright pain though, but I will try it.
> > But it might take some time, I'm not used to running them and msys
> > is...cumbersome.
>
> You have several ways to make your life easier: first, read
> tests/README.  Use 'make -jN' for parallel execution.  All of Automake's
> tests should complete without user interaction; if they don't, then we
> will introduce check-{non,}interactive just like we did with Libtool.
> You can use recheck to only check failed tests, and use TESTS=... for
> subsetting as indicated before.

The main trouble is setting up the environment. I have an old MSYS
install and the current MinGW install procedure seems to be in a
state of flux so I fear the upgrade, but I think I'm going to need
latest MSYS to stay sane while installing perl, autoconf etc. But
I'm just whining... I guess there is enough time before the
paperwork has gone all the way.

> > > Some nits and questions:
>
> > +path_conv=
> > +
> > +# func_path_conf build_path
>
> Another remark: beside the typo in the function description s/conf/conv/
> can we s/path/file/g globally on your patch?  The GNU Coding Standards
> want us to speak of paths only for things like $PATH that consists of
> potentially multiple 'file names'.  Thanks.

I don't really like 'file' since the function also handles directories.
But I'm a push-over, so fixed. But for the record, this feels weird:

        linker_opts="$linker_opts -LIBPATH:$file"

> > > > +      case $path_conv in
> > > > +       mingw)
> > > > +         path=`cmd //C echo "$path " | sed -e 's/"\(.*\) " *\$/\1/'`
> > >
> > > I fail to understand what this sed script is for.  Help?
> >
> > It was the easiest I could come up with after experimenting a lot. That
> > wasn't yesterday though, but IIRC if you want to convert paths with
> > spaces, you need to quote the $path for cmd, hence the quotes in the
> > echo "$path " construct. The space before the end quote will make the
> > argument always contain a space which forces MSYS to add quotes when the
> > path is fed to the Windows process (cmd in this case).
>
> We rely on 'echo' not interpreting incoming backslashes here, right?
> Is this something we need to be cautious over, or can 'cmd' only ever
> find echo programs that work this way?  (IIRC all MinGW shells were
> fixed to this end, but the echo called here is a stand-alone program
> right?)

The echo in that command will be the cmd-internal echo command, and how
that echo behaves should be consistent.

> The backslash before $ should not be necessary.  The shell does not
> expand a $ before a /, and the backslash makes me think you're trying
> to sed-match a literal dollar.

Ok, zapped.

> > The quotes are
> > added by MSYS after converting the path to windows form. Without that
> > space, the string is only quoted if it happens to contain a space, so
> > view it as a canonicalizer. The sed script is there to remove those
> > quotes (and the space before the end quote). Also, something seem to
> > mysteriously add a space at the end, so I'm removing that too while at
> > it, but only if it's really there (it felt like a bug that might be
> > fixed at some point). It might be possible to use eval to remove the
> > quotes, but since the path will typically contain backslashes I didn't
> > want to go there.
>
> Yes, eval sounds very wrong, and more dangerous, too.
>
> This explanation of yours lends itself nicely to a testsuite addition
> that exercises the 'compile' script (no need to go through Autoconf or
> Automake indirections), as in
>    cp $testsrcdir/../lib/compile .
>    ./compile ... "weird\path\that\has\backslash-t.c" ...
>    ...

If you create a script named .../cl that just dumps args you can test
that compile does the expected thing w/o having the real cl available.

> I have a more general question though: for all of 'cmd //c', cygpath,
> and wineconv, are their conversions idempotent?  I.e., when I insert a
> converted path, do they produce the same path again (a testsuite
> addition could try to verify this).  This might be necessary because we
> have other pending patches for libtool that convert names to host format
> there already, and those should not be broken.

If you look closely, the path conversion is only invoked if the path -
errmmh file - starts with a single forward slash, so we should be
completely safe if we are given pre-converted file names.

(I find it difficult is it to not say "path" when I refer to either
 a directory or a file name, and so do you...)

The "cost" here is that users with weird mount tables (where a relative
file name does not mean the same thing in posix-land as it does in
windows-land) will not get the correct outcome.

But I really didn't do that optimization to cater for pre-converted
file names, I did it to save a fork in the common case (relative file
names and a "sane" mount table).

So, to answer you question, I think that the MSYS converter is safe
with Windows file names. I also think cygpath is generally safe, but
I'm sure there are also moronic cases which will not work (directories
named C: etc). For winepath I don't know.

> > --- a/NEWS
> > +++ b/NEWS
> > @@ -4,6 +4,9 @@ New in 1.11a:
> >
> >     - automake now generates silenced rules for texinfo outputs.
> >
> > +  - The `compile' script now converts some options for MSVC to make the
> > +    user experience better.
>
> Let's make that 'for a better user experience'.  'make' is such a bad
> verb (sorry for the pun).

Sure, fixed.

> > +func_cl_wrapper ()
> > +{
> > +  # Assume a capable shell
> > +  linker_opts=
> > +  for arg
> > +  do
> > +    if test -n "$eat"; then
> > +      eat=
> > +    else
> > +      case $1 in
> > +       -o)
> > +         # configure might choose to run compile as `compile cc -o foo foo.c'.
> > +         eat=1
> > +         case $2 in
> > +           *.o | *.[oO][bB][jJ])
>
> What about *.[oO]?  Is that something we need to take into account?

I thought about that, but decided not to because .o is only there to
handle the (theoretical?) case where the project is totally unixy (not
even using $objext) and in that case the likelihood of .O seems very
remote. I simply didn't think .O would ever appear. What do you think?

Should I keep attaching the "current" version of the patch?

Cheers,
Peter

cl-wrapper-3.patch
Description: Text document