Support Java 21 main launch protocol

[geoand]

This solution is based on transforming the main class so call-sites don't need to change

Closes: #36154
Alternative to: #36158

[geoand]

P.S. we probably want to add some tests for this, but I am very short on time this week. If I do have find some, I'll add them in a follow up PR

[Ladicek]

Interesting thing about this is that we'll support the new styles of writing main even on older JDKs :-)

(We could also easily support private main methods, but I agree we shouldn't.)

[geoand]

Interesting thing about this is that we'll support the new styles of writing main even on older JDKs :-)

Yeah, I thought that was funny as well. I'll add the noteworthy-label for that reason alone :)

[geoand]

P.S. we probably want to add some tests for this, but I am very short on time this week. If I do have find some, I'll add them in a follow up PR

Actually, I see we have similar QuarkusProdModeTest type tests, so hopefully I can add some tests in a few minutes Done

[Ladicek]

I just realized there's one or two things this PR doesn't do and at least one of them it should.

One, the main method can be protected. That's easy to solve, since changing modifiers is just a bunch of bitwise operations on an int, so you just do removeModifiers(PROTECTED) in addition to addModifiers(PUBLIC) and there's no need for additional check.

Two, an instance main can be inherited from a superclass. Not sure if we wanna do anything about that, I guess we can just leave that unsupported. There's an interesting interaction between static main declared directly on the class and an instance main inherited from a superclass, so it's probably better to just ignore inherited main than implement it incorrectly.

[geoand]

so you just do removeModifiers(PROTECTED) in addition to addModifiers(PUBLIC) and there's no need for additional check.

Good point!

an instance main can be inherited from a superclass

Ah darn... I do see that https://openjdk.org/jeps/445 mentions a superclass as well... Let me chek what we can do about it..

[geoand]

@Ladicek WDYT now? I think the superclass case should be handled now

[geoand]

One thing I am not 100% certain of is the proper order of lookups when there are both main(String[] args) and main() and superclasses involved.
However I think that's an extreme corner case that we can fix later if need be.

[Ladicek]

The JEP says:

When launching a class, the launch protocol chooses the first of the following methods to invoke:

• A static void main(String[] args) method of non-private access (i.e., public, protected or package) declared in the launched class,
• A static void main() method of non-private access declared in the launched class,
• A void main(String[] args) instance method of non-private access declared in the launched class or inherited from a superclass, or, finally,
• A void main() instance method of non-private access declared in the launched class or inherited from a superclass.

That seems quite straightforward to me:

1. If the class declares static void main(String[]), that's the right one.
2. Otherwise, if the class declares static void main(), that's the right one.
3. Otherwise, if the class declares or inherits void main(String[]), that's the right one.
4. Otherwise, if the class declares or inherits void main(), that's the right one.
5. Otherwise, there's no main.

[geoand]

Okay, let's try that

[Ladicek]

I would structure the code differently, but it seems OK. The only objection I would have is that there's no test for a combination of multiple valid main methods (where the correct one must be picked).

[geoand]

Yeah, currentry it does not behave correctly, so I'll need to fix it

[geoand]

Should be fixed now (I also added a test to demonstrate it)

[geoand]

@Ladicek ok to merge?

[Ladicek]

If I'm reading the code correctly, if the main class defined these 2 methods:

• void main(String[])
• static void main()

We would prefer the 1st, even though we really should prefer the 1st, as specified in https://docs.oracle.com/javase/specs/jls/se21/preview/specs/unnamed-classes-instance-main-methods-jls.html#jls-12.1.4

[geoand]

Ah, that's an interesting edge case indeed...

[geoand]

I'm gonna rethink how this is done

[Ladicek]

I would just write an extremely stupid code like this:

ClassVisitor result = findStaticMainWithStringArgs();
if (result != null) {
    return result;
}
result = findStaticMainWithoutArgs();
if (result != null) {
    return result;
}
result = findInstanceMainWithStringArgs();
if (result != null) {
    return result;
}
result = findInstanceMainWithoutArgs();
if (result != null) {
    return result;
}
throw ...

[geoand]

That's where I am headed - at least unless I find any other problems :)

[geoand]

It was slighly more complicated, but not much.

I think that anything other than perhaps extreme edge cases should now be covered

[rsvoboda]

Solves #36154

Thanks @Ladicek and @geoand for the thorough discussion about possible cases + nice impl with attached tests

[geoand]

Thanks for reporting and checking!

[Ladicek]

I'm not a fan of renaming those private mains to $originalMain$, that seems fairly risky and I think it would be better to bail out if we detect a collision. But other than that, it seem fine.

[geoand]

I went back and forth on that.
The reason I did it was because I could not find anywhere in the JEP what is supposed to happen in this case.

Bailing out would definitely have been easier from an implementation standpoint 😎

[quarkus-bot]

Failing Jobs - Building fd9f134

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✖	Native Tests - Messaging1	Build	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Native Tests - Messaging1 #

- Failing: integration-tests/reactive-messaging-kafka 

📦 integration-tests/reactive-messaging-kafka

✖ io.quarkus.it.kafka.KafkaConnectorIT.testDataForKeyed - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.kafka.client.deployment.DevServicesKafkaProcessor#startKafkaDevService threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/vectorized/redpanda:v22.3.4