RFR: 8233995: java.vm.vendor (and potentially other properties/fields) not correctly set in Windows/Hotspot build of OpenJDK8

[Langer, Christoph]

Hi Andrew,

I've finally gotten back to this and spent some more cycles.

Here is a new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8233995.8u.1/

I'm now passing COMPANY_NAME, VENDOR_URL, VENDOR_URL_BUG, VENDOR_URL_VM_BUG and VERSION_CFLAGS via MAKE_ARGS in make/windows/makefiles/defs.make. I needed to add some logic for converting VERSION_CFLAGS into a value that can be passed as argument to the nmake call and would then also be consumable by the VS C++ compiler. The elegant thing of doing it so is that I don't need to duplicate the logic for creating VERSION_CFLAGS from spec.gmk in make/windows/makefiles/vm.make.

I also removed the common amendment of CXX_FLAGS with VENDOR defines in make/windows/makefiles/vm.make and I would pass VERSION_CFLAGS only to the compilation of vm_version.cpp and arguments.cpp as it is done on the other platforms. I furthermore fixed the value of VENDOR in generated file local.make (done in make/windows/build.make) and I also corrected the value of HS_COMPANY in the RC flags for creating jvm.dll in make/windows/makefiles/compile.make.

Cheers
Christoph

> -----Original Message-----
> From: Langer, Christoph
> Sent: Mittwoch, 13. November 2019 23:40
> To: Andrew John Hughes <gnu.andrew at redhat.com>; jdk8u-
> dev at openjdk.java.net
> Subject: RE: RFR: 8233995: java.vm.vendor (and potentially other
> properties/fields) not correctly set in Windows/Hotspot build of OpenJDK8
> 
> Hi Andrew,
> 
> thanks for looking into my webrev.
> 
> > Indeed, the build system did change in OpenJDK 9 with "JDK-8150601:
> > Remove the old Hotspot build system". As JDK-8189761: COMPANY_NAME,
> > IMPLEMENTOR, BUNDLE_VENDOR, VENDOR, but no configure flag"
> occurred
> > later than this, the HotSpot parts of the 8189761 had to be constructed
> > from scratch when I backported it. Not having access to Windows myself,
> > I'm reliant on others to test the changes there and report breakage.
> 
> Ah, I didn't do that bug-archeology. Good to know.
> 
> > Looking over this patch, and comparing with what we do for
> > AIX/Solaris/BSD/Linux in 8189761, I have two questions:
> >
> > 1. We echo the values of VENDOR, VENDOR_URL, VENDOR_URL_BUG and
> > VENDOR_URL_VM_BUG to local.make in build.make, in much the same
> way
> > as
> > buildtree.make does for the other platforms. Any idea why that is not
> > sufficient and these need to be also added to defs.make? If the
> > build.make variables aren't being set, that has an impact for other
> > variables too. I notice ZIPEXE is in both places, with no checks in the
> > defs.make case:
> >
> > hotspot/make/windows/build.make:        @ if "$(ZIPEXE)" NEQ "" echo
> > ZIPEXE=$(ZIPEXE)           >> $@
> > hotspot/make/windows/makefiles/defs.make:MAKE_ARGS +=
> > ZIPEXE=$(ZIPEXE)
> >
> > If possible, it would be better to fix the underlying issue rather than
> > papering over it.
> 
> Well, for Unix-ish builds, "make buildtree.make ..." gets called (e.g. from
> hotspot/make/linux/Makefile) and buildtree.mak includes spec.gmk and
> hence has access to all values.
> For Windows, however, we hand over to nmake. "nmake build.make ..." gets
> called from hotspot/make/Makefile and build.make does not include
> spec.gmk - it probably can't since nmake/gnumake have different syntax.
> Hence, the only way to pass arguments to build.make is via MAKE_ARGS in
> defs.make.
> 
> I don't see an easy fix here...
> 
> The handling of ZIPEXE seems correct, too.
> 
> > 2. The checks added in vm.make are done by configure for the other
> > builds and assembled into VERSION_CFLAGS. To avoid duplication  in
> > vm.make, is it possible to just use VERSION_CFLAGS there as the other
> > systems do? If Windows can't handle the -D syntax that uses, maybe a
> > better solution is to setup VERSION_WINFLAGS in the same place with the
> > correct syntax?
> 
> Hm, maybe we can try to pass VERSION_CFLAGS via MAKE_ARGS and use it
> in vm.make. I'll give it a try. Stay tuned...
> 
> Best regards
> Christoph