On 9/12/2019 4:00 PM, Langer, Christoph wrote: > Hi, > > I've also played with this already and support Simon's patch. > > Simon, shall I sponsor it for you?
Yes please, (and thank you). -Simon > > Best regards > Christoph > >> -----Original Message----- >> From: build-dev <build-dev-boun...@openjdk.java.net> On Behalf Of Erik >> Joelsson >> Sent: Donnerstag, 12. September 2019 17:10 >> To: David Holmes <david.hol...@oracle.com>; Simon Tooke >> <sto...@redhat.com>; build-dev <build-dev@openjdk.java.net> >> Subject: Re: JDK 14 RFR: 8216354: Syntax error in toolchain_windows.m4 >> >> Hello David, >> >> The initial report was about this construct not working at all, as Bash >> will not expand wildcards inside of quotes: >> >> "$with_ucrt_dll_dir/*.dll" >> >> In 13 I changed that to remove the quotes to make the statement work at >> all. The drawback then, as Simon pointed out, was that if >> $with_ucrt_dll_dir contains a space, then it won't work. When declaring >> JDK-8216354 as already fixed, I did not consider the case with space. >> >> In my opinion, the proposed construct is the preferred one. The set of >> dlls are known and do not contain spaces. It's however common practice >> on Windows to install these dlls in directories that contain spaces. I >> would support backporting this fix as far back as anyone wants to, >> though the patch will not apply cleanly since different update lines >> currently have different variants of the statement. >> >> Note that internally at Oracle, this code path is not used in any >> official builds, so we are unaffected by this change, which is also why >> we have not detected it. >> >> /Erik >> >> On 2019-09-11 15:07, David Holmes wrote: >>> Hi Simon, Erik, >>> >>> I'm a bit confused. Initially 8216354 was reported as already being >>> fixed by 8215445. But the fix proposed here actually reverts a change >>> from 8215445 which means that far from fixing this issue, 8215445 >>> actually introduced it - correct? >>> >>> But 8215445 was only fixed in 13 - no backports - so it can't be >>> responsible for any problem in 8u or 11u. So some other bug must have >>> originally fixed this before 13, but was not itself backported to 8u >>> or 11u. >>> >>> However the original version of the code was introduced by 8202557 in >>> JDK 11 and was backported to 8u202/8u211. So the code there should be >>> okay - no? >>> >>> The original code (still in 8u, I didn't check 11u) was: >>> >>> "$with_ucrt_dll_dir/*.dll" >>> >>> and the current proposed fix is: >>> >>> "$with_ucrt_dll_dir/"*.dll >>> >>> which suggests the new fix is less robust if the dll name were to >>> contain a space rather than the path (but that's probably not good >>> practice anyway). Or is there some further subtlety I'm missing here? >>> >>> Thanks, >>> David >>> ----- >>> >>> >>> On 12/09/2019 3:55 am, Simon Tooke wrote: >>>> This is a trivial fix that has been sitting around since JDK8. >>>> >>>> At one time, 8216354 was marked "won't fix", but I've seen people >>>> (including myself) run up against this issue enough that I think it >>>> should be addressed. It's been reopened, and I have a webrev ready that >>>> I've tested in the jdk repo, on Windows. It fixes the issue as >>>> described in the bug description. I have tested the patch using a >>>> Cygwin build but not a WSL build. >>>> >>>> This patch alters a change introduced in >>>> >> https://hg.openjdk.java.net/jdk/jdk/annotate/50677f43ac3d/make/autocon >> f/toolchain_windows.m4#l746 >>>> That change introduced an issue that prevented the use of directories >>>> with spaces. >>>> >>>> If this patch is accepted, I intend to backport it to 11u and 8u. >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8216354 >>>> >>>> Webrev: http://cr.openjdk.java.net/~stooke/webrevs/jdk-8216354- >> jdk14/00/ >>>> Thank you, >>>> >>>> -Simon >>>> >>>> >>>>
