RFR: 8243059: Build fails when --with-vendor-name contains a comma

[Severin Gehwolf]

On Fri, 2020-04-17 at 14:28 +0200, Magnus Ihse Bursie wrote:
> On 2020-04-17 14: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/
> 
> Looks good to me.

Thanks for the review!

> (But please verify that it really solves your problem, 
> so there's not a second layer of macro parsing that messes things up in 
> this case.)

I did. It seems to work. Thanks for the reminder :)

Cheers,
Severin