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

Server embeds both guava 15 and 21 #484

Closed
fbricon opened this issue Dec 6, 2017 · 12 comments
Closed

Server embeds both guava 15 and 21 #484

fbricon opened this issue Dec 6, 2017 · 12 comments

Comments

@fbricon
Copy link
Contributor

fbricon commented Dec 6, 2017

Only guava 21 was present in jdt.ls 0.4.0, then guava 15 creeped in 0.5.0

@fbricon
Copy link
Contributor Author

fbricon commented Dec 6, 2017

It's caused by Buildship core changing the guava requirement:

The Spring Tool Suite has a dependency to Guava 15 and to Buildship.
Due to how the dependencies are currently set up, the only way to
make STS and Buildship work together is to use Guava 15 everywhere.

@martinlippert @donat is this still a requirement? This adds 2.3MB to the jdt.ls distro and I'd like to keep it on the lightweight side

@martinlippert
Copy link

@fbricon would love to get rid of the extra lib, but forgot what the exact issue was back then. Need to investigate again and will let you know.

And sorry for the late reply, was at a conference last week, unveiling the public beta of the new spring boot language server... :-)

@fbricon fbricon added the gradle label Dec 13, 2017
@martinlippert
Copy link

Ok, coming back to this now. Sorry for the delay. Can't find the issue where we discussed and investigated this issue between Buildship and STS dealing with the Guava dependency. Do you have a pointer for me?

In general I am totally open to try a dev build of Buildship with Guava 21 as a dependency.

@martinlippert
Copy link

This starts to look funny... :-) I think we added a restriction to the guava lib to STS back in June 2017 to avoid a package-use conflict where buildship was involved (due to some re-exports), so this turns into a chicken-and-egg problem... LOL...

I think once we have a buildship dev build that has an updated version constraint on the guava lib [21,22), I can go ahead and update the STS dependency declarations, too - and we can get rid of guava 15, I hope.

@donat
Copy link

donat commented Jan 4, 2018

@martinlippert Removing Guava from Buildship is still on my list. I'll prepare something for you in the next sprint.

@martinlippert
Copy link

@donat Sounds good, send me a ping once you have a dev build around for testing.

@donat
Copy link

donat commented Jan 23, 2018

Sorry for the late reply. I've prepared a feature branch which uses Guava 18.0 and published it in a snapshot update site: http://download.eclipse.org/buildship/updates/e47/snapshots/3.x/3.0.0.v20180123-1455-s
If you are happy with the changes then it will be merged in the 3.x stream.

@fbricon
Copy link
Contributor Author

fbricon commented Jan 23, 2018

Why guava 18, not 21?

@donat
Copy link

donat commented Jan 23, 2018

Well, one big blocker I see is that Guava 21 requires Java 8 and we support older Eclipse releases that are still on Java 7. In addition, I've seen that we have IP approval for Guava 18 and I convinced myself that this is what you also want. This is on me, sorry about that.

@fbricon
Copy link
Contributor Author

fbricon commented Jan 23, 2018

https://github.com/eclipse/buildship/blob/master/docs/user/Installation.md shows you're building platform targeted builds, so can't you have a oxygen/photon build using 21, and older targets getting 18?

@donat
Copy link

donat commented Jan 23, 2018

That's correct. The only thing is that our build scripts are now not in the best shape and it requires some effort to define different dependency version per platform. In fact, one of my upcoming tasks is to clean up those scripts, I just wanted to give you something before that.

@fbricon
Copy link
Contributor Author

fbricon commented Jan 23, 2018

Migrating to Buildship 3.x requires some changes on our side

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:1.0.0:compile (default-compile) on project org.eclipse.jdt.ls.core: Compilation failure: Compilation failure:
[ERROR] /Users/fbricon/Dev/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/GradleProjectImporter.java:[150]
[ERROR] 	build.getModelProvider().fetchEclipseGradleProjects(FetchStrategy.LOAD_IF_NOT_CACHED, GradleConnector.newCancellationTokenSource().token(), monitor);
[ERROR] 	                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
[ERROR] The method fetchEclipseGradleProjects(FetchStrategy, CancellationTokenSource, IProgressMonitor) in the type ModelProvider is not applicable for the arguments (FetchStrategy, CancellationToken, IProgressMonitor)
[ERROR] /Users/fbricon/Dev/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/GradleProjectImporter.java:[151]
[ERROR] 	build.synchronize(NewProjectHandler.IMPORT_AND_MERGE);
[ERROR] 	      ^^^^^^^^^^^
[ERROR] The method synchronize(NewProjectHandler, CancellationTokenSource, IProgressMonitor) in the type GradleBuild is not applicable for the arguments (NewProjectHandler)
[ERROR] /Users/fbricon/Dev/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/GradleBuildSupport.java:[51]
[ERROR] 	build.get().synchronize(NewProjectHandler.IMPORT_AND_MERGE);
[ERROR] 	            ^^^^^^^^^^^
[ERROR] The method synchronize(NewProjectHandler, CancellationTokenSource, IProgressMonitor) in the type GradleBuild is not applicable for the arguments (NewProjectHandler)

@fbricon fbricon added this to the Mid December 2018 milestone Dec 1, 2018
@fbricon fbricon closed this as completed Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants