Re: RFR(xs) 8221621 FindTests.gmk cannot handle "=" in TEST.groups comments

[Ioi Lam]

Hi Erik and David,

Thanks for the review. I did a tier-1 run and all passed, so I pushed 
the changes.


- Ioi

On 3/28/19 6:50 PM, David Holmes wrote:

+1

Thanks,
David

On 29/03/2019 6:20 am, Erik Joelsson wrote:

Looks good. Thanks for fixing this!

/Erik

On 2019-03-28 10:04, Ioi Lam wrote:

https://bugs.openjdk.java.net/browse/JDK-8221621

The function FindJtregGroupsBody needs to filter out lines that 
starts with "#".


==
diff -r 7d5a4a48e876 make/common/FindTests.gmk
--- a/make/common/FindTests.gmk    Wed Mar 27 14:40:36 2019 -0700
+++ b/make/common/FindTests.gmk    Thu Mar 28 09:59:19 2019 -0700
@@ -53,6 +53,7 @@
 -e 's/^groups\w*=//p' $1/TEST.ROOT)
 $1_JTREG_GROUP_FILES := $$(addprefix $1/, 
$$($1_JTREG_GROUP_FILENAMES))

 $1_JTREG_TEST_GROUPS := $$(strip $$(shell $$(SED) -n \
+    -e 's/^\#.*//g' \
 -e 's/\([^ ]*\)\w*=.*/\1/gp' $$(wildcard 
$$($1_JTREG_GROUP_FILES)) \

 | $$(SORT) -u))
   endif
==

As a sanity check, I tested by adding $(error ${JTREG_TEST_GROUPS}) 
in this file to print out the groups that are found (on a clean repo 
without any "=" in comments). The output is identical with or 
without my change


Thanks
- Ioi

Re: RFR(xs) 8221621 FindTests.gmk cannot handle "=" in TEST.groups comments

[David Holmes]

+1

Thanks,
David

On 29/03/2019 6:20 am, Erik Joelsson wrote:

Looks good. Thanks for fixing this!

/Erik

On 2019-03-28 10:04, Ioi Lam wrote:

https://bugs.openjdk.java.net/browse/JDK-8221621

The function FindJtregGroupsBody needs to filter out lines that starts 
with "#".


==
diff -r 7d5a4a48e876 make/common/FindTests.gmk
--- a/make/common/FindTests.gmk    Wed Mar 27 14:40:36 2019 -0700
+++ b/make/common/FindTests.gmk    Thu Mar 28 09:59:19 2019 -0700
@@ -53,6 +53,7 @@
 -e 's/^groups\w*=//p' $1/TEST.ROOT)
 $1_JTREG_GROUP_FILES := $$(addprefix $1/, 
$$($1_JTREG_GROUP_FILENAMES))

 $1_JTREG_TEST_GROUPS := $$(strip $$(shell $$(SED) -n \
+    -e 's/^\#.*//g' \
 -e 's/\([^ ]*\)\w*=.*/\1/gp' $$(wildcard 
$$($1_JTREG_GROUP_FILES)) \

 | $$(SORT) -u))
   endif
==

As a sanity check, I tested by adding $(error ${JTREG_TEST_GROUPS}) in 
this file to print out the groups that are found (on a clean repo 
without any "=" in comments). The output is identical with or without 
my change


Thanks
- Ioi

Re: [RFR] [8u] 8189761: COMPANY_NAME, IMPLEMENTOR, BUNDLE_VENDOR, VENDOR, but no configure flag

[Andrew John Hughes]

On 28/03/2019 08:51, Severin Gehwolf wrote:
> On Thu, 2019-03-28 at 03:56 +, Andrew John Hughes wrote:
>> On 26/03/2019 21:57, Langer, Christoph wrote:
>>> Hi Andrew,
>>>
>>> thanks for doing this backport. I agree, Severin's finding needs to be 
>>> added to hotspot's Unix/Posix vm.make files.
>>
>> Yes, it was missed because it's already there prior to this patch in the
>> 9 and up HotSpot build which is quite different. It also seems to
>> require a change to vm_version.cpp so the value isn't double-quoted.
> 
> Nice catch! This is JDK-8200115, right? Perhaps we should keep
> "XSTR(VENDOR)" in this backport and then also backport JDK-8200115
> separately?
> 

I wasn't aware of that bug until you mentioned it. I came up with the
same fix independently. I was looking at the 10 sources - where 8189761
was first applied - which still has XSTR(VENDOR) and I guess still has
this bug.

