Skip to content

Modularize project #244

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

Closed
wants to merge 3 commits into from
Closed

Modularize project #244

wants to merge 3 commits into from

Conversation

IceBlizz6
Copy link
Contributor

@IceBlizz6 IceBlizz6 commented Apr 19, 2020

Now introducing java modules.

  1. Added java modularity plugin to gradle
    • as gradle doesn't support modules by default
  2. Updated to java 9
  3. Added module-info files to both projects
  4. Updated bnd to exclude graphql.kickstart.execution from graphql-java-servlet jar
  5. Travis now uses jdk11
    • Tried jdk9, but version 1.5 of module plugin seems to require 11

As for the bnd change i'm not sure if this was the right way to do it.
The main goal was to avoid graphql-java-servlet to include execution packages into the jar, if there is a better way to do this then please let me know.

Any project that references graphql-java-servlet will also end up with graphql-java-kickstart, even though they dont reference this directly.
graphql-java-servlet has all packages that are also in graphql-java-kickstart, this creates a split package for the maven project that tries to reference them.

The error ends up as:
java.lang.module.ResolutionException: Module graphql.kickstart.servlet contains package graphql.kickstart.execution.error, module graphql.kickstart.execution exports package graphql.kickstart.execution.error to graphql.kickstart.servlet

By excluding execution packages from servlet jar this error is avoided.

@IceBlizz6 IceBlizz6 marked this pull request as ready for review April 19, 2020 20:15
@IceBlizz6
Copy link
Contributor Author

IceBlizz6 commented Apr 19, 2020

@oliemansm
To continue our discussion from the other thread about the jar issue:
If i create a new maven project and add this as the only dependency:

<dependency>
    <groupId>com.graphql-java-kickstart</groupId>
    <artifactId>graphql-java-servlet</artifactId>
    <version>9.1.0</version>
</dependency>

Then i end up with this:
image
You can see here that servlet drags with it the other kickstart project jar, and together they create a conflict as they both have the same packages.

If i understood your point correctly, you said that servlet is the "leaf project", meaning projects will only add and use this as dependency, then it shouldn't matter whatever that project is using further down.
In practice this doesn't seems to work as it is configured now, Projects that add this dependency gets both projects and conflicting packages with it.

The bnd file change fixes this issue

@oliemansm
Copy link
Member

I don't get that... graphql-java-servlet is a multi-module gradle project consisting of two modules. That's because the kickstart module contains some shared code that's used by other libraries too. the graphql-java-servlet module does not contain a graphql.kickstart.execution package (look at the code). graphql-java-servlet does depend on graphql-java-kickstart module, so it does indeed pull that in. But then I'd expect just the top part of your screenshot to be visible, not the bottom part. So I don't get where those packages come from in this screenshot.

On another note... The main issue is that these project don't play nice in a Java 9 modular setup and the idea is to make it compatible with it so it can be used in those projects. However the way it's being made compatible now is by upgrading the entire thing to Java 9 making it incompatible with anyone still using Java 8. Java 8 is still the most widely used version of Java today, so we can't make this library incompatible with that.

@IceBlizz6
Copy link
Contributor Author

the graphql-java-servlet module does not contain a graphql.kickstart.execution package (look at the code).

You are correct here, the graphql-java-servlet does indeed NOT contain the graphql.kickstart.execution package, but even so... it does copy this package into the output jar.
I have never used bnd before so i cannot make an explanation for why it does this.
It was mostly by pure chance that i tried to edit the bnd file and got a configuration where graphql-java-servlet did not copy the execution package.
I suspect this may simply have been an error in the configuration (?)

As for java 8 compatibility i will look into a solution for this.
https://dzone.com/articles/how-to-create-modular-jars-that-target-a-java-rele
Based on this link it looks like it may be possible to compile with java 8 and then insert the module-info file into the jar after.

@oliemansm
Copy link
Member

Nice catch for the bnd module! That's something specific for OSGi (which I don't use myself). I'm actually not sure if that's an error in the configuration or not, but it does sound like it.

@oliemansm
Copy link
Member

Hi @IceBlizz6. Did you find any time to look into that Java 8 plugin solution to make it backwards compatible?

@oliemansm
Copy link
Member

oliemansm commented Nov 19, 2020

@IceBlizz6 There have been some updates to the bnd configuration too in the meantime. Does the latest released version of this library still cause problems with modularized projects requiring these changes or is this no longer relevant?

@federicorispo
Copy link
Member

@IceBlizz6 Is this still necessary?

@IceBlizz6
Copy link
Contributor Author

@IceBlizz6 Is this still necessary?

We are not using this project anymore,
I'm closing it.

@IceBlizz6 IceBlizz6 closed this Dec 19, 2023
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.

3 participants