-
Notifications
You must be signed in to change notification settings - Fork 866
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
Update the mx suite parsing to work with sources of GraalVM version 23.1.0 #7013
Conversation
NB21 or master? |
Well, master. It's too late for 21, I guess, right? |
restarting CI with
MX
|
Not yet. There's at least one pending fix marked for NB21, so if you want this fix in there target delivery and it can be reviewed on that basis. |
This will need rebasing and force pushing on head of delivery. |
Yes, sure, I'm sorry, will do ASAP... |
No problem, that's normal. Once it's done, feel free to remove the |
55acba7
to
9ab7f14
Compare
I also realized the MX project support is broken and I created |
JDK 11:
JDK 17/21 have failures in:
I believe the JDK 11 failure is expected since current graal versions are based on later JDKs. |
9ab7f14
to
f93dce0
Compare
@entlicher on what JDK versions should the mx project tests now run? if its JDK 17 and 21 I suggest the following change: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 8e28440..786f1bc 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -900,6 +900,12 @@
# - name: java/ant.grammar
# run: ant $OPTS -f java/ant.grammar test
+ # only JDK 17 and 21
+ - name: java/java.mx.project
+ if: ${{ matrix.java != '11' }}
+ continue-on-error: ${{ github.event_name != 'pull_request' }}
+ run: .github/retry.sh ant $OPTS -f java/java.mx.project test
+
- name: Set up JDK 17 for JDK 21 incompatible tests
if: ${{ matrix.java == '21' }}
uses: actions/setup-java@v4
@@ -908,10 +914,6 @@
distribution: ${{ env.default_java_distribution }}
# TODO fix JDK 21 incompatibilites
- - name: java/java.mx.project
- continue-on-error: ${{ github.event_name != 'pull_request' }}
- run: .github/retry.sh ant $OPTS -f java/java.mx.project test
-
- name: java/gradle.java
run: .github/retry.sh ant $OPTS -f java/gradle.java test
NetBeans 22 will be JDK 17+ anyway, so I can clean this up later when we update the test matrix. |
Many thanks for the suggested change! |
f93dce0
to
784e4b8
Compare
i enabled all tests since I had the feeling this might cause more indirect issues. The "Build all Tests" step fails now since it was used as validator for JDK 11 compatibility. We could continue the same path and bump that job to 17 too. This is probably not ideal to do in the week before release (given that NB 21 is supposed to build/run on JDK 11 while NB 22 will be 17+) but on the other hand it would be nice if NB 21 would be able to be used for mx projects. This is also just about building tests, so I would be ok with this early bump - other opinions welcome of course. relevant line would be here: netbeans/.github/workflows/main.yml Line 341 in 1c19e60
lets hope it builds on JDK 17 ;) the JDK 21
|
784e4b8
to
f5197be
Compare
Feel free to pick whatever you find useful in https://github.com/apache/netbeans/pull/7011/files#diff-d09b7dca37327eaec02560e4951f9024cf208ad0eacd20755dde663b9b15c612R46 - for example this test. Or I can integrate the test later, when this PR merges. |
Thanks Jaroslav! I've reverted the version of |
Thanks! Is the test now stable enough to drop the retry behaviour in the workflow file, or is that change unintentional in here? |
Yes, the test is stable. At least, I've run it locally several times without issues. |
I've removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good from my side, please consider the retry script and make sure you read @JaroslavTulach comment #7013 (comment)
I see you reverted the graal version bump -> I think we can bump it again in a week or so for NB 22 when CI is ready for JDK 17.
Thanks a lot!
f5197be
to
dc24f41
Compare
for the interested: #7019 is now green. If it makes it into master, it should be easily possible to update graal tests to JDK 17 equivalent graal versions - which is great since graal 17 would be still a supported release. The graal job is still using an EOL graal sdk, I haven't touched that since #6369 |
The mx project of recent versions of GraalVM can not be successfully parsed. There is an issue in graal/sdk suite.
This change adapts to the mx suite form and a test is updated to work with the new unchained project structure.