I've flagged it with jdk8u-fix-request and jdk8u-critical-request.

https://bugs.openjdk.java.net/browse/JDK-8200115

and will move it to a separate webrev.
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew

Re: RFR(xs) 8221621 FindTests.gmk cannot handle "=" in TEST.groups comments

[Erik Joelsson]

Looks good. Thanks for fixing this!

/Erik

On 2019-03-28 10:04, Ioi Lam wrote:

https://bugs.openjdk.java.net/browse/JDK-8221621

The function FindJtregGroupsBody needs to filter out lines that starts 
with "#".


==
diff -r 7d5a4a48e876 make/common/FindTests.gmk
--- a/make/common/FindTests.gmk    Wed Mar 27 14:40:36 2019 -0700
+++ b/make/common/FindTests.gmk    Thu Mar 28 09:59:19 2019 -0700
@@ -53,6 +53,7 @@
 -e 's/^groups\w*=//p' $1/TEST.ROOT)
 $1_JTREG_GROUP_FILES := $$(addprefix $1/, 
$$($1_JTREG_GROUP_FILENAMES))

 $1_JTREG_TEST_GROUPS := $$(strip $$(shell $$(SED) -n \
+    -e 's/^\#.*//g' \
 -e 's/\([^ ]*\)\w*=.*/\1/gp' $$(wildcard 
$$($1_JTREG_GROUP_FILES)) \

 | $$(SORT) -u))
   endif
==

As a sanity check, I tested by adding $(error ${JTREG_TEST_GROUPS}) in 
this file to print out the groups that are found (on a clean repo 
without any "=" in comments). The output is identical with or without 
my change


Thanks
- Ioi

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Alexey Ivanov]

On 28/03/2019 17:15, Philip Race wrote:

> I've run SplashScreen jtreg tests, all tests pass.

I assume you mean you did this for 32 AND 64 bit builds ?


Yes, I ran the tests for both 32 and 64 bit builds.

--
Alexey



If so, then +1

-phil.

On 3/28/19, 9:11 AM, Alexey Ivanov wrote:

Any volunteers for review?

On 24/03/2019 19:18, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13.

bug: https://bugs.openjdk.java.net/browse/JDK-8217707
webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/

Description:
Splash screen functionality is broken in 32 bit Windows. It's 
because the functions in splashscreen.dll are exported with 
decorated names now, yet they're looked up by the undecorated names.


The easiest way to reproduce the problem is to run SwingSet2:
java -jar SwingSet2.jar


Fix:
The fix removes JNICALL (__stdcall) declarations from splash screen 
functions. Thus the functions are exported undecorated.


I've run SplashScreen jtreg tests, all tests pass.

This is a follow-up fix to
https://bugs.openjdk.java.net/browse/JDK-8201226
https://bugs.openjdk.java.net/browse/JDK-8202476
which re-enabled 32 bit Windows post removal of mapfiles / compiler 
options.



Thank you in advance.

Regards,
Alexey

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Thomas Stüfe]

Looks fine.

Cheers, Thomas

On Thu, Mar 28, 2019 at 5:11 PM Alexey Ivanov 
wrote:

> Any volunteers for review?
>
> On 24/03/2019 19:18, Alexey Ivanov wrote:
> > Hi,
> >
> > Please review the fix for jdk 13.
> >
> > bug: https://bugs.openjdk.java.net/browse/JDK-8217707
> > webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/
> >
> > Description:
> > Splash screen functionality is broken in 32 bit Windows. It's because
> > the functions in splashscreen.dll are exported with decorated names
> > now, yet they're looked up by the undecorated names.
> >
> > The easiest way to reproduce the problem is to run SwingSet2:
> > java -jar SwingSet2.jar
> >
> >
> > Fix:
> > The fix removes JNICALL (__stdcall) declarations from splash screen
> > functions. Thus the functions are exported undecorated.
> >
> > I've run SplashScreen jtreg tests, all tests pass.
> >
> > This is a follow-up fix to
> > https://bugs.openjdk.java.net/browse/JDK-8201226
> > https://bugs.openjdk.java.net/browse/JDK-8202476
> > which re-enabled 32 bit Windows post removal of mapfiles / compiler
> > options.
> >
> >
> > Thank you in advance.
> >
> > Regards,
> > Alexey
>
>

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Philip Race]

> I've run SplashScreen jtreg tests, all tests pass.

I assume you mean you did this for 32 AND 64 bit builds ?

If so, then +1

