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

Get rid of aether-okhttp-connector #508 #509

Merged

Conversation

vrubezhny
Copy link
Contributor

Fixes: #508

@vrubezhny
Copy link
Contributor Author

@mickaelistria @fbricon Could you please take a look: It looks like with removing of takagi OkHttp3 dependency I removed (or not added) some required dependency that provides a connector factory, so maven builder fails:

Caused by: org.eclipse.aether.transfer.ArtifactTransferException: org.apache.maven.plugins:maven-javadoc-plugin:pom:3.2.0 failed to transfer from https://repo.maven.apache.org/maven2 during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of central has elapsed or updates are forced. Original error: Could not transfer artifact org.apache.maven.plugins:maven-javadoc-plugin:pom:3.2.0 from/to central (https://repo.maven.apache.org/maven2): No connector factories available
	at org.eclipse.aether.internal.impl.DefaultUpdateCheckManager.newException(DefaultUpdateCheckManager.java:235)
	at org.eclipse.aether.internal.impl.DefaultUpdateCheckManager.checkArtifact(DefaultUpdateCheckManager.java:201)
	at org.eclipse.aether.internal.impl.DefaultArtifactResolver.gatherDownloads(DefaultArtifactResolver.java:586)
	at org.eclipse.aether.internal.impl.DefaultArtifactResolver.performDownloads(DefaultArtifactResolver.java:525)
	at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:449)
	... 100 more

@HannesWell
Copy link

Would it be possible to move directly to the Maven resolver API instead of relying on wagon as suggested in eclipse-m2e/m2e-core#1510 (comment)?
I assume that's the more future proof way.

@HannesWell
Copy link

Any updates on this one?
We soon reach the december release deadline.

@fbricon
Copy link
Contributor

fbricon commented Nov 8, 2023

For me the build fails with:

INFO] --- maven-invoker-plugin:3.6.0:run (provision-test-localrepo) @ lemminx-maven ---
[WARNING] Filtering of parent/child POMs is not supported without cloning the projects
[INFO] Building: pom-localrepo-test-dependencies.xml
	[INFO]   The build exited with code 1. See /Users/fbricon/Dev/projects/lemminx-maven/lemminx-maven/src/test/resources/build.log for details.
[INFO]           pom-localrepo-test-dependencies.xml .............. FAILED (28.01 s)
[INFO] -------------------------------------------------
[INFO] Build Summary:
[INFO]   Passed: 0, Failed: 1, Errors: 0, Skipped: 0
[INFO] -------------------------------------------------
[ERROR] The following builds failed:
[ERROR] *  pom-localrepo-test-dependencies.xml
[INFO] -------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] lemminx-maven 0.10.2-SNAPSHOT ...................... FAILURE [ 32.570 s]
[INFO] lemminx-maven-parent 0.0.1-SNAPSHOT ................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  32.637 s
[INFO] Finished at: 2023-11-08T11:11:28+01:00
[INFO] ------------------------------------------------------------------------

and in the build.log:

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project get-localrep-test-dependencies: Could not resolve dependencies for project org.eclipse.lemminx.maven.tests:get-localrep-test-dependencies:pom:0.0.1-SNAPSHOT: Could not find artifact jakarta.platform.:jakarta.jakartaee-api:jar:10.0.0 in central (https://repo.maven.apache.org/maven2) -> [Help 1]

@fbricon
Copy link
Contributor

fbricon commented Nov 8, 2023

in the build.log, I can see

[INFO] Downloading from central: https://repo.maven.apache.org/maven2/jakarta/platform//jakarta.jakartaee-api/10.0.0/jakarta.jakartaee-api-10.0.0.jar

Notice the double / in platform//jakarta.jakartaee-api, this is causing the download failure. Why is there a double / in the 1st place? I have no clue

@fbricon
Copy link
Contributor

fbricon commented Nov 8, 2023

This is caused by a combination of https://github.com/eclipse/lemminx-maven/blob/06815c25ef95ac163e4052073abcf8fa0ad4e27a/lemminx-maven/src/test/resources/pom-localrepo-test-dependencies.xml#L40-L50

and lemminx-maven using Maven wrapper 3.6.0. Bumping the wrapper to 3.9.5 makes the hack usable.

@fbricon
Copy link
Contributor

fbricon commented Nov 8, 2023

Adding maven-resolver-connector-basic, there are now only 12 failures:

Error:  Failures: 
Error:    RemoteCentralRepoTest.testGetArtifactVersionss:106 expected: not <null>
Error:    RemoteCentralRepoTest.testGetArtifacts:55 expected: not <null>
Error:    RemoteCentralRepoTest.testGetGroupIdss:130 expected: not <null>
Error:    RemoteCentralRepoTest.testGetPlugiArtifacts:177 expected: not <null>
Error:    RemoteCentralRepoTest.testGetPluginArtifactVersionss:228 expected: not <null>
Error:    RemoteCentralRepoTest.testGetPluginGroupIdss:251 expected: not <null>
Error:  Errors: 
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralArtifactIdCompletion � Timeout testRemoteCentralArtifactIdCompletion() timed out after 15000 milliseconds
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralGroupIdCompletion � Timeout testRemoteCentralGroupIdCompletion() timed out after 15000 milliseconds
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralPluginArtifactIdCompletion � Timeout testRemoteCentralPluginArtifactIdCompletion() timed out after 15000 milliseconds
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralPluginGroupIdCompletion � Timeout testRemoteCentralPluginGroupIdCompletion() timed out after 15000 milliseconds
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralPluginVersionCompletion � Timeout testRemoteCentralPluginVersionCompletion() timed out after 15000 milliseconds
Error:    RemoteCentralRepoAssistanceTest.testRemoteCentralVersionCompletion � Timeout testRemoteCentralVersionCompletion() timed out after 15000 milliseconds
[INFO] 
Error:  Tests run: 177, Failures: 6, Errors: 6, Skipped: 0

RemoteCentralRepoTest don’t fail if run standalone, eg ./mvnw clean verify -Dtest=RemoteCentralRepoTest
RemoteCentralRepoAssistanceTest likely caused by some CompletableFuture shenanigan

@vrubezhny vrubezhny changed the title Replace aether-okhttp-connector with Maven's wagon-http #508 Get rid of aether-okhttp-connector #508 Nov 9, 2023
@vrubezhny vrubezhny marked this pull request as ready for review November 9, 2023 01:05
@vrubezhny
Copy link
Contributor Author

@fbricon Could you please take a look - does it work for m2e-core?
(No Takagi OkHttp3 nor WagonHttp added)

Copy link

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Thanks Victor for working on this.

lemminx-maven/pom.xml Outdated Show resolved Hide resolved
@vrubezhny vrubezhny force-pushed the fixGetRidOfTakagiOkHttp3 branch 2 times, most recently from b3e084f to 1c97809 Compare November 9, 2023 11:52
@vrubezhny vrubezhny marked this pull request as draft November 9, 2023 12:31
@mickaelistria
Copy link
Contributor

This looks good to me.

Added a simple Password Authenticator implementation, however it's not tested as well as in old version with Takagi OkHttp3

That's life... Let users test it ;)

@vrubezhny vrubezhny marked this pull request as ready for review November 9, 2023 18:47
Copy link

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

I cannot say much about the code, but from a dependencies perspective this looks good to me. 👍🏽

@vrubezhny
Copy link
Contributor Author

@fbricon @mickaelistria @HannesWell Thanks!

@vrubezhny vrubezhny merged commit 19e793a into eclipse-lemminx:master Nov 9, 2023
3 checks passed
@HannesWell
Copy link

Great. Thanks @vrubezhny.
Do you plan to provide a PR to temporarily use the latest snapshot of lemminx-maven in m2e?
This would require a lemminx-maven release soon since I want to release m2e in time for the december release, but we could test the current state for a few days/a week I think.

@vrubezhny
Copy link
Contributor Author

@HannesWell I'm going to release Lemminx-Maven, so m2e-core be able to pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace aether-okhttp-connector with Maven's wagon-http
4 participants