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

Upgrade to Java 11? #1114

Closed Drakulix closed 2 years ago

Drakulix commented 3 years ago

Saros/E fails to build with JDK 8 as seen by our pipeline.

 > Task :saros.eclipse:compileJava FAILED
import org.eclipse.ui.plugin.AbstractUIPlugin;
                            ^
  bad class file: /home/runner/.gradle/caches/modules-2/files-2.1/org.eclipse.platform/org.eclipse.ui.workbench/3.122.0/60c038f66a106d3d437e66d6109bb42657eec6b4/org.eclipse.ui.workbench-3.122.0.jar(org/eclipse/ui/plugin/AbstractUIPlugin.class)
    class file has wrong version 55.0, should be 52.0
    Please remove or make sure it appears in the correct subdirectory of the classpath.
Note: Some input files use or override a deprecated API.

Steps to reproduce the behavior:

  1. Clone saros
  2. If necessary clean you build folder / global gradle cache
  3. Run ./gradlew prepareEclipse sarosEclipse
  4. See error

Environment:

Additional context I suspect a version / dependency mismatch of some sort. The pipeline did build with the same commit days ago, some students first noticed the issue and informed me about it.

Building with Java 11 (as indicated by class file has wrong version 55.0, should be 52.0) fixes the problem, maybe the downloaded eclipse platform now defaults to a new version? So far I was not able to pin down the problem.

tobous commented 3 years ago

Uhhh, Eclipse is now also finally building with Java 11 since release 4.17 (2020-09). See wiki. So we can revive the discussion on whether we want to upgrade to Java 11 or not.

Updating to Java 11 would potentially reduce the usability of Saros as users not on the newest release would have to manually configure Eclipse to run using Java 11. But this is still worth it in my opinion, just to keep up with the times, now that we finally have the opportunity.

Migrating the GitHub Actions and documentation is no problem, I could create a PR for that. Updating the actual build configuration is a bit more complicated. We could just require the user setting the correct java home environment variable before building, but requiring Java 11 as part of the configuration would be preferable in my opinion. Changing this in the configuration is kind of cumbersome as I haven't found a way to configure it locally in the base build file (root build.gradle.kts). And specifying it in every build file (for Core, Eclipse, IntelliJ, Server, etc.) seems sub-optimal. But, to be honest, I am still not familiar with the new gradle configuration after moving to the "plugin" structure and the kotlin configuration file and I find it hard to find good documentation to understand it better. (But I also haven't invested a lot of time into it.) So there might be a way of configuring it in the root build file, but I couldn't figure it out.

@Drakulix @srossbach What do you think about moving to building with Java 11?

tobous commented 3 years ago

Also, this is the second time that changes of newer Eclipse releases cause issues for our build. The base build file specifies that we want to use Eclipse 4.8 (see here), but this configuration does not seem to be honored for the downloaded Eclipse sources (at least not for all). But I also lack the knowledge to properly investigate this further.

FKHals commented 3 years ago

I may have found the root of the problem: The Maven-Repository which distributes the eclipse packages states that the last update of the org.eclipse.ui.workbench-class has been on 16th December 2020 which correlates with the appearance of the bug at our own project (which uses a mostly identical build-system).

I have not yet found out how to force a certain version to be used for compile dependencies (in this case org.eclipse.ui.workbench is the compile dependency of org.eclipse.ui which i would want to downgrade to a previous version). So i just copied an older version (3.120.0 to be exact) of the potentially problematic jar-file into the cache in place of the new version 3.122.0 and it successfully compiled with Java 8 (Yes, that was filthy but it did the job).

If my set-up was correct, this should isolate the problem so a possible solution would be to force the use of an older version of the org.eclipse.ui.workbench-dependency but i am not sure if that is desirable.

srossbach commented 3 years ago

JDK 9 made reflection changes. Has anyone actually tried to run Saros in a JDK > 8 ?

tobous commented 3 years ago

JDK 9 made reflection changes. Has anyone actually tried to run Saros in a JDK > 8 ?

Well, at least it builds on master and the test's don't explicitly fail (although I saw some stdout errors when skimming the tests). This is tested through the Java 11 action every week. You can have a look at the build results here. But I haven't tested actually using the plugin for Eclipse or IntelliJ IDEA. Additionally, as the docker container are set up using JDK 8, I can't run the STF tests on the server to check it (and I don't feel like blocking my laptop for 40 minutes).

tobous commented 3 years ago

Smoke test with Saros/I build on Java 11 was successful (session starts successfully, text edits are synchronized, and awareness information is correctly displayed). So Saros/I and the Saros Core seem to work. The unexpectedly high amount of STANDARD_ERROR results of different Core test with various issues can be recreated when running the tests with Java 8.

m273d15 commented 3 years ago

Oh wow, this issue again. We already had the same situation with #1086.

My current knowledge about the dependency resolution process is:

My assumption is that the transitive dependencies defined in the pom files of the artifacts in maven central lead to the issue. They define dependency version ranges:

<dependency>
      <groupId>org.eclipse.platform</groupId>
      <artifactId>org.eclipse.core.runtime</artifactId>
      <version>[3.2.0,4.0.0)</version>
</dependency>

This allows gradle to resolve newer versions and since the goomph plugin does not define further version restrictions/preferences the newer version is used as transitive dependency and in the classpath.

I think there are multiple ways to fix this issue:

Drakulix commented 3 years ago

@Drakulix @srossbach What do you think about moving to building with Java 11?

In principle I am all for it. But as @srossbach pointed out, we should test this first. Although I think passing STF-Tests for Java 11 would be enough to switch. A full round of testing would be done before any proper release anyway.

Also, this is the second time that changes of newer Eclipse releases cause issues for our build.

Oh wow, this issue again. We already had the same situation with #1086.

If this a frequent problem, we should look into solving this long term anyway. Newer eclipse platform packages should not be easily able to break our build. This is especially cumbersome for newcomers, which do not have old versions cached and might assume a problem on their side given our somewhat complicated build process.

As a stop-gap we could deploy the same work-around, that was used in #1086. I went ahead and created a draft-PR for anyone that need it: #1115

I think there are multiple ways to fix this issue:

* Use the goomph plugin classes to re-implement the bundle dependency resolution.

* OR Use a dependency resolution strategy that enforces the corresponding version

* OR Contribute to the goomph project and add the feature to add dependencies with version preferences.

I am in favor of forcing a tested "maximum"-version, but I have no knowledge about which of these proposals is best.

We still should seriously consider moving to Java 11 at this point, as any newly released org.eclipse.platform-package will break this again until we either pin all versions (and block us out of future updates) or have migrated.

tobous commented 3 years ago

Running the STF proved harder than I thought. Using the docker container after updating to a JDK 11 image works, but it fails so spectacularly that even a runtime of 2 hours isn't enough (as as the result of the premature termination, it does not give any results or an overview of the encountered failures). And running it through Eclipse is not an option as far as I can tell as Eclipse Photon (4.1.8), i.e. the Eclipse version we are currently using for testing, does not seem to support Java 11. And with newer Eclipse versions there are changes to the Eclipse UI framework leading to issues with the STF tests if I remember correctly.

So I am not really sure how to run the STF on for Java 11 builds.

m273d15 commented 3 years ago

Running the STF proved harder than I thought. Using the docker container after updating to a JDK 11 image works, but it fails so spectacularly that even a runtime of 2 hours isn't enough (as as the result of the premature termination, it does not give any results or an overview of the encountered failures). And running it through Eclipse is not an option as far as I can tell as Eclipse Photon (4.1.8), i.e. the Eclipse version we are currently using for testing, does not seem to support Java 11. And with newer Eclipse versions there are changes to the Eclipse UI framework leading to issues with the STF tests if I remember correctly.

I found only one bug in the SWTBot's issue tracker concerning JDK 11 (here) and I don't know if we make use of the affected feature. However, we are currently using SWTBot 2.7.0 (3.1.0 is the new version) and I don't know if 2.7.0 supports the latest Eclipse versions that supports JDK 11.

@tobous have you also updated the eclipse version in the docker container and verified that the rmi server starts?

tobous commented 3 years ago

@tobous have you also updated the eclipse version in the docker container and verified that the rmi server starts?

No, I only updates the docker image to use JDK 11 (as I couldn't find any official opendkj alpine images for JDK 11, I used an adoptjdk one). I did not think updating the Eclipse version for the images would be necessary. Even though the Eclipse version itself does not recognize Java 11 for building, I assumed that it should still run on Java 11.

The base test setup still seems to work as at least some of the tests ran successfully.

tobous commented 3 years ago

Manual smoke test using a newer Eclipse version (2019-09) succeeded as well. So either there are issues with the test setup or there are more intricate/non-obvious issues caused by the move to Java 11.

stefaus commented 2 years ago

Hello all!

After no one had worked on Saros for a long time, the build pipeline was even more broken. With #1156 I merged building with JDK 11, which fixes the problems good enough that my smoke tests and STF Tests on CI are working. Next Step will be to Update eclipse to a more recent version.