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

Consider adding plug-in nature to LSP4J projects #542

Closed
pisv opened this issue Apr 5, 2021 · 32 comments
Closed

Consider adding plug-in nature to LSP4J projects #542

pisv opened this issue Apr 5, 2021 · 32 comments
Milestone

Comments

@pisv
Copy link
Contributor

pisv commented Apr 5, 2021

As mentioned in #537 (comment), unless API tooling is on, despite best efforts, projects break API compatibility/semantic versioning.

As also noted in #537 (comment), to enable Eclipse API Tools for LSP4J projects, they have to become plug-in projects, i.e. each of them needs to have:

  1. META-INF/MANIFEST.MF file

  2. Plug-in nature and builders

It seems that xtext-core could serve as an example of how this can be done.

eclipse/xtext-core#1273 demonstrates how to add (generate) plug-in nature and builders with Gradle.

To be able to use all the features of Eclipse API Tools, MANIFEST.MF will need to become a primary artefact (i.e. not generated), just as it is in xtext-core.

This probably also means that a target platform will need to be defined.

My assumption is that if it works for xtext-core, then it should probably also work for LSP4J, as the projects look similar (e.g. both of them need to work with or without OSGi, both use Gradle). I don't know how well it works in xtext-core though.

What do you think about it? Should it be investigated further or perhaps there are already known issues with this approach?

My main concern at the moment is Gradle/Buildship -- which admittedly I don't have much experience with -- as opposed to Maven/Tycho, which is a more common way of building plug-in projects.

@jonahgraham
Copy link
Contributor

Besides LSP4E, are there other OSGi consumers of LSP4J? does Handly or LXTK consume it?

The answer to whether we should do this depends on who are consumers and contributors are (or are likely to be).

As an aside - I am fine with ditching gradle and moving to standard Maven/Tycho. It has been difficult at times to handle gradle for me, and this is the only project that I work on that is gradle.

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Yes, LXTK is also an OSGi consumer of LSP4J, and I would like to contribute personally in making this happen to enable Eclipse API Tools for LSP4J, but... I'm a bit puzzled, since I thought this was more about facilitating proper API management/versioning in LSP4J itself. It will not affect LXTK directly, for example, as far as I can tell.

I would welcome moving from Gradle for the same reasons, but it will be a major frightening change, of course :-)

@jonahgraham
Copy link
Contributor

Yes, LXTK is also an OSGi consumer of LSP4J, and I would like to contribute personally in making this happen to enable Eclipse API Tools for LSP4J, but...

Great!

I'm a bit puzzled, since I thought this was more about facilitating proper API management/versioning in LSP4J itself. It will not affect LXTK directly, for example, as far as I can tell.

It is indeed - my question was to try to align technologies on related projects. With LSP4E, LXTK both in the OSGi workflows having LSP4J makes more sense - same reasoning on gradle vs other technologies.

I would welcome moving from Gradle for the same reasons, but it will be a major frightening change, of course :-)

It is a tradeoff - can we get the API checking added to gradle more or less easily that transitioning to maven+tycho.

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

my question was to try to align technologies on related projects

Oh, I see, thanks!

it is a tradeoff - can we get the API checking added to gradle more or less easily that transitioning to maven+tycho.

As a potential contributor, I would vote for ignoring the trade-off (i.e. the "more or less easily" part) and move to Maven/Tycho directly if that seems to be what we want to see strategically. Should a separate issue be filed for this?

@nixel2007
Copy link
Contributor

nixel2007 commented Apr 5, 2021

I hope moving to eclipse api tooling/plugin project won't break scenario when lsp4j is used as regular maven dependency without all osgi-stuff

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

I hope moving to eclipse api tooling/plugin project won't break scenario when lsp4j is used as regular maven dependency without all osgi-stuff

It should certainly not break anything. Otherwise, it is no-go, as far as I can tell.

@jonahgraham
Copy link
Contributor

I hope moving to eclipse api tooling/plugin project won't break scenario when lsp4j is used as regular maven dependency without all osgi-stuff

It should certainly not break anything. Otherwise, it is no-go, as far as I can tell.

100% - regular maven support with publishing to maven central is a requirement.

Also, building/editing LSP4J without requiring Eclipse IDE is also a requirement.

[...] without all osgi-stuff

FWIW - the bundles published today to Maven are OSGi compatible.

Should a separate issue be filed for this?

No - it can be done as part of this issue. Personally I don't mind either way.

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Also, building/editing LSP4J without requiring Eclipse IDE is also a requirement.

Regarding the editing part -- if it is understood as full featured development and not as something very basic -- this is going to be pretty hard to address, I think :-(

At least, I don't know how to implement it without introducing a (development-time) dependency on the PDE. Especially if we are going 'the classic way' without Gradle, things like .project, .classpath, .target files will need to become primary artefacts (i.e. not generated). Even with Gradle, I think it is going to be hard.

Just to confirm, is this actually possible currently and to what extent?

(Our contributing guide talks only about Eclipse as an IDE.)

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Besides, we already have a dependency on Xtend SDK for full-featured development...

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Stupid me. Have you probably meant "building/editing LSP4J without requiring an IDE is also a requirement"? :-)

