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

Do not check for multi-release jar when reading files from META-INF #41397

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Jun 24, 2024

Related to #41395

@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Jun 24, 2024
@gsmet
Copy link
Member Author

gsmet commented Jun 24, 2024

@dmlloyd my understanding is it's the classes that can be multi-releases, right?

Here, I'm trying to avoid reading the manifest when we go find the quarkus-extension metadata in the jars.

@dmlloyd
Copy link
Member

dmlloyd commented Jun 24, 2024

Right, this check looks safe to me.

@aloubyansky
Copy link
Member

Just in case, the multi release concept isn't limited to classes.
See Class Loader Resources paragraph in https://openjdk.org/jeps/238 for an example.

@dmlloyd
Copy link
Member

dmlloyd commented Jun 24, 2024

I think in this case (META-INF) it's OK. The JEP says this:

A JAR file has a content root, which contains classes and resources, as well as a META-INF directory which contains metadata about the JAR.

And then later:

JAR metadata, such as that found in the MANIFEST.MF file and the META-INF/services directory, need not be versioned.

I would generally extrapolate this to include anything in META-INF (including the versioned directories themselves, which otherwise would lead to some ugly recursion problems).

@aloubyansky
Copy link
Member

Agreed

Copy link

quarkus-bot bot commented Jun 24, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit f3fe247.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/reactive-messaging-kafka

io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

  • Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@gsmet gsmet merged commit 39dd3b2 into quarkusio:main Jun 25, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants