Re: [PATCH] msvc: eliminate spaces in the library search path.

[Peter Rosin]

Den 2010-09-21 19:33 skrev Ralf Wildenhues:
> Hi Peter,
> 
> * Peter Rosin wrote on Tue, Sep 21, 2010 at 09:37:16AM CEST:
>> I know it's late for the release, but I'd like to squeeze this one in
>> too, if at all possible. After all, it doesn't affect anything but MSVC.
> 
> I have questions:
> 
> What does Charles have to say to this?
> 
> What is $LIB?  Is this an API you just made up?  If not, where is it
> documented?  Hmm, we used it before, so I guess that's not new.

MS tools use it to find libraries when linking:
http://msdn.microsoft.com/en-us/library/6y6t9esh.aspx

>> Subject: [PATCH] msvc: eliminate spaces in the library search path.
>>
>> * libltdl/m4/libtool.m4 (_LT_SYS_DYNAMIC_LINKER) [mingw, cygwin]
>> <cl*, sys_lib_search_path_spec>: The LIB path variable telling
>> where MSVC looks for libraries is with high probably containing
> 
> s/probably/probability/  ?  If yes, I'd rather write "is likely to
> contain ..."

Ok, I'll fix that.

>> directory names with spaces.  Convert those directory names to
>> the short 8.3 dos form (i.e. without spaces) when storing them
> 
> DOS

Ok.

>> in sys_lib_search_path_spec, as that is a space separated
>> variable.
> 
>> --- a/libltdl/m4/libtool.m4
>> +++ b/libltdl/m4/libtool.m4
>> @@ -2313,15 +2313,46 @@ m4_if([$1], [],[
>>      libname_spec='$name'
>>      soname_spec='${libname}`echo ${release} | $SED -e 
>> 's/[[.]]/-/g'`${versuffix}${shared_ext}'
>>      library_names_spec='${libname}.dll.lib'
>> -    sys_lib_search_path_spec="$LIB"
>> -    if $ECHO "$sys_lib_search_path_spec" | [$GREP ';[c-zC-Z]:/' 
>> >/dev/null]; then
>> -      # It is most probably a Windows format PATH.
>> -      sys_lib_search_path_spec=`$ECHO "$sys_lib_search_path_spec" | $SED -e 
>> 's/;/ /g'`
>> -    else
>> -      sys_lib_search_path_spec=`$ECHO "$sys_lib_search_path_spec" | $SED -e 
>> "s/$PATH_SEPARATOR/ /g"`
>> -    fi
>> -    # FIXME: find the short name or the path components, as spaces are
>> -    # common. (e.g. "Program Files" -> "PROGRA~1")
>> +
>> +    case $build_os in
>> +    mingw*)
>> +      sys_lib_search_path_spec=
>> +      lt_save_ifs=$IFS
>> +      # Doesn't work to have IFS=; so select some other char that is
>> +      # invalid in w32 file names.
>> +      IFS=?
>> +      for lt_path in `echo "$LIB" | tr ';' '?'`
> 
> You should use the fix that you discovered.

Indeed.

>> +      do
>> +        IFS=$lt_save_ifs
>> +        # Let DOS variable expansion print the short 8.3 style file name.
>> +        lt_path=`cd "$lt_path" && cmd //C "for %i in (".") do @echo %~si"`
> 
> Can you explain what this command does?  I mean, no need to change the
> patch, but I don't understand the %~si syntax and I can only infer the
> %i and (...) bits, but can't tell whether they are correct, work by
> accident, or something else.  I'm willing to believe you, but it would
> be nice to know for sure.

Page 5 in this document may give you a hint:
http://www.kohnos.net/files/cmd-quickshots.pdf

Loop around the files in the (".") list, i.e. the current dir (not the content
of the current dir), and echo full path which short names.

> Can the command fail?

Investigation reveals that if a component of $LIB does not exist you'd get
this on stderr:
sh: cd: C:\nowhere: No such file or directory

I'll use this instead:

+        lt_path=`cd "$lt_path" 2>/dev/null && cmd //C "for %i in (".") do 
@echo %~si"`

>> +        sys_lib_search_path_spec="$sys_lib_search_path_spec $lt_path"
>> +      done
>> +      IFS=$lt_save_ifs
>> +      # Convert to MSYS style.
>> +      sys_lib_search_path_spec=`$ECHO "$sys_lib_search_path_spec" | sed -e 
>> 's|\\\\|/|g' -e 's| \\([[a-zA-Z]]\\):| /\\1|g' -e 's|^ ||'`
>> +      ;;
>> +    cygwin*)
>> +      # Convert to unix form, then to dos form, then back to unix form
>> +      # but this time dos style (no spaces!) so that the unix form looks
>> +      # like /cygdrive/c/PROGRA~1:/cygdr...
>> +      sys_lib_search_path_spec=`cygpath --path --unix "$LIB"`
>> +      sys_lib_search_path_spec=`cygpath --path --dos 
>> "$sys_lib_search_path_spec"`
>> +      sys_lib_search_path_spec=`cygpath --path --unix 
>> "$sys_lib_search_path_spec" | $SED -e "s/$PATH_SEPARATOR/ /g"`
> 
> Can any of the cygpath commands fail?
> What about LT_CYGPATH?

LT_CYGPATH is for cases involving Cygwin, but when $build_os is not cygwin.
Quoting the comment in func_convert_file_cygwin_to_w32:

    # because $build is cygwin, we call "the" cygpath in $PATH; no need to use
    # LT_CYGPATH in this case.

So LT_CYGPATH does not apply here.

Investigation reveals that if $LIB has an invalid component, cygpath will not
be able to find the --dos name (2nd conversion) and will output this on stderr:
cygpath: cannot create short name of C:\nowhere
In that case, sys_lib_search_path_spec will be empty even if only one of its
components is bad.  Writing a loop to save the user in this case seems
overkill, no?

I'll use this instead:

+      sys_lib_search_path_spec=`cygpath --path --unix "$LIB"`
+      sys_lib_search_path_spec=`cygpath --path --dos 
"$sys_lib_search_path_spec" 2>/dev/null`
+      sys_lib_search_path_spec=`cygpath --path --unix 
"$sys_lib_search_path_spec" | $SED -e "s/$PATH_SEPARATOR/ /g"`

The 3rd conversion is happy with empty input.

>> +      ;;
>> +    *)
>> +      sys_lib_search_path_spec="$LIB"
>> +      if $ECHO "$sys_lib_search_path_spec" | [$GREP ';[c-zC-Z]:/' 
>> >/dev/null]; then
>> +        # It is most probably a Windows format PATH.
>> +        sys_lib_search_path_spec=`$ECHO "$sys_lib_search_path_spec" | $SED 
>> -e 's/;/ /g'`
>> +      else
>> +        sys_lib_search_path_spec=`$ECHO "$sys_lib_search_path_spec" | $SED 
>> -e "s/$PATH_SEPARATOR/ /g"`
>> +      fi
>> +      # FIXME: find the short name or the path components, as spaces are
>> +      # common. (e.g. "Program Files" -> "PROGRA~1")
> 
> Is this comment still relevant in light of the above changes?
> Assuming yes, for the (*) case.

Yes, it is still true for the (*) case (Wine I suppose).

>> +      ;;
>> +    esac
>> +
>>      # DLL is installed to $(libdir)/../bin by postinstall_cmds
>>      postinstall_cmds='base_file=`basename \${file}`~
>>        dlpath=`$SHELL 2>&1 -c '\''. $dir/'\''\${base_file}'\''i; echo 
>> \$dlname'\''`~

Cheers,
Peter