-phil.

On 3/28/19, 9:11 AM, Alexey Ivanov wrote:

Any volunteers for review?

On 24/03/2019 19:18, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13.

bug: https://bugs.openjdk.java.net/browse/JDK-8217707
webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/

Description:
Splash screen functionality is broken in 32 bit Windows. It's because 
the functions in splashscreen.dll are exported with decorated names 
now, yet they're looked up by the undecorated names.


The easiest way to reproduce the problem is to run SwingSet2:
java -jar SwingSet2.jar


Fix:
The fix removes JNICALL (__stdcall) declarations from splash screen 
functions. Thus the functions are exported undecorated.


I've run SplashScreen jtreg tests, all tests pass.

This is a follow-up fix to
https://bugs.openjdk.java.net/browse/JDK-8201226
https://bugs.openjdk.java.net/browse/JDK-8202476
which re-enabled 32 bit Windows post removal of mapfiles / compiler 
options.



Thank you in advance.

Regards,
Alexey

RFR(xs) 8221621 FindTests.gmk cannot handle "=" in TEST.groups comments

[Ioi Lam]

https://bugs.openjdk.java.net/browse/JDK-8221621

The function FindJtregGroupsBody needs to filter out lines that starts 
with "#".


==
diff -r 7d5a4a48e876 make/common/FindTests.gmk
--- a/make/common/FindTests.gmk    Wed Mar 27 14:40:36 2019 -0700
+++ b/make/common/FindTests.gmk    Thu Mar 28 09:59:19 2019 -0700
@@ -53,6 +53,7 @@
 -e 's/^groups\w*=//p' $1/TEST.ROOT)
 $1_JTREG_GROUP_FILES := $$(addprefix $1/, 
$$($1_JTREG_GROUP_FILENAMES))

 $1_JTREG_TEST_GROUPS := $$(strip $$(shell $$(SED) -n \
+    -e 's/^\#.*//g' \
 -e 's/\([^ ]*\)\w*=.*/\1/gp' $$(wildcard 
$$($1_JTREG_GROUP_FILES)) \

 | $$(SORT) -u))
   endif
==

As a sanity check, I tested by adding $(error ${JTREG_TEST_GROUPS}) in 
this file to print out the groups that are found (on a clean repo 
without any "=" in comments). The output is identical with or without my 
change


Thanks
- Ioi

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Alexey Ivanov]

Any volunteers for review?

On 24/03/2019 19:18, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13.

bug: https://bugs.openjdk.java.net/browse/JDK-8217707
webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/

Description:
Splash screen functionality is broken in 32 bit Windows. It's because 
the functions in splashscreen.dll are exported with decorated names 
now, yet they're looked up by the undecorated names.


The easiest way to reproduce the problem is to run SwingSet2:
java -jar SwingSet2.jar


Fix:
The fix removes JNICALL (__stdcall) declarations from splash screen 
functions. Thus the functions are exported undecorated.


I've run SplashScreen jtreg tests, all tests pass.

This is a follow-up fix to
https://bugs.openjdk.java.net/browse/JDK-8201226
https://bugs.openjdk.java.net/browse/JDK-8202476
which re-enabled 32 bit Windows post removal of mapfiles / compiler 
options.



Thank you in advance.

Regards,
Alexey

Re: RFR: 8221610: Resurrect (legacy) JRE bundle target

[Martin Buchholz]

On Thu, Mar 28, 2019 at 4:55 AM Alan Bateman 
wrote:

> I'm curious who these "stakeholders" are and what they use these JRE
> bundle for? As you know, moving to a modular platform has blurred the
> historical distinction between what we knew as the JRE and JDK in the
> past. Are they concerned about disk space?
>

Most java deployments today are still jdk8, so the JRE/JDK binary choice is
the only choice users have known for many years.
Even when they migrate to jdk11, initially they will simply want to
compatibly replace their current deployment, which is likely to be a JRE.
Changing the deployment model to building custom runtimes with jlink is
something that can happen when a jdk11 migration is complete - O(years).

Re: RFR: 8221610: Resurrect (legacy) JRE bundle target

[Erik Joelsson]

Hello,

On 2019-03-28 04:47, Zeller, Arno wrote:


Hi Christoph,

thanks for the patch. Just one small suggestion – I think you could 
use the same extension for jdk archive also for the jre archive in 
make/autoconf/spec.gmk.in?


Something like this:

JRE_BUNDLE_NAME := 
jre-$(BASE_NAME)_bin$(DEBUG_PART).$(JDK_BUNDLE_EXTENSION)



