JDK 14 RFR: 8216354: Syntax error in toolchain_windows.m4

[Langer, Christoph]

Done.

http://hg.openjdk.java.net/jdk/jdk/rev/593005ac5a0a


> -----Original Message-----
> From: Simon Tooke <stooke at redhat.com>
> Sent: Donnerstag, 12. September 2019 22:25
> To: Langer, Christoph <christoph.langer at sap.com>; Erik Joelsson
> <erik.joelsson at oracle.com>; David Holmes <david.holmes at oracle.com>;
> build-dev <build-dev at 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-bounces at openjdk.java.net> On Behalf Of
> Erik
> >> Joelsson
> >> Sent: Donnerstag, 12. September 2019 17:10
> >> To: David Holmes <david.holmes at oracle.com>; Simon Tooke
> >> <stooke at redhat.com>; build-dev <build-dev at 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
> >>>>
> >>>>
> >>>>