On Fri, 2020-04-17 at 07:32 -0700, Erik Joelsson wrote: > This version looks good to me.
Thanks for the review, Erik! Cheers, Severin > /Erik > > On 2020-04-17 05:15, Severin Gehwolf wrote: > > Hi Magnus, > > > > On Fri, 2020-04-17 at 13:44 +0200, Magnus Ihse Bursie wrote: > > > On 2020-04-17 12:18, Severin Gehwolf wrote: > > > > Hi, > > > > > > > > Could I please get a review of this build fix? When --with-vendor-name > > > > contains a comma, like 'foo, bar, Inc.' the build fails. As it turns > > > > out SetupBuildLauncherBody calls SetupJdkExecutable with some > > > > parameters. If $(VERSION_CFLAGS) contain a comma, the comma is being > > > > treated as a prameter delimiter and the build fails. > > > > > > > > I propose to substitute ',' with $(DOLLAR)COMMA in VERSION_CFLAGS so as > > > > to avoid inadvertent expansion of the comma and, thus, it being treated > > > > as a parameter to the SetupJdkExecutable macro. > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8243059 > > > > webrev: > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243059/02/webrev > > > > > > > > NOTE: Magnus pointed out that there were two pre-existing bugs too. > > > > LAUNCHER_NAME and LAUNCHER_CFLAGS should have $$ in front of them, not > > > > just a single one. Fixed this too. > > > Hi Severin, > > > > > > I was a bit unclear in my comment in the bug. :-) > > > > > > What I meant was that you *only* need to use $$(VERSION_CFLAGS). The > > > subst thing should not be necessary. The problem you are seeing is due > > > to VERSION_CFLAGS being evaluated too early without the $$ in front. > > Aah, I see. Makes sense. Updated now: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243059/03/webrev/ > > > > Thanks, > > Severin > >