@jonahgraham
Copy link
Contributor

Stupid me.

No!

Have you probably meant "building/editing LSP4J without requiring an IDE is also a requirement"? :-)

Sorry for lack of clarity - I want to make sure the flow of developing LSP4J with whatever IDE (or none) is not worse after this change. People are developing their language server with VS Code or Theia - we shouldn't force them into Eclipse IDE to develop this component.

However I don't have explicit rules as to what that means. But at a minimum it means that the build & test from the command line is required.

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Thank you for clarifying it, Jonah.

But at a minimum it means that the build & test from the command line is required.

I think this minimum can be fully met with Maven/Tycho.

Regarding using other IDEs besides Eclipse for developing LSP4J itself (as opposed to a language server or client using LSP4J), I'm not even sure to what extent it is supported currently. (I have not ever had any need for trying it myself.) But I do note that we have a special Gradle task for Eclipse, but not e.g. for IntelliJ or for any other IDE. Does it mean anything? I don't know yet.

@pisv
Copy link
Contributor Author

pisv commented Apr 5, 2021

Also, building/editing LSP4J without requiring Eclipse IDE is also a requirement.

Jonah, can we just agree that if the command line build for LSP4J produces essentially the same artefacts (including binary and source bundles) as it does currently, this requirement will also be met? It seems necessary and sufficient to me.

@jonahgraham
Copy link
Contributor

@pisv It will be sufficient unless someone steps up to say something contrary.

I always do the dev in Eclipse because that is the best for me. But Maven+Tycho is a slow command line dev environment, no incremental builds, slow target resolution. Is anyone actively developing LSP4J in a way that would be substantially harmed by this move? Does gradle incremental builds work well? Are people developing in environments like VS Code / gitpod / IntelliJ / etc?

@spoenemann do you have any objection to moving to Eclipse standard Maven+Tycho? We still will produce the maven artefacts as they are today.

@cdietrich
Copy link
Contributor

cdietrich commented Apr 6, 2021

@jonahgraham
Copy link
Contributor

:-) I don't plan on doing this work anytime soon. @pisv if you take this on please consider @cdietrich question above.

@pisv
Copy link
Contributor Author

pisv commented Apr 6, 2021

:-) I don't plan on doing this work either until a consensus is reached on exactly what (if anything) shall be done.

