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

Embed the google cache dependency in the bundle #1703

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Nov 10, 2024

Guava is known as a bit "problematic" dependency, as lemminx only uses guava-cache, this can be embedded in the bundle and be made embedded package.

This changes the used felix-bundle to bnd-maven-plugin with the conditionalpackage to include the caching api in the generated bundle.

FYI @mickaelistria I just choose this a startingpoint because with

this type does not appear in any public (exported) API anymore, if it could be agreed that this is a way to go I'll try to identify other possible types to embed as well.

@mickaelistria
Copy link
Contributor

Can you please clarify what is "the bundle" here? Is it the uber-jar?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 12, 2024

If you run the build you get these artifacts (as of today):

  1. org.eclipse.lemminx-0.28.1-SNAPSHOT.jar that is a bundle and what I'm referring/changing here
    2. org.eclipse.lemminx-0.28.1-SNAPSHOT-tests.jar that includes the tests
  2. org.eclipse.lemminx-uber.jar includes all dependencies

Guava is known as a bit "problematic" dependency, as lemminx only uses
guava-cache, this can be embedded in the bundle and be made embedded
package.

This changes the used felix-bundle to bnd-maven-plugin with the
conditionalpackage to include the caching api in the generated bundle

Signed-off-by: Christoph Läubrich <[email protected]>
@mickaelistria
Copy link
Contributor

I would rather not touch the default artifact/bundle as the proposal is adding some complexity while the artifact/bundle is itself working well.
If the story is still to improve consumption of lemminx in wild web developer, can't such tweaks only take place in the uber-jar ?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 12, 2024

The idea is to make the dependency requirements of the bundle less a concern and at some point make the uber-jar not required for usual deployments especially the ones you mentioned here

[...] may fail in other deployments because of undocumented dependency ranges. In the current state, LemMinX is totally unable to support such "exotic" deployments.
Some LemMinX deps are/were not OSGi-ready

So even though google-cache is OSGi it falls in the first category and always produces some interoperability problems if someone uses a quite narrow range. As it is not part of any public API its a good candidate for embedding (see rationale here) and as everything is automatically handled by BND in this case I don't see much complexity (for the developers/users)

while the artifact/bundle is itself working well.

Can you give examples where lemminx bundle is actually used inside an OSGi deployment?

If the story is still to improve consumption of lemminx in wild web developer, can't such tweaks only take place in the uber-jar ?

The problem is that the uber-jar includes much more I also found here:

that even a very simple approach requires never maven, maybe even never build JVM, even though it might be solvable.

So currently I somehow tend to maybe make the "real" bundle more easily deployable and less exposed to some kinds of dependency issues (e.g. embedd as much as possible at that stage already).

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.

2 participants