Use `ChainedLocalRepositoryManager` to support `-Dmaven.repo.local.tail` #43352

tiagobento · 2024-09-17T22:48:24Z

Hi Quarkus team! 👋 My first PR here! ~~(still draft as I didn't have time to go through all the contribution guidelines yet)~~

maven-resolver introduced Chained LRM a while back. This PR adds support to it, making it possible for Quarkus Maven Plugin to find dependencies on local repositories declared as tail.

Tested locally with a project I have and it worked great, but still need to research how to create tests for it and update the relevant parts of the documentation.

All feedback is appreciated. Thanks!

geoand · 2024-09-18T05:49:30Z

Thanks a lot for the contribution!

@aloubyansky can you have a look?

.../maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/BootstrapMavenContext.java

tiagobento · 2024-09-18T14:30:08Z

@aloubyansky Any pointers on how to test this? And on what parts of the documentation need to be updated?

aloubyansky · 2024-09-18T14:38:17Z

I guess create a couple of local Maven repos in some temp dir and init the resolver, such as in https://github.com/quarkusio/quarkus/blob/main/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/resolver/maven/test/BootstrapMavenContextTestBase.java?

tiagobento

@aloubyansky Added tests and fixed a few bugs that the tests helped me catch :)

I took the liberty to make a small change to BootstrapMavenContextTestBase.java to make it easier to configure BootstrapMavenContext, and had to adapt another test (PreferPomsFromWorkspaceTest.java)

I gave some thought to the documentation changes, and maybe there isn't a need to update anything, given -Dmaven.repo.local.tail is a feature of Maven itself, so actually it would be expected that it works out of the box with Quarkus. From this perspective, this PR is more like a "bug fix" then adding new stuff.

Let me know what you think. I converted the PR to "ready for review" :)

Thanks!

aloubyansky

Thanks @tiagobento
Would you mind adding a test for multiple locations in the tail? Perhaps you want to use different artifacts for that. Also if the same version of the artifact is found in multiple locations, I suppose the first location in the list will win?
BTW, you could use txt artifacts instead of actual binaries for test artifacts.

.../maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/BootstrapMavenContext.java

aloubyansky · 2024-09-19T07:15:56Z

...rc/test/java/io/quarkus/bootstrap/resolver/maven/test/ChainedLocalRepositoryManagerTest.java

+    public void testValidTailViaConfig() throws Exception {
+        final BootstrapMavenContext mvn = bootstrapMavenContextForProject("workspace-with-local-repo-tail",
+                BootstrapMavenContext.config()
+                        .setLocalRepositoryTail(new String[] { M2_LOCAL }));


Could you explain why this one passes? In this case ignore availability will be true, correct?

This is the baseline test, the "happy path"... You're correct that ignoreAvailability will be true by default. It works because the artifact is there, thus is resolved.

tiagobento · 2024-09-19T13:28:30Z

Added two more tests:

testValidTailResolutionOrder; and
testValidTailMultiplicity

Fixed the ArrayList creation with a fixed length.

Kept the .jar extension since they're only empty files. I can change to .txt, though. Just let me know.

aloubyansky · 2024-09-19T15:43:07Z

...rc/test/java/io/quarkus/bootstrap/resolver/maven/test/ChainedLocalRepositoryManagerTest.java

+                BootstrapMavenContext.config()
+                        .setLocalRepositoryTail(new String[] { M2_FROM_REMOTE }));
+
+        assertNotNull(resolveOrgAcmeFooJar001(mvn));


@tiagobento could you please explain this one. ignoreAvailability is true. Is it availability of artifacts that should be ignored?

Sure. So maybe the ignoreAvailability name kind of gives the wrong idea of what it actually does, and maybe a more explicit, albeit verbose, name would be ignoreRemoteAvailability? cc @cstamas

