Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS #608

psoujany · 2024-09-10T14:18:20Z

This PR reverts below commit which exports IBM_JAVA_OPTIONS to IBM-1047, as this option isn't required anymore. JDK21 z/OS latest builds works fine without any options.
psoujany@28c7f76

defaultCharset of Java21 is UTF-8, TKG scripts which are generating makefiles like utils.mk, rerun.mk are getting created in UTF for 21 on z/OS causing encoding issues. Hence, changing scripts to forcefully create these makefiles in Charset.forName("IBM-1047") only for 21 on z/OS.

psoujany · 2024-12-04T14:04:48Z

Verification runs on sanity.functional
z/OS
21 - Grinder_CR/27631/ - 1 known failure
17 - Grinder_CR/27632 - all passed
11 - Grinder_CR/27633 - all passed

JDK21
zLinux - Grinder_CR/27634 - 1 failure not related to the changes
LinuxAMD - Grinder_CR/27673/

JDK17 LinuxPPCLe - Grinder_CR/27674/

JDK11
AIX - Grinder_CR/27675/
Win - Grinder_CR/27676
LinuxAMD - Grinder_CR/27677

psoujany · 2024-12-06T08:59:11Z

@llxia Could you please review this PR. Thank you.

llxia

This PP does not only revert IBM_JAVA_OPTIONS="-Dfile.encoding=IBM-1047", it sets JVM_OPTIONS=-Dfile.encoding=IBM-1047 in makefile. If this option isn't required anymore. JDK21 z/OS latest builds works fine without any options., then we do not need to set JVM_OPTIONS=-Dfile.encoding=IBM-1047

psoujany · 2024-12-08T13:01:24Z

This PP does not only revert IBM_JAVA_OPTIONS="-Dfile.encoding=IBM-1047", it sets JVM_OPTIONS=-Dfile.encoding=IBM-1047 in makefile. If this option isn't required anymore. JDK21 z/OS latest builds works fine without any options., then we do not need to set JVM_OPTIONS=-Dfile.encoding=IBM-1047

The reason we're setting JVM_OPTIONS here, EnvDetector creates autoGenEnv.mk file for 21 it's in UTF-8. Hence added this JVM_OPTIONS. I'll remove this settings in makefile and will add check for 21 in EnvDetector.java itself.

llxia · 2024-12-09T14:21:21Z

This PR seems to be actively being updated. Is it ready for review? If not, please move it to draft.

psoujany · 2024-12-09T14:33:48Z

Removed JVM_OPTIONS from makefile and added Charset.forName("IBM-1047") to create autoGenEnv.mk in ebcdic for JDK21+ on z/OS.

Verification
z/OS
21 - Grinder_CR/27875
17 - Grinder_CR/27877
11 - Grinder_CR/27878

zLinux 21 - Grinder_CR/27876

@llxia Could please review the changes. Thank you.

llxia · 2024-12-09T19:10:52Z

src/org/testKitGen/MkGen.java

