Add aarch32 & aarch64 to list of supported archs in openj9-systemtest build script #86
Conversation
Mesbah-Alam
changed the title
Mesbah-Alam
force-pushed
the
addAarch64ToSupportedArchs
branch
from
8a8b25c
to
4166295
Compare
Signed-off-by: [email protected] <[email protected]>
Mesbah-Alam
force-pushed
the
addAarch64ToSupportedArchs
branch
from
4166295
to
a9f51c4
Compare
| <and> | ||
| <contains string="@{arch}" substring="aarch64"/> | ||
| </and> | ||
| </condition> |
There was a problem hiding this comment.
Could we be more generic? For example, set property to @{arch} be the default and only handle exception cases?
In the current code, we will have to update it whenever there is a new platform introduced.
There was a problem hiding this comment.
The task of generalizing the logic is not very straight forward, as the code "Sets a property containing a normalised architecture string from the java os.arch property"; i.e., in the code we are doing a "contains" search in @{arch} for a normalized architecture string, e.g. "ppc", and if found, we are setting the property to that normalized string. If we set property to @{arch} directly, then we won't be setting it to the normalized name of the architecture, but the full name -- which violates the original purpose of the macrodef. However, in any case, if we do want to find a generic solution to avoid having to add the architecture name each time we have a new one, we should ideally handle that in a separate PR.
There was a problem hiding this comment.
If arch is not the exact value that we want, could we use PLATFORM or SPEC and avoid to construct the value again in system tests? PLATFORM and SPEC should be defined before or detected at the beginning of running TKG.
But I agree, we can handle it in another PR.
There was a problem hiding this comment.
We could. I'll open a separate PR to investigate this further. Thanks Lan!
|
The PR is tested here: https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/1183/console Please note that the failure in STF is a separate issue and I'll handle it in a PR belonging to the STF repo -- STF is not liking the non-normal Java version output that Hiro's SDK generates (it has an error reported for the absent JIT). STF should be updated to allow this. |
|
Reviewer @smlambert |
|
Agree, we should create issues against the systemtest repos to improve/simplify the addition of new platforms as it seems like too many different changes in too many different places has to occur to accommodate it, but for now we can proceed with this change, to enable systemtests against the upcoming j9 aarch64 builds. |
|
I will approve this so that we are unblocked for running aarch64 system test builds. |
|
Created new issue: adoptium/aqa-systemtest#228 |
|
Here's a Grinder build that runs a system test successfully using Hiro's sdk: https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/1187/console |
|
Hi @llxia, could we please merge this PR? |
Resolves: #84
Signed-off-by: [email protected] [email protected]