This flag is only applicable for dependencies that have been downloaded from a remote repository prior to being consumed as part of a tail local repository ("tracked", in Maven Resolver's terminology) . See the _remote.repositories file.

This particular test succeeds because maven.repo.local.tail.ignoreAvailability ends up being true, so even though org.acme:foo:jar:0.0.1 was supposedly initially resolved from a remote repository with id = some-repo-id, this fact is ignored, and it being available in the local repository suffices. When maven.repo.local.tail.ignoreAvailability is false, it fails. That case is exactly what is covered by testValidTailFromRemoteCheckingForAvailabilityViaSystemProp.

Thanks for the explanation. The property name is indeed very confusing. It sounds like the meaning is "trust any remote repo" that provided an artifact.

"availability" in resolver means that tracked remote repository (by repoID) is available in the context artifact is asked for. So "remote" may come in as "remote repo ID is available in current context asking for artifact" :)

aloubyansky · 2024-09-19T20:38:25Z

@tiagobento could you please squash the commits? Thanks.

tiagobento · 2024-09-19T20:43:51Z

@aloubyansky Done. Thanks!

github-actions · 2024-09-19T21:17:09Z

🎊 PR Preview bddf9a3 has been successfully built and deployed to https://quarkus-pr-main-43352-preview.surge.sh/version/main/guides/

Images of blog posts older than 3 months are not available.
Newsletters older than 3 months are not available.

quarkus-bot · 2024-09-19T21:18:41Z

Status for workflow `Quarkus Documentation CI`

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

✅ 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.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

quarkus-bot · 2024-09-20T02:09:32Z

Status for workflow `Quarkus CI`

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

✅ 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 17

📦 extensions/redis-client/deployment

✖ io.quarkus.redis.deployment.client.devmode.RedisClientPreloadDevModeTestCase.testImportFileUpdateInDevMode - History

java.lang.RuntimeException: Unable to start Quarkus test resource class io.quarkus.redis.deployment.client.RedisTestResource - java.util.concurrent.CompletionException

java.util.concurrent.CompletionException: java.lang.RuntimeException: Unable to start Quarkus test resource class io.quarkus.redis.deployment.client.RedisTestResource
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
	at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1807)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.RuntimeException: Unable to start Quarkus test resource class io.quarkus.redis.deployment.client.RedisTestResource

tiagobento · 2024-09-20T14:41:01Z

@aloubyansky Thanks for merging it!

Is there a chance to get this back-ported to 3.15? I see the tag is already created for 3.15.0, but maybe for 3.15.1? My team is waiting for 3.15 to be out so we can upgrade from 3.8. Having this feature on the LTS release would be very valuable to us 🙏 Let me know and I'll send the PR if that's ok. Thanks again!

aloubyansky · 2024-09-20T20:21:57Z

I think it can be backported. I added a label, it'll be considered.
FYI, I opened #43420 as a follow up.

Thank you and congratulations on your first contribution to Quarkus! It's a nice one, I'll use it in our tests.

tiagobento · 2024-09-20T20:49:08Z

@aloubyansky Thanks!

tiagobento · 2024-10-15T15:49:40Z

