Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support vendor names with spaces in compile.mk #494

Closed
wants to merge 1 commit into from

Conversation

pushkarnk
Copy link

#493 added the JDK_VENDOR property to the compile command. This value is picked up from the "java.vendor" property of the test JDK in TEST_JDK_HOME. If the "java.vendor" value has a whitespace, it breaks the new compile command. Hence, the commit proposes using "" around $(JDK_VENDOR) in COMPILE_CMD.

The new TKG caused failure on Ubuntu because the Ubuntu JDK builds have "java.vendor" set to "Private Build".

adoptium#493 added the JDK_VENDOR
property to the compile command. This value is picked up from
the "java.vendor" property of the test JDK in TEST_JDK_HOME. If
the "java.vendor" value has a whitespace, it breaks the new compile
command. Hence, the commit proposes using "" around $(JDK_VENDOR) in
COMPILE_CMD.
@pushkarnk
Copy link
Author

@llxia Can you review this please?

@llxia
Copy link
Contributor

llxia commented Feb 5, 2024

vendor is a key value that we will use to enable/disable tests. Each vendor would have its own value

public String getJDKVendor() {
String vendor = System.getProperty("java.vendor");
System.out.println("System.getProperty('java.vendor')=" + vendor + "\n");
String vendorLC = vendor.toLowerCase();
if (vendorLC.contains("adoptopenjdk")) {
return "adoptopenjdk";
} else if (vendorLC.contains("eclipse")) {
return "eclipse";
} else if (vendorLC.contains("ibm") || vendorLC.contains("international business machines corporation")) {
return "ibm";
} else if (vendorLC.contains("alibaba")) {
return "alibaba";
} else if (vendorLC.contains("amazon")) {
return "amazon";
} else if (vendorLC.contains("azul")) {
return "azul";
} else if (vendorLC.contains("sap")) {
return "sap";
} else if (vendorLC.contains("bellsoft")) {
return "bellsoft";
} else if (vendorLC.contains("oracle")) {
return "oracle";
} else if (vendorLC.contains("red hat")) {
return "redhat";
} else if (vendorLC.contains("microsoft")) {
return "microsoft";
} else if (vendorLC.contains("loongson")) {
return "loongson";
} else {
System.out.println("Warning: cannot determine vendor, use System.getProperty('java.vendor')=" + vendor + " directly.\n");
return vendor;
}

What is the full output of your "java.vendor"? Just "Private Build" or it contains vendor name (i.e., Canonical Private Build)?

By setting "java.vendor" to just "Private Build" (a generic name), it would be hard to enable/disable tests based on the specific vendor build (i.e., Canonical build) in the future.

Ideally, the correct way is to add "java.vendor" support above JavaInfo.java:

else if (vendorLC.contains("canonical")) { 
            return "canonical"; 
}

@pushkarnk
Copy link
Author

Got your point @llxia. Thank you.

@pushkarnk pushkarnk closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants