search

saros-project / saros

Open Source IDE plugin for distributed collaborative software development
https://www.saros-project.org
GNU General Public License v2.0
160 stars 52 forks source link

[BUILD] Separate transitive dependencies in core #1124

Open FKHals opened 3 years ago

FKHals commented 3 years ago

so that the core can act as a dependency-container without leaking it's own (non-transitive) dependencies into the classpaths of the packages importing it while still providing the transitive dependencies that are needed by those packages.

This is implemented by creating the two new configurations "bundle" and "bundleApi" which replace the previously used "releaseDep". Dependencies of the "bundle"-configuration gets extend from the "implementation"-configuration (by the gradle-java-plugin) which is not transitive and therefore it's dependencies are only visible to the core package itself while the dependencies in "bundleApi" (which the corresponding "api"-config extends from) can be used by all packages that import the core. Extension in the context of configurations means that that all the dependencies in the "bundle"-config also get added to the "implementation"-config.

This solution has the advantage to other solutions (e.g. creating a new OSGI package) that the complexity of the build system is not significantly increased and no versioning of any additional OSGI packages needs to be considered.

The solution was proposed and kindly explained to the authors by @m273d15 following the PR #1118.

FKHals commented 3 years ago

Nice! Your PR contains only the Gradle changes. I would expect that you also need to adjust the META-INF/MANIFEST.MF file and remove entries that export libraries that are not part of the API (for example commons-codec in the core). Do you plan follow-up PRs that get rid of the remaining API/exported dependencies (for example commons-io)?

We have done the proposed fixes (and also moved xstream, jmdns and xpp3 to bundle instead of bundleApi) including the removal of the unnecessary exports in the MANIFEST.MF. Our goal is to contain all the changes in this (hopefully coherent) commits so we do not plan any follow ups. Anything that we have not done yet is probably an error/oversight on our side and we are glad to fix it if more problems get pointed out.

Since commons-io seems to get used by the server (in server/src/saros/server/editor/Editor.java and others) we did not think it would be possible to also exclude it from the exported dependencies. Or does the server somehow get its dependencies differently?

m273d15 commented 3 years ago

Since commons-io seems to get used by the server (in server/src/saros/server/editor/Editor.java and others) we did not think it would be possible to also exclude it from the exported dependencies. Or does the server somehow get its dependencies differently?

You could simply add the dependency as a Gradle dependency of the server project. This is the intended way of handling dependencies. Instead of relying on the transitive dependencies of another project, the server defines its own dependencies.

FKHals commented 3 years ago

We just made the changes so that commons-io won't get exported by the core and instead gets imported by every project for itself. We are still testing/researching if this could be done with commons-lang too, but the tests still fail at the moment.

Edit: For some reason the STF tests of both commits seem to fail..

FKHals commented 3 years ago

Is there any reason why the commons-io-entries still need to be listed in Export-Packages for the STF-tests to work even if the commons-io-package itself is in the cores bundle-configuration and therefore should not be exposed anyways?

I just confirmed on a testing branch at our fork that this seems to be the case since the tests succeed when the Export-Packages-entries are present.