@aloubyansky Hi! I saw this missed 3.15.1 :( Any change it can get included in 3.15.2?

aloubyansky · 2024-10-15T19:00:01Z

It has the backport label, so yes, it will be considered

gsmet · 2024-10-18T12:14:12Z

Unfortunately, this has not been released yet in the wild as 3.16 has not been released and we want some bake time for it.

Depending on when we release 3.15.2, it might miss it but if all goes with the patch well we will consider it for 3.15.3.

(We try to have the patches in a released version before backporting them)

tiagobento · 2024-10-19T18:48:11Z

@gsmet Understood. I'll keep an eye on 3.16 and the upcoming 3.15 patches. Thanks a lot for considering it and getting back to me here ☺️

quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Sep 17, 2024

aloubyansky reviewed Sep 18, 2024

View reviewed changes

.../maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/BootstrapMavenContext.java Outdated Show resolved Hide resolved

tiagobento force-pushed the clrm branch from 85d816b to a8b0b5d Compare September 18, 2024 18:39

tiagobento commented Sep 18, 2024

View reviewed changes

tiagobento marked this pull request as ready for review September 18, 2024 18:44

aloubyansky reviewed Sep 19, 2024

View reviewed changes

tiagobento force-pushed the clrm branch from e0f60e2 to e69ede4 Compare September 19, 2024 20:41

quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Sep 19, 2024

Use ChainedLocalRepositoryManager

5ae6c3b

tiagobento force-pushed the clrm branch from e69ede4 to 5ae6c3b Compare September 19, 2024 20:43

quarkus-bot bot added the triage/flaky-test label Sep 20, 2024

aloubyansky approved these changes Sep 20, 2024

View reviewed changes

aloubyansky merged commit 14af184 into quarkusio:main Sep 20, 2024
52 checks passed

quarkus-bot bot added this to the 3.16 - main milestone Sep 20, 2024

aloubyansky mentioned this pull request Sep 20, 2024

Use varargs for localRepositoryTail #43420

Merged

aloubyansky added the triage/backport-3.15 label Sep 20, 2024

gsmet modified the milestones: 3.16.0.CR1, 3.15.2 Nov 13, 2024

gsmet removed the triage/backport-3.15 label Nov 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `ChainedLocalRepositoryManager` to support `-Dmaven.repo.local.tail` #43352

Use `ChainedLocalRepositoryManager` to support `-Dmaven.repo.local.tail` #43352

tiagobento commented Sep 17, 2024 •

edited

Loading

geoand commented Sep 18, 2024

tiagobento commented Sep 18, 2024

aloubyansky commented Sep 18, 2024

tiagobento left a comment

aloubyansky left a comment

aloubyansky Sep 19, 2024

tiagobento Sep 19, 2024

tiagobento commented Sep 19, 2024

aloubyansky Sep 19, 2024

tiagobento Sep 19, 2024 •

edited

Loading

aloubyansky Sep 19, 2024

cstamas Sep 19, 2024

aloubyansky commented Sep 19, 2024

tiagobento commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

quarkus-bot bot commented Sep 19, 2024

quarkus-bot bot commented Sep 20, 2024 •

edited by github-actions bot

Loading

tiagobento commented Sep 20, 2024

aloubyansky commented Sep 20, 2024

tiagobento commented Sep 20, 2024

tiagobento commented Oct 15, 2024

aloubyansky commented Oct 15, 2024

gsmet commented Oct 18, 2024

tiagobento commented Oct 19, 2024

Use ChainedLocalRepositoryManager to support -Dmaven.repo.local.tail #43352

Use ChainedLocalRepositoryManager to support -Dmaven.repo.local.tail #43352

Conversation

tiagobento commented Sep 17, 2024 • edited Loading

geoand commented Sep 18, 2024

tiagobento commented Sep 18, 2024

aloubyansky commented Sep 18, 2024

tiagobento left a comment

Choose a reason for hiding this comment

aloubyansky left a comment

Choose a reason for hiding this comment

aloubyansky Sep 19, 2024

Choose a reason for hiding this comment

tiagobento Sep 19, 2024

Choose a reason for hiding this comment

tiagobento commented Sep 19, 2024

aloubyansky Sep 19, 2024

Choose a reason for hiding this comment

tiagobento Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

aloubyansky Sep 19, 2024

Choose a reason for hiding this comment

cstamas Sep 19, 2024

Choose a reason for hiding this comment

aloubyansky commented Sep 19, 2024

tiagobento commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

quarkus-bot bot commented Sep 19, 2024

Status for workflow Quarkus Documentation CI

quarkus-bot bot commented Sep 20, 2024 • edited by github-actions bot Loading

Status for workflow Quarkus CI

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/redis-client/deployment

tiagobento commented Sep 20, 2024

aloubyansky commented Sep 20, 2024

tiagobento commented Sep 20, 2024

tiagobento commented Oct 15, 2024

aloubyansky commented Oct 15, 2024

gsmet commented Oct 18, 2024

tiagobento commented Oct 19, 2024

Use `ChainedLocalRepositoryManager` to support `-Dmaven.repo.local.tail` #43352

Use `ChainedLocalRepositoryManager` to support `-Dmaven.repo.local.tail` #43352

tiagobento commented Sep 17, 2024 •

edited

Loading

tiagobento Sep 19, 2024 •

edited

Loading

Status for workflow `Quarkus Documentation CI`

quarkus-bot bot commented Sep 20, 2024 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`