I think this makes sense.

Otherwise this looks ok to me. Have you verified that it still works 
when not building any legacy-images and no legacy-image is present in 
the build dir?


Otherwise you will have a difference between jdk and jre archives on 
windows and zip might be more common on windows. I think to get a real 
zip in the end you might need more changes.


This is actually the only change needed. The makefile will figure out 
the tool to use based on the extension.



Best regards,

Arno

*From:*Langer, Christoph
*Sent:* Donnerstag, 28. März 2019 10:07
*To:* build-dev@openjdk.java.net; Erik Joelsson 
*Cc:* Zeller, Arno ; Schuenemann, Rene 


*Subject:* RFR: 8221610: Resurrect (legacy) JRE bundle target

Hi build-dev,

today I’m coming up with kind of a backward oriented suggestion… don’t 
know how well that would be received. Let’s see.


For JDK 11, with JDK-8200132 [0], the JRE build has been moved to legacy.

There has been some discussion beforehand whether the JRE build can 
completely be dropped or how far one can go removing the support for 
it [1]. In the end the decision was made to at least remove the JRE 
from the main targets and only offer some “legacy” targets that would 
still build the JRE. Unfortunately, you were under the assumption that 
nobody except Oracle would use the bundle target and removed it as 
well [2]. Unfortunately, this assumption was not quite true (and I was 
not there to raise my concern ☹). In SapMachine builds, we use the 
bundle targets and we are also still building JRE images for several 
stakeholders. So it would really be good if we could get the JRE 
bundle target back. At the moment we mimick the bundling in a shell 
script that is run after the build.



I'm happy to hear that this got use outside of Oracle!

/Erik

So, I suggest to add back the BUILD_JRE_BUNDLE target in Bundles.gmk 
and add an additional main target called legacy-bundles which can be 
called for creating the JRE bundle.


Of course, this can go eventually when JRE support is finally dropped 
for OpenJDK.


Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8221610.0/ 



Bug: https://bugs.openjdk.java.net/browse/JDK-8221610 



Thanks

Christoph

[0] https://bugs.openjdk.java.net/browse/JDK-8200132 



[1] 
https://mail.openjdk.java.net/pipermail/build-dev/2018-June/022249.html


[2] 
https://mail.openjdk.java.net/pipermail/build-dev/2018-June/022274.html

Re: RFR: 8221610: Resurrect (legacy) JRE bundle target

[Alan Bateman]

On 28/03/2019 09:07, Langer, Christoph wrote:

Hi build-dev,

today I’m coming up with kind of a backward oriented suggestion… don’t know how 
well that would be received. Let’s see.

For JDK 11, with JDK-8200132 [0], the JRE build has been moved to legacy.
There has been some discussion beforehand whether the JRE build can completely 
be dropped or how far one can go removing the support for it [1]. In the end 
the decision was made to at least remove the JRE from the main targets and only 
offer some “legacy” targets that would still build the JRE. Unfortunately, you 
were under the assumption that nobody except Oracle would use the bundle target 
and removed it as well [2]. Unfortunately, this assumption was not quite true 
(and I was not there to raise my concern ☹). In SapMachine builds, we use the 
bundle targets and we are also still building JRE images for several 
stakeholders. So it would really be good if we could get the JRE bundle target 
back. At the moment we mimick the bundling in a shell script that is run after 
the build.

I'm curious who these "stakeholders" are and what they use these JRE 
bundle for? As you know, moving to a modular platform has blurred the 
historical distinction between what we knew as the JRE and JDK in the 
past. Are they concerned about disk space?


-Alan

RE: RFR: 8221610: Resurrect (legacy) JRE bundle target

[Zeller, Arno]

Hi Christoph,

thanks for the patch. Just one small suggestion – I think you could use the 
same extension for jdk archive also for the jre archive in 
make/autoconf/spec.gmk.in?
Something like this:

JRE_BUNDLE_NAME := jre-$(BASE_NAME)_bin$(DEBUG_PART). $(JDK_BUNDLE_EXTENSION)

Otherwise you will have a difference between jdk and jre archives on windows 
and zip might be more common on windows. I think to get a real zip in the end 
you might need more changes.
Best regards,
Arno

From: Langer, Christoph
Sent: Donnerstag, 28. März 2019 10:07
To: build-dev@openjdk.java.net; Erik Joelsson 
Cc: Zeller, Arno ; Schuenemann, Rene 