+
+	public Writer getWriterObject(String filetype) {
+		try {
+			if (arg.getSpec().toLowerCase().contains("zos") && (arg.getJdkVersion().matches("2\\d"))) {


This regex does not work if we have a Java version 30+.

llxia · 2024-12-09T19:18:04Z

src/org/testKitGen/UtilsGen.java

 	private String utilsMk;
 	private String rerunMk;
 	private List<String> ignoreOnRerunList;

 	public UtilsGen(Arguments arg, ModesDictionary md, List<String> ignoreOnRerunList) {
 		this.arg = arg;
 		this.md = md;
+		this.mg = new MkGen(arg, null, null, rerunMk, ignoreOnRerunList, ignoreOnRerunList);


It doesn't seem right to initialize MkGen solely to use getWriterObject() in MkGen.java, especially when it involves setting fake parameters in MkGen.

getWriterObject() should be a utility function that can be utilized by all files, including EnvDetector.java.

Please remove this.mg = new MkGen(arg, null, null, rerunMk, ignoreOnRerunList, ignoreOnRerunList);. It is not used.

llxia · 2024-12-09T19:18:40Z

src/org/testKitGen/ModesDictionary.java

@@ -98,7 +98,7 @@ private void parseInvalidSpec(Element modes) throws IOException {
 		ArrayList<String> specs = new ArrayList<String>();
 		int lineNum = 0;
 		BufferedReader reader = null;
-		if (arg.getSpec().toLowerCase().contains("zos")) {
+		if (arg.getSpec().toLowerCase().contains("zos") && !(arg.getJdkVersion().matches("2\\d"))) {


Same here. This regex does not work if we have a Java version 30+.

This has not been addressed

psoujany · 2024-12-11T05:23:16Z

Modified scripts as suggested.

Verification jobs
JDK21 zLinux Grinder_CR/27914/ - All Passed

JDK21 z/OS - Grinder_CR/27949/
JDK17 z/OS - Grinder_CR/27950
JDK11 z/OS - Grinder_CR/27951

llxia · 2024-12-10T15:32:06Z

src/org/openj9/envInfo/Utility.java

+
+    public static Writer writer;
+
+    public static Writer getWriterObject(String jdkVersion, String SpecInfo, String filetype) {


jdkVersion should be int, not String.

filetype -> fileName

llxia · 2024-12-10T15:33:27Z

src/org/testKitGen/UtilsGen.java

 import java.util.List;

+import org.openj9.envInfo.Utility;;


remove extra ;

llxia · 2024-12-11T15:44:21Z

src/org/testKitGen/ModesDictionary.java

@@ -98,7 +98,7 @@ private void parseInvalidSpec(Element modes) throws IOException {
 		ArrayList<String> specs = new ArrayList<String>();
 		int lineNum = 0;
 		BufferedReader reader = null;
-		if (arg.getSpec().toLowerCase().contains("zos")) {
+		if (arg.getSpec().toLowerCase().contains("zos") && !(arg.getJdkVersion().matches("2\\d"))) {


This has not been addressed

llxia · 2024-12-11T15:44:51Z

src/org/testKitGen/UtilsGen.java

 	private String utilsMk;
 	private String rerunMk;
 	private List<String> ignoreOnRerunList;

 	public UtilsGen(Arguments arg, ModesDictionary md, List<String> ignoreOnRerunList) {
 		this.arg = arg;
 		this.md = md;
+		this.mg = new MkGen(arg, null, null, rerunMk, ignoreOnRerunList, ignoreOnRerunList);


Please remove this.mg = new MkGen(arg, null, null, rerunMk, ignoreOnRerunList, ignoreOnRerunList);. It is not used.

llxia · 2024-12-11T15:45:39Z

src/org/testKitGen/UtilsGen.java

@@ -40,7 +44,7 @@ public void generateFiles() {
 	}

 	private void generateUtil() {
-		try (FileWriter f = new FileWriter(utilsMk)) {
+		try (Writer f = Utility.getWriterObject(arg.getJdkVersion(), arg.getSpec(), utilsMk)) {


JDK_VERSION is an optional parameter. Please ensure this works if the user does not set JDK_VERSION.

… for JDK21+ on z/OS Signed-off-by: Pasam Soujanya <[email protected]>

psoujany · 2024-12-16T06:21:40Z

@llxia Could you please review the changes. Thank you.

llxia · 2024-12-16T18:55:12Z

src/org/testKitGen/ModesDictionary.java

@@ -98,7 +98,7 @@ private void parseInvalidSpec(Element modes) throws IOException {
 		ArrayList<String> specs = new ArrayList<String>();
 		int lineNum = 0;
 		BufferedReader reader = null;
-		if (arg.getSpec().toLowerCase().contains("zos")) {
+		if (arg.getSpec().toLowerCase().contains("zos") && !(arg.getJdkVersion().matches("[2-9][0-9]"))) {


arg.getJdkVersion() is still used. Please ensure #608 (comment) is addressed.

llxia · 2024-12-16T21:50:16Z

@psoujany Could you please clarify the purpose of this PR? Based on my observation, the zos JDK21 sanity.functional test build appears to be fine without this PR.

psoujany · 2024-12-17T04:36:13Z

@psoujany Could you please clarify the purpose of this PR? Based on my observation, the zos JDK21 sanity.functional test build appears to be fine without this PR.

The main intention of this PR is to run JDK21 tests without using -Dfile.encoding options. Once we revert 28c7f76, TKG scripts that generates makefiles like utils.mk, rerun.mk, autoGenEnv.mk will be in ascii for JDK21 due to JEP400(default.Charset() is UTF-8). Hence, the changes proposed in this PR will force test specific makefiles to be in IBM-1047 for JDK21 and above on z/OS.

llxia · 2024-12-18T19:30:31Z

Thanks @psoujany. Some of the Grinder link expired. Could you rerun a non-zos Grinder? Thanks

psoujany · 2024-12-19T06:22:28Z

Submitted 11/17/21 jobs on non-z/OS.
11 ppcle - Grinder_CR/28522/
17 zLinux - Grinder_CR/28519/
21 linuxAMD - Grinder_CR/28517/

llxia · 2024-12-19T20:38:48Z

Grinder zos using AQA master branch failed. Is there a related change in AQA repo?

psoujany · 2024-12-20T05:46:22Z

@llxia yes, we need to address this issue in AQA. For time being can we fix encoding of jars using .gitattributes? For verification I've used my fork with the change for z/OS. Please let me know if we can fix .gitattributes so that I can raise PR for that as well. Thank you.
adoptium/aqa-tests#5534

llxia · 2024-12-20T14:58:56Z

We need to ensure that the .gitattributes zOS update does not impact testing on other platforms.

psoujany · 2024-12-23T09:36:52Z

We need to ensure that the .gitattributes zOS update does not impact testing on other platforms.

Raised a PR with the .gitattributes change adoptium/aqa-tests#5833.

psoujany marked this pull request as draft September 10, 2024 14:31

psoujany force-pushed the revert-options branch from f72613c to 9162d92 Compare October 3, 2024 15:02

psoujany force-pushed the revert-options branch 3 times, most recently from 52df309 to 3ed7f0a Compare December 4, 2024 13:58

psoujany changed the title ~~Remove -Dfile.encoding=IBM-1047 for JDK21 zOS~~ Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS Dec 4, 2024

psoujany marked this pull request as ready for review December 6, 2024 05:55

llxia requested changes Dec 6, 2024

View reviewed changes

karianna requested a review from llxia December 8, 2024 21:30

psoujany force-pushed the revert-options branch from a43e10d to 2a548c1 Compare December 9, 2024 11:47

psoujany force-pushed the revert-options branch from 8cb3d2f to 8ee1faa Compare December 9, 2024 14:26

llxia reviewed Dec 9, 2024

View reviewed changes

llxia requested a review from LongyuZhang December 9, 2024 19:19

psoujany marked this pull request as draft December 10, 2024 07:17

llxia reviewed Dec 11, 2024

View reviewed changes

psoujany force-pushed the revert-options branch 2 times, most recently from 0cf4b62 to 0ae6dd7 Compare December 12, 2024 10:21

Remove IBM-1047 export options and create test config files in EBCDIC…

137cecf

… for JDK21+ on z/OS Signed-off-by: Pasam Soujanya <[email protected]>

psoujany force-pushed the revert-options branch from 0ae6dd7 to 137cecf Compare December 12, 2024 10:58

psoujany marked this pull request as ready for review December 12, 2024 10:59

karianna requested a review from llxia December 12, 2024 19:42

llxia reviewed Dec 16, 2024

View reviewed changes

Remove using IBM_JAVA_OPTIONS

a95bb77

karianna requested a review from llxia December 19, 2024 19:51

psoujany mentioned this pull request Dec 23, 2024

Add .gitattributes file to tag jar/war files in binary adoptium/aqa-tests#5833

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS #608

Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS #608

psoujany commented Sep 10, 2024 •

edited

Loading

psoujany commented Dec 4, 2024 •

edited

Loading

psoujany commented Dec 6, 2024

llxia left a comment •

edited

Loading

psoujany commented Dec 8, 2024

llxia commented Dec 9, 2024 •

edited

Loading

psoujany commented Dec 9, 2024

llxia Dec 9, 2024

llxia Dec 9, 2024 •

edited

Loading

llxia Dec 11, 2024

llxia Dec 9, 2024

llxia Dec 11, 2024

psoujany commented Dec 11, 2024

llxia Dec 10, 2024

llxia Dec 10, 2024

llxia Dec 11, 2024

llxia Dec 11, 2024

llxia Dec 11, 2024

psoujany commented Dec 16, 2024

llxia Dec 16, 2024

llxia commented Dec 16, 2024

psoujany commented Dec 17, 2024 •

edited

Loading

llxia commented Dec 18, 2024

psoujany commented Dec 19, 2024

llxia commented Dec 19, 2024

psoujany commented Dec 20, 2024 •

edited

Loading

llxia commented Dec 20, 2024

psoujany commented Dec 23, 2024


		public static Writer writer;

		public static Writer getWriterObject(String jdkVersion, String SpecInfo, String filetype) {

Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS #608

Are you sure you want to change the base?

Remove IBM-1047 export option and generate makefiles in IBM-1047 encoding for JDK21 on z/OS #608

Conversation

psoujany commented Sep 10, 2024 • edited Loading

psoujany commented Dec 4, 2024 • edited Loading

psoujany commented Dec 6, 2024

llxia left a comment • edited Loading

Choose a reason for hiding this comment

psoujany commented Dec 8, 2024

llxia commented Dec 9, 2024 • edited Loading

psoujany commented Dec 9, 2024

Choose a reason for hiding this comment

llxia Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psoujany commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psoujany commented Dec 16, 2024

Choose a reason for hiding this comment

llxia commented Dec 16, 2024

psoujany commented Dec 17, 2024 • edited Loading

llxia commented Dec 18, 2024

psoujany commented Dec 19, 2024

llxia commented Dec 19, 2024

psoujany commented Dec 20, 2024 • edited Loading

llxia commented Dec 20, 2024

psoujany commented Dec 23, 2024

psoujany commented Sep 10, 2024 •

edited

Loading

psoujany commented Dec 4, 2024 •

edited

Loading

llxia left a comment •

edited

Loading

llxia commented Dec 9, 2024 •

edited

Loading

llxia Dec 9, 2024 •

edited

Loading

psoujany commented Dec 17, 2024 •

edited

Loading

psoujany commented Dec 20, 2024 •

edited

Loading