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

Drop aether-connector-okhttp dependency #1510

Closed
fbricon opened this issue Aug 16, 2023 · 16 comments · Fixed by #1513
Closed

Drop aether-connector-okhttp dependency #1510

fbricon opened this issue Aug 16, 2023 · 16 comments · Fixed by #1513

Comments

@fbricon
Copy link
Contributor

fbricon commented Aug 16, 2023

m2e currently brings an old okhttp version (as seen in redhat-developer/vscode-java#3245), because of aether-connector-okhttp. That extension having been iced, m2e should drop it, and use the same http resolver as maven.

This will probably fix #250 as well.

Either I or someone else from Red Hat will take a stab at it

@laeubi
Copy link
Member

laeubi commented Aug 16, 2023

@fbricon please just make sure that this works on Maven 3.8.x and Maven 3.9.x some of the "deprecated" maven features sadly don't play well with older maven versions see here for testing support of maven 3.8.x:
https://github.com/eclipse-m2e/m2e-core/blob/master/RELEASE_NOTES.md#240

@HannesWell
Copy link
Contributor

m2e currently brings an old okhttp version (as seen in redhat-developer/vscode-java#3245), because of aether-connector-okhttp. That extension having been iced, m2e should drop it, and use the same http resolver as maven.

Can you tell what needs to be changed in the code of m2e to accomplish this goal? If I search the code base for io.takari.aether I only get a hit for HttxWagon in org.eclipse.m2e.tests.common. I added that a while ago to get rid of some test-repositories artifacts committed into the m2e-core-tests submodule. But I remember that this was relatively difficult and fragile.

@fbricon
Copy link
Contributor Author

fbricon commented Aug 19, 2023

We probably just need to replace okhttp connector with a maven-resolver-transport-http, then fix the HttxWagon, which acts like a spy, to get rid of Okhttp.

But I can't figure out how m2e works these days.

  • I have all m2e projects imported in the workspace,
  • activated the m2e target platform,
  • made the changes to maven runtime, changed its version to 3.9.102-SNAPSHOT,
  • updated org.eclipse.m2e.maven.runtime 3.9.102-SNAPSHOT in m2e-core/pom.xml
  • ran the "Updated Maven Project..." command on the maven runtime project
  • m2e-core still shows it's picking up maven runtime 3.8.701 from disk, the okhttp connector is still added to the plugin-dependencies

What happened to workspace project resolution?

@HannesWell
Copy link
Contributor

  • updated org.eclipse.m2e.maven.runtime 3.9.102-SNAPSHOT in m2e-core/pom.xml

    • ran the "Updated Maven Project..." command on the maven runtime project

    • m2e-core still shows it's picking up maven runtime 3.8.701 from disk, the okhttp connector is still added to the plugin-dependencies

What happened to workspace project resolution?

The o.e.m2e.maven.runtime is unfortunately a bit fragile when it comes to changes. I recommend you delete the target folder after modifications, refresh and clean and if still Maven o.e.m2e.maven.runtime 3.8.1 is used a change in the pom.xml of o.e.m2e.maven.runtime help to trigger another build so that the Manifest of o.e.m2e.maven.runtime was generated again.
In general the workspace resolution works and the manifest of o.e.m2e.maven.runtime is generated on the fly, it is just as said a bit fragile, but I didn't have the time yet to investigate why this is the case.

In case you wondered why a 3.8.X is present, it is also contained in the TP to allow users to install older version of the maven-runtime since the jump from 3.8 to 3.9 seems to be not trivial for some users. But you have to ask Christoph if you need details.

@HannesWell
Copy link
Contributor

We probably just need to replace okhttp connector with a maven-resolver-transport-http, then fix the HttxWagon, which acts like a spy, to get rid of Okhttp.

Btw. do you know if there is an equivalent for OkHttpsWagon in maven-resolver?

@fbricon
Copy link
Contributor Author

fbricon commented Aug 19, 2023

no I'm not familiar with that part at all. Maybe @cstamas might know

@fbricon
Copy link
Contributor Author

fbricon commented Aug 19, 2023

looks like triggering a runtime project build is the key. For the Httpx class, I just replaced OkHttpsWagon with Maven's HttpWagon. See #1513

@cstamas
Copy link

cstamas commented Aug 19, 2023

So, Wagon was Maven2 abstraction for various transports (file. http, ssh etc). Since Maven3 and introduction of Aether/MavenResolver the resolver itself carries the "transport api" and provides 3 implementations: classpath, file and http (using Apache HttpClient 4.x). Also, there is an adapter, that adapts Wagon to "transport api", but again, Wagon API is legacy, and IMHO should not be used anymore (at least in resolving).

It just adds anoher ton (to existing) abstraction layers to transport, that literally becomes rather impediment than anything else...

@cstamas
Copy link

cstamas commented Aug 19, 2023

The upcoming Aether/Resolver version 2.0.0 will contain new tranports to enable HTTP/2, and OkHttp was considered but dropped (as it uderperformed), see here apache/maven-resolver#231

@HannesWell
Copy link
Contributor

Thanks Tamas. Do you have some pointers how to use the suggested APIs explicitly?

@fbricon could you consider this?

As far as I understand it, this would mean we don't have to add any jar to the embedded maven-runtime and would only adapt the tests. But I think this would be a good template for downstream consumers of m2e.

The upcoming Aether/Resolver version 2.0.0 will contain new tranports to enable HTTP/2, and OkHttp was considered but dropped (as it uderperformed), see here apache/maven-resolver#231

Resolver 2.0 will not be used for Maven 3.9, will it?

@fbricon
Copy link
Contributor Author

fbricon commented Aug 19, 2023

m2e simply won't perform any http related operations if wagon-http is not provided by the maven runtime. So I don't know which other dependencies to add to make it happy. A whole bunch of resolver jars are there already:

Screenshot 2023-08-19 at 19 56 40

@cstamas
Copy link

cstamas commented Aug 19, 2023

Resolver vs Maven versions:

  • Maven 3.x -> resolver 1.x
  • Maven 4.x -> resolver 2.x (kinda "binary compatible" to 1.9.x, but will have deprecated code removed -- hence "kinda". If you don't use anything deprecated, 1.9.x -> 2.x transitioning should be seamless)
  • Maven 5 (undecided yet, so pinch of salt here, but whenever Resolver becomes inaccessible to "client code" like Mojos) -> resolver 3.x (package rename from org.eclipse to org.apache is planned)

See this thread on ML https://lists.apache.org/thread/g5m53q2qfqf8py4nyr3mf00vb4o9rmj1

@cstamas
Copy link

cstamas commented Aug 19, 2023

Thanks Tamas. Do you have some pointers how to use the suggested APIs explicitly?

Um, use for what? I mean, resolver "transport api" implementations are picked up by resolver and used during fetching of remote artifacts... What "explicit use" you have on your mind?

Note that unlike Wagon, that attempted to be "transport swiss army knife", resolver "transport api" is not trying that. Still, it can be used outside of resolver for some simpler "get me that URL" cases, as done for example in Maven 4 transport API:
https://github.com/apache/maven/blob/maven-4.0.0-alpha-7/api/maven-api-core/src/main/java/org/apache/maven/api/services/Transport.java
is implemented using resolver "transport api"
https://github.com/apache/maven/blob/maven-4.0.0-alpha-7/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultTransport.java

@cstamas
Copy link

cstamas commented Aug 19, 2023

m2e simply won't perform any http related operations if wagon-http is not provided by the maven runtime. So I don't know which other dependencies to add to make it happy. A whole bunch of resolver jars are there already:

The transports in resolver are selected (if protocol "conflicts" like HTTP is, that offers at least 2 transport implementations) in following manner:

  • they are sorted by priority, see here for transport-wagon and here for transport-http
  • this implies that IF both transport-wagon and transport-http are present on classpath, transport-http wins
  • In pre-3.9 Maven versions there were no conflict, as they did deliver only transport-wagon (in lib, transport-http was not included)
  • post-3.9 Maven there is some "tuning" code (along with change of defaults): Check the code that handles these configuration properties (in same class).
  • if no transport can be selected (for any reason), it should throw error, so if m2e is "silent" about this, there may be some exception swallowed somewhere?

Also, check out this page https://maven.apache.org/guides/mini/guide-resolver-transport.html

@cstamas
Copy link

cstamas commented Aug 19, 2023

From the @fbricon screenshot, to get rid of Wagon:

REMOVE:

  • maven-resolver-transport-wagon
  • (but leave wagon-* JARs on classpath as Wagon is directly exposed by core to Mojos, Maven 3.x does expose Wagon)

ADD:

  • maven-resolver-transport-file
  • maven-resolver-transport-http
  • httpclient
  • httpcore

And that should do it (unless there is some more code that somehow "forces" use of Wagon which am unaware of).

@HannesWell
Copy link
Contributor

Thanks @cstamas for these insights. That was very helpful.
Your suggestions were incorporated into #1513 .

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 a pull request may close this issue.

4 participants