-
Notifications
You must be signed in to change notification settings - Fork 223
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
Upgrade to Eclipse 4.30.0 #1357
Conversation
This newer Eclipse release requires Java 17 or better. However, we still prefer to keep most of WALA compatible with Java 11. Therefore, this Eclipse upgrade adds nontrivial build infrastructure to let us use a newer Java release only where Eclipse dependencies require it. A new `com.ibm.wala.jdk-version` property in `gradle.properties` allows setting the default Java version used throughout WALA. If this property is set to 17 or higher, then that same version is used both for Eclipse-dependent and Eclipse-independent WALA components. However, if this property is set to 16 or earlier, then it is only used for WALA's Eclipse-independent components; the Eclipse-dependent components will override this and use Java 17 instead. In a default configuration, most of WALA will be compiled with (and for) Java 11, with the Eclipse-dependent components using Java 17 instead. Resolves wala#1354.
70b3deb
to
bad0b1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this change! I have one specific comment below.
Also, so I understand at a high level, with this change, the Java version that Gradle is running on becomes irrelevant in terms of what JDK is used to compile WALA code and run the tests; that is entirely controlled by the new property in gradle.properties
with customization for Eclipse projects. Is this correct?
plugins { id("com.diffplug.configuration-cache-for-platform-specific-build") version "3.44.0" } | ||
plugins { | ||
id("com.diffplug.configuration-cache-for-platform-specific-build") version "3.44.0" | ||
id("org.gradle.toolchains.foojay-resolver-convention") version "0.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit hesitant to add this in on another project, since it downloads JDKs and then runs them. But I guess since it comes from Gradle, it's probably pretty trustworthy? The alternative to using this would just be documenting which JDK versions need to be installed and installing them ourselves on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's trustworthy. It's also widely used, so I'd expect any vulnerability to be discovered quickly. Certainly, this is far from being the only code we download from the Internet and run as part of building and testing WALA.
The alternative to using this would just be documenting which JDK versions need to be installed and installing them ourselves on CI.
Agreed. But I prefer an automated mechanism over prose instructions that each new WALA developer must manually apply.
That being said, manual pre-installation of suitable JDKs is still an option. If someone is working on WALA in a restricted environment that would prevent toolchain auto-provisioning from working, that's fine. WALA's Eclipse components will build using whatever JDK 17 Gradle can find already installed on the system or custom-configured.
Yes, that's exactly right. Note also that this property can be set on the |
Thanks a lot, @liblit, everything looks good here. Right now I don't think we have any docs on what Java versions are required to run WALA. Do you think this belongs in the main README.md? I'm wondering if we could briefly document that in this PR. Alternately, we could do it in a separate PR before cutting the next release. |
Sure, no harm in letting people know what to expect. I'll add that to the main |
de424eb
to
ab840ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this!
You're welcome! It was a fun challenge. |
This newer Eclipse release requires Java 17 or better. However, we still prefer to keep most of WALA compatible with Java 11. Therefore, this Eclipse upgrade adds nontrivial build infrastructure to let us use a newer Java release only where Eclipse dependencies require it.
A new
com.ibm.wala.jdk-version
property ingradle.properties
allows setting the default Java version used throughout WALA. If this property is set to 17 or higher, then that same version is used both for Eclipse-dependent and Eclipse-independent WALA components. However, if this property is set to 16 or earlier, then it is only used for WALA's Eclipse-independent components; the Eclipse-dependent components will override this and use Java 17 instead.In a default configuration, most of WALA will be compiled with (and for) Java 11, with the Eclipse-dependent components using Java 17 instead.
Resolves #1354.