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

Maven: enable configuring maven.top-level-basedir in quarkus:dev #46145

Merged

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Feb 7, 2025

Fixes #22321

Copy link

quarkus-bot bot commented Feb 7, 2025

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • description should not be empty, describe your intent or provide links to the issues this PR is fixing (using Fixes #NNNNN) or changelogs

This message is automatically generated by a bot.

@Ladicek
Copy link
Contributor Author

Ladicek commented Feb 7, 2025

Marking as a draft, because I need to figure out if there's a way to write a test for this.

@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Feb 7, 2025
@Ladicek Ladicek changed the title Enable maven.top-level-basedir in quarkus dev Enable maven.top-level-basedir in quarkus:dev Feb 7, 2025
The `quarkus:dev` configuration `copySurefireVariables` that was added
recently used to copy values without interpolating them. This is not
desirable and this commit fixes that by using what Maven also uses
to configure mojos: `PluginParameterExpressionEvaluator`.
The `quarkus:dev` goal now reads `maven.top-level-basedir` from its
`systemProperties` configuration (which is potentically copied from
Surefire if `copySurefireVariables` is set) and if set, configures
the root project directory for the `BootstrapMavenContext`.

This effectively means that the `maven.top-level-basedir` configuration
may occur not just in a system property, but also in the POM.
@Ladicek Ladicek force-pushed the enable-maven-top-level-basedir-in-quarkus-dev branch from 8790a30 to 9f4118b Compare February 13, 2025 09:55
@Ladicek Ladicek changed the title Enable maven.top-level-basedir in quarkus:dev Maven: enable configuring maven.top-level-basedir in quarkus:dev Feb 13, 2025
@Ladicek
Copy link
Contributor Author

Ladicek commented Feb 13, 2025

Added test, marking as ready.

@Ladicek Ladicek marked this pull request as ready for review February 13, 2025 09:56
Copy link
Member

@aloubyansky aloubyansky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ladicek !

Copy link

quarkus-bot bot commented Feb 13, 2025

Status for workflow Quarkus CI

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

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Build report Produce report and add it as job summary ⚠️ Check → Logs Raw logs 🚧

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingWorkingAppPropsTestCase.testContinuousTestingScenario3 - History

  • io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor\#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7 at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:124) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732) at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856) - java.lang.RuntimeException
java.lang.RuntimeException: 
io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7
	at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:124)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:255)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

@gastaldi gastaldi merged commit 841f37c into quarkusio:main Feb 13, 2025
68 of 70 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 13, 2025
@gastaldi
Copy link
Contributor

Should this be backported to 3.19?

@Ladicek Ladicek deleted the enable-maven-top-level-basedir-in-quarkus-dev branch February 14, 2025 07:34
@Ladicek
Copy link
Contributor Author

Ladicek commented Feb 14, 2025

I don't think this warrants a backport, but I don't really feel strongly on either side.

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 area/maven kind/bugfix triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using maven.top-level-basedir property in quarkus-maven-plugin
3 participants