Great, thanks!
On Mon, May 7, 2018 at 5:41 PM, Erik Joelsson <erik.joels...@oracle.com> wrote: > Yes, sure, I'm working on a different fix in the same area now anyway so can > sneak this fix in there. Will get done this week. > > /Erik > > > > On 2018-05-07 07:37, Thomas Stüfe wrote: >> >> Hi Erik, >> >> since your proposal worked, will you do a fix? >> >> Thanks, Thomas >> >> On Fri, May 4, 2018 at 8:30 AM, Thomas Stüfe <thomas.stu...@gmail.com> >> wrote: >>> >>> Hi Erik, >>> >>> that worked on both machines for all builds. >>> >>> Thanks, Thomas >>> >>> On Thu, May 3, 2018 at 10:13 PM, Erik Joelsson <erik.joels...@oracle.com> >>> wrote: >>>> >>>> Hello, >>>> >>>> In my case, VCINSTALLDIR is in short form already. Could you try >>>> changing >>>> line 543 from BASIC_WINDOWS_REWRITE_AS_UNIX_PATH to BASIC_FIXUP_PATH? >>>> >>>> /Erik >>>> >>>> >>>> >>>> On 2018-05-03 11:17, Thomas Stüfe wrote: >>>>> >>>>> Hi Erik, >>>>> >>>>> I see the error on two very different machines. One is my private >>>>> machine - windows 7, VS2013 and VS2017 installed. The second one is my >>>>> corporate Laptop, Windows 10 and only VS2013. >>>>> >>>>> In both cases I use a very recent cygwin64 installation. >>>>> >>>>> Both cases run in TOOLCHAIN_SETUP_MSVC_DLL into the "# Probe: Using >>>>> well-known location from Visual Studio 12.0 and older" case, and in >>>>> the for loop at line 559 attempt to tokenize the POSSIBLE_MSVC_DLL >>>>> variable content. Here the 64bit build on my corporate laptop: >>>>> >>>>> checking if fixpath.exe works... yes >>>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program >>>>> POSSIBLE_MSVC_DLL Files >>>>> POSSIBLE_MSVC_DLL (x86)/Microsoft >>>>> POSSIBLE_MSVC_DLL Visual >>>>> POSSIBLE_MSVC_DLL Studio >>>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcr120.dll >>>>> configure: Found msvcr120.dll at >>>>> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in >>>>> SYSTEMROOT >>>>> checking found msvcr120.dll architecture... ok >>>>> checking for msvcr120.dll... /cygdrive/c/WINDOWS/system32/msvcr120.dll >>>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program >>>>> POSSIBLE_MSVC_DLL Files >>>>> POSSIBLE_MSVC_DLL (x86)/Microsoft >>>>> POSSIBLE_MSVC_DLL Visual >>>>> POSSIBLE_MSVC_DLL Studio >>>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcp120.dll >>>>> configure: Found msvcp120.dll at >>>>> /cygdrive/c/WINDOWS/system32/msvcp120.dll using well-known location in >>>>> SYSTEMROOT >>>>> >>>>> As you can see, we fall back to the default in windows/system32, which >>>>> happens to be working for 64bit. On 32bit, we get this error: >>>>> >>>>> checking if fixpath.exe works... yes >>>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program >>>>> POSSIBLE_MSVC_DLL Files >>>>> POSSIBLE_MSVC_DLL (x86)/Microsoft >>>>> POSSIBLE_MSVC_DLL Visual >>>>> POSSIBLE_MSVC_DLL Studio >>>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll >>>>> configure: Found msvcr120.dll at >>>>> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in >>>>> SYSTEMROOT >>>>> checking found msvcr120.dll architecture... incorrect, ignoring >>>>> configure: The file type of the located msvcr120.dll is PE32+ >>>>> executable (DLL) (GUI) x86-64, for MS Windows >>>>> configure: Found msvcr120.dll at /cygdrive/c/Program Files >>>>> (x86)/Microsoft Visual Studio >>>>> 12.0/VC/redist/arm/Microsoft.VC120.CRT/msvcr120.dll using search of >>>>> VCINSTALLDIR >>>>> checking found msvcr120.dll architecture... incorrect, ignoring >>>>> configure: The file type of the located msvcr120.dll is PE32 >>>>> executable (DLL) (GUI) ARMv7 Thumb, for MS Windows >>>>> checking for msvcr120.dll... no >>>>> configure: error: Could not find msvcr120.dll. Please specify using >>>>> --with-msvcr-dll. >>>>> configure exiting with result code 1 >>>>> >>>>> Best Regards, Thomas >>>>> >>>>> On Thu, May 3, 2018 at 7:18 PM, Erik Joelsson >>>>> <erik.joels...@oracle.com> >>>>> wrote: >>>>>> >>>>>> Also, on my windows system, if I try using my local Visual Studio, >>>>>> both >>>>>> MSVC >>>>>> dll's are found in the Visual Studio dir, and the spaces are all >>>>>> cleaned >>>>>> up. >>>>>> I wonder what's different about your setup, Thomas. >>>>>> >>>>>> /Erik >>>>>> >>>>>> >>>>>> >>>>>> On 2018-05-03 10:13, Erik Joelsson wrote: >>>>>>> >>>>>>> Hello, >>>>>>> >>>>>>> On 2018-05-03 03:41, Magnus Ihse Bursie wrote: >>>>>>>> >>>>>>>> I think the main issue here is that BASIC_FIXUP_PATH should be >>>>>>>> called >>>>>>>> for >>>>>>>> the located msvcr*.dll files. I don't have time to look at it now, >>>>>>>> but >>>>>>>> see >>>>>>>> if you can do a rewrite using BASIC_FIXUP_PATH when the potential >>>>>>>> file >>>>>>>> is >>>>>>>> located. This should remove all spaces from the path. >>>>>>>> >>>>>>> No, that is not a good solution. I intentionally removed that >>>>>>> BASIC_FIXUP_PATH because in VS2017, one of the files has a file name >>>>>>> longer >>>>>>> than 8 characters and BASIC_FIXUP_PATH rewrites that filename to >>>>>>> short >>>>>>> style. This in turn has weird consequences in make when we copy that >>>>>>> file >>>>>>> using generated copy rules. The target file then gets the short style >>>>>>> name >>>>>>> literally. >>>>>>> >>>>>>> When I made that change, I looked at all the calls for potential >>>>>>> locations >>>>>>> and thought all of them had the directory prepared properly already. >>>>>>> It >>>>>>> seems I was wrong. I think the correct solution is to either get rid >>>>>>> of >>>>>>> spaces earlier for all the input directories we search, or maybe >>>>>>> tweak >>>>>>> BASIC_FIXUP_PATH to not touch the actual filename. >>>>>>> >>>>>>> /Erik >>>>>>>> >>>>>>>> /Magnus >>>>>>>> >>>>>>>> On 2018-05-02 20:46, Thomas Stüfe wrote: >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> My 32bit builds on Windows were failing since quite a while and I >>>>>>>>> finally had some minutes to look into that. >>>>>>>>> >>>>>>>>> See prior discussion here: >>>>>>>>> >>>>>>>>> >>>>>>>>> http://mail.openjdk.java.net/pipermail/build-dev/2018-March/021150.html >>>>>>>>> >>>>>>>>> My output used to look like this: >>>>>>>>> >>>>>>>>> checking if fixpath.exe works... yes >>>>>>>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program >>>>>>>>> POSSIBLE_MSVC_DLL Files >>>>>>>>> POSSIBLE_MSVC_DLL (x86)/Microsoft >>>>>>>>> POSSIBLE_MSVC_DLL Visual >>>>>>>>> POSSIBLE_MSVC_DLL Studio >>>>>>>>> POSSIBLE_MSVC_DLL >>>>>>>>> 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcr120.dll >>>>>>>>> configure: Found msvcr120.dll at >>>>>>>>> /cygdrive/c/Windows/system32/msvcr120.dll using well-known location >>>>>>>>> in >>>>>>>>> SYSTEMROOT >>>>>>>>> >>>>>>>>> So basically, build does not correctly search for msvcr120.dll in >>>>>>>>> "/cygdrive/c/Program Files (x86)/Microsoft Visual Studio >>>>>>>>> 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll" - instead, it >>>>>>>>> fails and falls back to the system default >>>>>>>>> "/cygdrive/c/Windows/system32/msvcr120.dll". That dll is a 64bit >>>>>>>>> dll, >>>>>>>>> and so configure fails. >>>>>>>>> >>>>>>>>> Note that 64bit build shows exactly the same behaviour! Only there >>>>>>>>> it >>>>>>>>> works by accident, since the default >>>>>>>>> /cygdrive/c/Windows/system32/msvcr120.dll it finds happens to be a >>>>>>>>> 64bit library too, so configure succeeds. >>>>>>>>> >>>>>>>>> Part of the problem is TOOLCHAIN_SETUP_MSVC_DLL in >>>>>>>>> toolchain_windows.m4. We use a bash for loop to iterate thru a list >>>>>>>>> of >>>>>>>>> one or more files, but that for expression should be quoted. >>>>>>>>> >>>>>>>>> If I make this fix: >>>>>>>>> >>>>>>>>> --- a/make/autoconf/toolchain_windows.m4 Mon Apr 23 18:04:17 >>>>>>>>> 2018 >>>>>>>>> -0700 >>>>>>>>> +++ b/make/autoconf/toolchain_windows.m4 Wed May 02 18:38:04 >>>>>>>>> 2018 >>>>>>>>> +0200 >>>>>>>>> @@ -556,7 +556,7 @@ >>>>>>>>> fi >>>>>>>>> fi >>>>>>>>> # In case any of the above finds more than one file, loop >>>>>>>>> over >>>>>>>>> them. >>>>>>>>> - for possible_msvc_dll in $POSSIBLE_MSVC_DLL; do >>>>>>>>> + for possible_msvc_dll in "$POSSIBLE_MSVC_DLL"; do >>>>>>>>> $ECHO "POSSIBLE_MSVC_DLL $possible_msvc_dll" >>>>>>>>> TOOLCHAIN_CHECK_POSSIBLE_MSVC_DLL([$DLL_NAME], >>>>>>>>> [$possible_msvc_dll], >>>>>>>>> [well-known location in VCINSTALLDIR]) >>>>>>>>> >>>>>>>>> >>>>>>>>> the 32bit configure correctly sets the msvcrt dll: >>>>>>>>> >>>>>>>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program Files (x86)/Microsoft Visual >>>>>>>>> Studio 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll >>>>>>>>> configure: Found msvcr120.dll at /cygdrive/c/Program Files >>>>>>>>> (x86)/Microsoft Visual Studio >>>>>>>>> 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll using >>>>>>>>> well-known >>>>>>>>> location in VCINSTALLDIR >>>>>>>>> checking found msvcr120.dll architecture... ok >>>>>>>>> >>>>>>>>> and I can start the build, but I get follow up errors: >>>>>>>>> >>>>>>>>> ... >>>>>>>>> Creating hotspot/variant-server/tools/adlc/adlc.exe from 13 file(s) >>>>>>>>> Compiling 2 files for BUILD_JVMTI_TOOLS >>>>>>>>> make[3]: *** No rule to make target '/cygdrive/c/Program', needed >>>>>>>>> by >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> '/cygdrive/c/mine/projects/openjdk/jdk-jdk/output-fastdebug-32/support/modules_libs/java.base/Program'. >>>>>>>>> Stop. >>>>>>>>> make[3]: *** Waiting for unfinished jobs.... >>>>>>>>> make[2]: *** [make/Main.gmk:165: java.base-copy] Error 2 >>>>>>>>> make[2]: *** Waiting for unfinished jobs.... >>>>>>>>> >>>>>>>>> I stopped looking at that point, but to me it looks like the build >>>>>>>>> cannot survive msvcrt.dll locations with spaces in them. >>>>>>>>> >>>>>>>>> Kind Regards, Thomas >>>>>>>> >>>>>>>> >
