Done. http://hg.openjdk.java.net/jdk/jdk/rev/593005ac5a0a
> -----Original Message----- > From: Simon Tooke <sto...@redhat.com> > Sent: Donnerstag, 12. September 2019 22:25 > To: Langer, Christoph <christoph.lan...@sap.com>; Erik Joelsson > <erik.joels...@oracle.com>; David Holmes <david.hol...@oracle.com>; > build-dev <build-dev@openjdk.java.net> > Subject: Re: JDK 14 RFR: 8216354: Syntax error in toolchain_windows.m4 > > > 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 > >>>> > >>>> > >>>>