Subject: RFR: 8221610: Resurrect (legacy) JRE bundle target

Hi build-dev,

today I’m coming up with kind of a backward oriented suggestion… don’t know how 
well that would be received. Let’s see.

For JDK 11, with JDK-8200132 [0], the JRE build has been moved to legacy.
There has been some discussion beforehand whether the JRE build can completely 
be dropped or how far one can go removing the support for it [1]. In the end 
the decision was made to at least remove the JRE from the main targets and only 
offer some “legacy” targets that would still build the JRE. Unfortunately, you 
were under the assumption that nobody except Oracle would use the bundle target 
and removed it as well [2]. Unfortunately, this assumption was not quite true 
(and I was not there to raise my concern ☹). In SapMachine builds, we use the 
bundle targets and we are also still building JRE images for several 
stakeholders. So it would really be good if we could get the JRE bundle target 
back. At the moment we mimick the bundling in a shell script that is run after 
the build.

So, I suggest to add back the BUILD_JRE_BUNDLE target in Bundles.gmk and add an 
additional main target called legacy-bundles which can be called for creating 
the JRE bundle.

Of course, this can go eventually when JRE support is finally dropped for 
OpenJDK.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8221610.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8221610

Thanks
Christoph

[0] https://bugs.openjdk.java.net/browse/JDK-8200132
[1] https://mail.openjdk.java.net/pipermail/build-dev/2018-June/022249.html
[2] https://mail.openjdk.java.net/pipermail/build-dev/2018-June/022274.html

RE: [RFR] [8u] 8189761: COMPANY_NAME, IMPLEMENTOR, BUNDLE_VENDOR, VENDOR, but no configure flag

[Langer, Christoph]

Hi,

> > Revised HotSpot webrev:
> >
> > https://cr.openjdk.java.net/~andrew/openjdk8/8189761/hotspot.02
> 
> +++ new/src/share/vm/runtime/vm_version.cpp   2019-03-28
> 03:52:51.384737947 +
> @@ -140,7 +140,7 @@
> 
>  const char* Abstract_VM_Version::vm_vendor() {
>  #ifdef VENDOR
> -  return XSTR(VENDOR);
> +  return VENDOR;
> 
> This looks like the change from JDK-8200115. Have you considered
> backporting this separately? Failing that, we should mention JDK-
> 8200115 in the backport commit message as well.

I suggest inlining the backport of 8200115 to this commit by adding "8200115: 
System property java.vm.vendor value includes quotation marks" to the commit 
message. It will resolve/backport 8200115 as well. Process wise we should 
probably tag 8200115 with jdk8u-critcal-request and get it approved, though?

Please also update the copyright years accordingly when pushing.

Thanks & Best regards
Christoph

Re: [RFR] [8u] 8189761: COMPANY_NAME, IMPLEMENTOR, BUNDLE_VENDOR, VENDOR, but no configure flag

[Severin Gehwolf]

On Thu, 2019-03-28 at 03:56 +, Andrew John Hughes wrote:
> On 26/03/2019 21:57, Langer, Christoph wrote:
> > Hi Andrew,
> > 
> > thanks for doing this backport. I agree, Severin's finding needs to be 
> > added to hotspot's Unix/Posix vm.make files.
> 
> Yes, it was missed because it's already there prior to this patch in the
> 9 and up HotSpot build which is quite different. It also seems to
> require a change to vm_version.cpp so the value isn't double-quoted.

Nice catch! This is JDK-8200115, right? Perhaps we should keep
"XSTR(VENDOR)" in this backport and then also backport JDK-8200115
separately?

> > Also, the additional printing of those variables in the Unixish 
> > buildtree.make files should be added to windows' make/windows/build.make in 
> > target $(variantDir)\local.make.
> 
> Ah, I was looking for the equivalent but it's odd that it's not in
> make/windows, and so didn't show up with grep.
> 
> Revised HotSpot webrev:
> 
> https://cr.openjdk.java.net/~andrew/openjdk8/8189761/hotspot.02

+++ new/src/share/vm/runtime/vm_version.cpp 2019-03-28 03:52:51.384737947 
+
@@ -140,7 +140,7 @@
 
 const char* Abstract_VM_Version::vm_vendor() {
 #ifdef VENDOR
-  return XSTR(VENDOR);
+  return VENDOR;

This looks like the change from JDK-8200115. Have you considered
backporting this separately? Failing that, we should mention JDK-
8200115 in the backport commit message as well.

Looks good to me otherwise.

Thanks,
Severin