Make kotlin load parent first (revert PR which changes that)

[holly-cummins]

I think the changes in #29697 may save a lot of headaches, but they also have the potential to break users of the kotlin extension. We should hold off on making them to a major version boundary.

Discussion here (in the earlier messages): https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Review.20of.20.22make.20this.20Kotlin-y.20library.20work.20in.20dev.20mode.22/near/315122157

cc @stuartwdouglas @aloubyansky

[quarkus-bot]

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should not contain an issue number (use Fix #1234 in the description instead)

This message is automatically generated by a bot.

[holly-cummins]

I've checked the mac failure, and it's a machine failure (although I have no explanation for why it's happening, since there is enough disk space).

[holly-cummins]

The windows test failure is a bit more worrying, since it's in the kotlin tests. I'll trigger a new build to see if it's something intermittent, since these changes should be relatively innocuous.

[aloubyansky]

I did re-run it yesterday, so it failed twice already.

[holly-cummins]

Oh, thanks for re-running, @aloubyansky. Bah.

I guess the two options are:

• My revert wasn't as clean as I thought and I missed something in the pom.xml
• The other half of Don't load Kotlin parent first #29697, the changes to QuarkusClassLoader, break something kotlin-y on windows, only when kotlin isn't parent-first

I've just pasted in from https://raw.githubusercontent.com/stuartwdouglas/quarkus/422a492c32ab1046f7a872ff4434ebac2923e06e/extensions/kotlin/runtime/pom.xml (the revision before Stuart's changes), and it's identical to what I made manually, so I don't think it's that. I'll try reverting the classloader changes and pushing to this PR to see what happens on Windows, since I can't try it locally. We like those changes, but maybe they expose some other bug.

[holly-cummins]

... and the build that had test failures the first time ... died before finishing. I've looked through the logs and I can't see any evidence it got to integration-tests/kotlin one way or another. Grr.

[stuartwdouglas]

So from the test archives the remote side looks like:

2022-12-15 22:22:40,420 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, kotlin, resteasy, smallrye-context-propagation, vertx, websockets, websockets-client]
2022-12-15 22:22:54,847 INFO  [io.qua.dep.dev.RuntimeUpdatesProcessor] (vert.x-eventloop-thread-0) Scheduled for removal lib/main/org.jetbrains.annotations-17.0.0.jar
2022-12-15 22:22:55,518 INFO  [io.qua.dep.dev.RuntimeUpdatesProcessor] (vert.x-eventloop-thread-0) Scheduled for removal lib/main/org.jetbrains.annotations-17.0.0.jar
2022-12-15 22:22:55,547 INFO  [io.quarkus] (Quarkus Main Thread) acme stopped in 0.012s

And from the test logs:

2022-12-15T22:22:54.8819894Z 2022-12-15 22:22:54,854 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Sending lib/deployment/appmodel.dat
2022-12-15T22:22:54.8820779Z 2022-12-15 22:22:54,867 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Sending lib/boot/org.jetbrains.annotations-17.0.0.jar
2022-12-15T22:22:54.8821481Z 2022-12-15 22:22:54,871 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Sending app/acme-1.0-SNAPSHOT.jar
2022-12-15T22:22:54.8822386Z 2022-12-15 22:22:54,875 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Sending quarkus-run.jar
2022-12-15T22:22:54.8823069Z 2022-12-15 22:22:54,878 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Sending lib/deployment/deployment-class-path.dat
2022-12-15T22:22:55.5946763Z 2022-12-15 22:22:55,520 INFO  [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Connected to remote server
2022-12-15T22:22:55.5947967Z 2022-12-15 22:22:55,577 ERROR [io.qua.ver.htt.dep.dev.HttpRemoteDevClient] (Remote dev client thread) Remote dev request failed: java.net.SocketException: Connection reset

Basically it is sending the first request, and then the remote side shuts down.

[stuartwdouglas]

I think this may caused by #29785 that automatically propagated parent first dependencies.

It looks like somehow the remote and local side end up with different ideas about what is parent first.

[holly-cummins]

I wonder why it only affects Windows?

The options now are:

• Leave the parent first kotlin changes in the codebase. The advantage is that on our side everything just works, the disadvantage is consumers of using kotlin 2.16 may need to make classloading-config changes (and I'll need to have a >2.16 branch and a <2.16 branch of my extension and figure out how to tell the registry about those compatibility rules)
• Revert both the parent-first changes and Propagate parent-first flag #29785 and save them for the 3.0 stream
• Debug why the problem is happening and come up with a fix that allows us to keep Propagate parent-first flag #29785 without also requiring Don't load Kotlin parent first #29697

Either way, we have confirmation that the QuarkusClassloader changes aren't the culprit, so I'll remove the part of this PR which reverted those.

[aloubyansky]

Yes, it shouldn't be specific to Windows then. I don't have Windows installed but I'll have a look.

[stuartwdouglas]

@holly-cummins can you try making org.jetbrains.annotations-17.0.0.jar explicitly parent first?

[holly-cummins]

Done, @stuartwdouglas. We'll find out in five hours if it helped :/ ... well, ten hours if the windows build gets cancelled before completing and we need to respin. (We should probably increase the timeout window on those.)

[holly-cummins]

Double-good news:

• Without the classloader changes, this is a fast build, so we've got the windows results back already
• ... and they're clean

Mac M1 is playing up again with a podman initialization failure (sigh), but I think we can ignore that. So it looks like your suggestion did the trick, @stuartwdouglas.

[aloubyansky]

Did findResource make it significantly slower? Did you notice whether it took longer for other CI steps to complete with it? It'd be interesting to check that.

[holly-cummins]

Ah, @aloubyansky, the build-speed-up I noticed was just because we only need to test the kotlin projects, rather than testing everything, as we do if we change a part of core quarkus like the classloader.

[holly-cummins]

I've just spotted this hasn't been merged. @aloubyansky or @stuartwdouglas, should one of you do the honours?

[holly-cummins]

(I'll squash and rebase.)

[quarkus-bot]

Failing Jobs - Building ea5625d

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 17 MacOS M1	Set up runner	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 18