@cdietrich please speak up if you already know some obstacles or if you have some objections, especially from your experience with Xtext (see the first comment). It was my understanding (please correct me if I'm wrong), after reading eclipse/xtext#1721, that there are some issues with using Gradle in Xtext, hence multiple proposals for moving to Maven/Tycho.

As mentioned in the first comment, I have almost no experience with Gradle, but also it was not my original intention to propose a move to Maven/Tycho. It just so happened that, in the course of the discussion with Jonah, I have proposed to "move to Maven/Tycho directly if that seems to be what we want to see strategically".

This issue is first and foremost about trying to enable Eclipse API Tools for LSP4J. Any suggestions on the best course of action would be most welcome.

Regarding

how to you plan to provide the maven dependencies currently inplace when using lsp4j

Please explain what issues you see with this. Is it known that it is not possible to achieve with Maven+Tycho (as opposed to plain Maven)? I'm not an expert in releng.

Thanks!

@cdietrich
Copy link
Contributor

right how if i add

org.eclipse.lsp4j.jsonrpc.debug

as a dependency to a gradle or maven project the dependencies will be automatically pulled.
if we just take the tycho artifacts and publish them to maven central then
the transitive dependencies will be not pulled as they are no longer in the maven metadata.

jdt and emf solve this problem by postprocessing and creating special metadata for maven central that reflect the dependencies from the manifest into the pom.

the problem with Xtext also stretches to Xtend and does not affect gradle only, thus i dont know e.g. when we will be able to provide a java 17 capable xtend-maven-plugin.

in Xtext we work two-fold. the projects are gradle, but we maintain the manifests manually so that we can use them to test with downstream p2 only code from eclipse

@cdietrich
Copy link
Contributor

Bildschirmfoto 2021-04-06 um 18 37 11

@pisv
Copy link
Contributor Author

pisv commented Apr 6, 2021

Thank you @cdietrich for the much valuable input to the discussion!

if we just take the tycho artifacts and publish them to maven central then
the transitive dependencies will be not pulled as they are no longer in the maven metadata.

I didn't know about it. Thanks for the explanation!

jdt and emf solve this problem by postprocessing and creating special metadata for maven central that reflect the dependencies from the manifest into the pom.

Good to know that there are already some precedents of solving this problem.

in Xtext we work two-fold. the projects are gradle, but we maintain the manifests manually so that we can use them to test with downstream p2 only code from eclipse

This is what I originally proposed for LSP4J. But how well does it work, in your experience? Would you recommend this setup?

E.g., I noticed, when recently playing with xtext-core set up as described in https://github.com/eclipse/xtext/blob/master/CONTRIBUTING.md#set-up-your-eclipse-workspace, that the projects, although they had the plug-in nature, did not have the ordinary Plug-in Dependencies PDE classpath container, using instead the Project and External Dependencies Gradle classpath container. But how does it interact with the active target platform? Does not it (sometimes) lead to some weird issues?

Thanks,

@pisv
Copy link
Contributor Author

pisv commented Apr 6, 2021

Also, did I understand it correctly that in Xtext the information about dependencies (including version ranges) is duplicated in MANIFEST.MF files and Gradle files and needs to be manually maintained in both places?

@cdietrich
Copy link
Contributor

yes we need to maintain both manually

@cdietrich
Copy link
Contributor

cdietrich commented Apr 6, 2021

But how does it interact with the active target platform?

we get some warnings/errors on the manifests if they dont fit to the tp.

@nixel2007
Copy link
Contributor

my 2cents about generating pom.xml.
if you deside to stick with gradle, you can generate/tune pom.xml via maven-publish plugin and its' gradle tasks:

https://github.com/1c-syntax/bsl-language-server/blob/develop/build.gradle.kts#L256-L271

@pisv
Copy link
Contributor Author

pisv commented Apr 7, 2021

Thank you for your replies @cdietrich and @nixel2007

@pisv
Copy link
Contributor Author

pisv commented Apr 7, 2021

Based on the feedback received so far, I would suggest the following alternatives:

  1. Do nothing about it. Consequently, live without Eclipse API Tools. UPDATE: Use japicmp instead as suggested in the next comment.

  2. Add plug-in nature to LSP4J projects to enable use of Eclipse API Tools and go the standard Eclipse PDE way:

    • Centralize the information about dependencies in manually maintained MANIFEST.MF and .target files

    • Use Maven+Tycho for the command line build (with a post-processing step for generating Maven dependency metadata, such as in EMF and JDT)

I'm no so sure about other alternatives such as Gradle/PDE combination used in Xtext; it will certainly add more complexity, while not allowing us to use Eclipse API Tools to the full extent (due to the duplication of dependency information in both Gradle and PDE).

As mentioned above, the main concern of this issue is enabling effective use of Eclipse API Tools for the LSP4J team, not ditching Gradle per se. The main goal of this issue is attempting to reach a consensus on what exactly (if anything) needs to be done.

Of course, there is no hurry in making the decision right now; this can be decided as part of preparation to the LSP4J 1.0.0 release or even later.

Thank you all for your feedback so far.

@cdietrich
Copy link
Contributor

cdietrich commented Apr 7, 2021

at Xtext we use japicmp to see at least what changes/breaks (after the fact, with a separate jenkins job that creates a report)

https://ci.eclipse.org/xtext/job/releng/job/xtext-xtend-api-diff/lastSuccessfulBuild/artifact/output/*zip*/output.zip

@pisv
Copy link
Contributor Author

pisv commented Apr 7, 2021

Thanks for the suggestion @cdietrich! This might be a reasonable alternative to using Eclipse API Tools.

@pisv
Copy link
Contributor Author

pisv commented Apr 9, 2021

Here is a decision tree that might be used for choosing between the two alternatives suggested in #542 (comment):

  1. If the ability to use IDEs other than Eclipse for development of LSP4J itself (as opposed to development of language servers/clients using LSP4J) is really important, choose the first alternative.

  2. Otherwise, if the ability to use Eclipse API Tools is really important for LSP4J development (probably, post version 1.0.0), choose the second alternative.

  3. Otherwise, choose the first alternative.

I've tried to use IntelliJ IDEA for LSP4J development. It works well enough. The initial import/build of LSP4J projects happens in almost no time (smoother than in Eclipse with Buildship). The incremental Gradle build does work well inside the IDE. There are no problems with running tests from within the IDE either. Of course, there is no syntax highlighting/rich editing/debugging support for Xtend. But this may change when/if an Xtend language server becomes available. I think that these findings can reasonably be generalized to other IDEs as well.

So, using IDEs other than Eclipse for development of LSP4J itself does seem possible currently, and it does seem important to preserve this ability. Otherwise, there is a risk of turning away some of the potential contributors.

As an aside, using japicmp would not only be IDE-agnostic, but could probably also allow us to have links to the generated API reports in the CHANGELOG. Since LSP4J is not a huge project in size, I believe that the absence of Eclipse API Tools would not be intolerable.

Therefore, I recommend choosing the first alternative as a safer way to proceed.

@jonahgraham
Copy link
Contributor

Therefore, I recommend choosing the first alternative as a safer way to proceed.

I think this is a good conclusion. I will add generating of a japicmp report to the release notes/changelog/somewhere as part of the next release.

@jonahgraham jonahgraham added this to the v0.13.0 milestone Apr 9, 2021
@jonahgraham jonahgraham mentioned this issue Apr 9, 2021
33 tasks
@pisv
Copy link
Contributor Author

pisv commented Apr 12, 2021

Thank you Jonah! It seems that a consensus has been reached among the participants of this discussion and there are no comments from others so far. Since this issue appears to have no actionable items anymore (japicmp being taken care of as part of #541), do you think it may be closed now?

@jonahgraham
Copy link
Contributor

do you think it may be closed now?

Yes.

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

No branches or pull requests

4 participants