-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
JAVA-5452 #1378
base: scala3
Are you sure you want to change the base?
JAVA-5452 #1378
Conversation
Implement scala version support via standard gradle build conventions JAVA-5452
@@ -1916,15 +1916,15 @@ axes: | |||
- id: "2.11" | |||
display_name: "Scala 2.11" | |||
variables: | |||
SCALA: "2.11.12" |
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.
libs.versions.toml
now handles the scala library versioning. So this is now just for which scala release to test against.
import config.Extensions.scalaVersion | ||
import config.Extensions.setAll | ||
|
||
plugins { id("conventions.scala") } |
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.
Main scala configuration is done by using buildSrc
conventions
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
import config.Extensions.scalaVersion |
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.
Some buildSrc
extension functions to make life easier.
|
||
@RunWith(classOf[JUnitRunner]) |
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.
No longer needed thanks to the scala test configurations done in the scala convention.
@@ -530,6 +530,7 @@ class MacrosSpec extends BaseSpec { | |||
} | |||
|
|||
it should "support tagged types in case classes" in { | |||
assume(!scala.util.Properties.versionNumberString.startsWith("2.11")) |
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.
This is failing to compile with Scala 2.11 - not sure how it worked with the previous plugin as its a scala error. Its possible that we are using a newer scala 2.11 version patch which highlights this error.
As Scala 2.11 has been EOL for a long time so I'm not sure its worth investigating further.
try { | ||
directory = new File(resource.toURI()); | ||
} catch (IllegalArgumentException e) { | ||
directory = new File(resource.toExternalForm()); |
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.
Resources may now come from test jars so use the external form in that case.
@@ -106,68 +106,6 @@ configure(javaProjects) { | |||
|
|||
} | |||
|
|||
configure(scalaProjects) { |
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.
The root build.gradle no longer handles the configuration of scala projects.
} | ||
|
||
dependencies { | ||
implementation(libs.spotless.plugin) |
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.
Conventions now configure the spotless and logger plugins for the projects that use them.
plugins { | ||
`kotlin-dsl` | ||
// When updating, update in libs.versions.toml | ||
id("com.diffplug.spotless") version "6.25.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.
The buildSrc project also uses spotless for formatting. Unfortunately, at the plugin stage we can't access the version from libs.versions.toml
.
implementation(files(libs.javaClass.superclass.protectionDomain.codeSource.location)) | ||
} | ||
|
||
spotless { |
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.
The spotless configuration for buildSrc
files only.
@@ -13,22 +13,13 @@ | |||
* See the License for the specific language governing permissions and | |||
* limitations under the License. | |||
*/ | |||
@Suppress("UnstableApiUsage") | |||
dependencyResolutionManagement { versionCatalogs { create("libs") { from(files("../gradle/libs.versions.toml")) } } } |
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.
Allows access from within the convention to the libs.versions.toml
import org.gradle.api.Project | ||
import org.gradle.api.plugins.ExtraPropertiesExtension | ||
|
||
object Extensions { |
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.
Handy extension functions that make life easier
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package conventions |
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.
A convention to add integrationTesting
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package conventions |
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.
A convention to support publishing
* limitations under the License. | ||
*/ | ||
|
||
package conventions |
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.
A convention to support scala.
Note it builds upon other plugin extensions! (publishing, testing, spotless).
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package conventions |
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.
A convention to create test artifact jars. This provides support for test resources / classes to be consumed across projects using the kotlin dsl.
Previously, we'd use the following gradle syntax:
testImplementation project(':bson').sourceSets.test.output
testImplementation project(':driver-core').sourceSets.test.output
testImplementation project(':driver-sync').sourceSets.test.output
This isn't supported in the kotlin dsl. Infact sourceSets
is even greyed out in intellij with the groovy. So its unclear how this is resolved.
There are test fixtures in gradle now - but I'm not sure they meet our requirements (and require their own project layout conventions).
@@ -17,6 +17,7 @@ | |||
|
|||
plugins { | |||
id 'com.github.gmazzo.buildconfig' version '3.0.3' | |||
id('conventions.testArtifacts') |
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.
Configures driver-core
to produce test artifacts that can be consumed by other projects (eg the Scala project).
|
||
base.archivesName.set("mongo-scala-driver") | ||
|
||
extra.setAll( |
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.
Configures extra properties, which are consumed by the various conventions - mainly the publishing convention.
|
||
plugins { | ||
id("conventions.scala") | ||
id("conventions.integrationTesting") |
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.
Adds integration testing support to the driver-scala project.
|
||
"integrationTestImplementation"(project(path = ":driver-sync")) | ||
"integrationTestImplementation"(project(path = ":driver-reactive-streams")) | ||
"integrationTestImplementation"(project(path = ":bson", configuration = "testArtifacts")) |
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.
Here we define the test artifacts that are needed to run the tests. eg. resources files and JsonPoweredTestHelper.
@@ -16,8 +16,8 @@ | |||
|
|||
org.gradle.daemon=true | |||
org.gradle.jvmargs=-Duser.country=US -Duser.language=en | |||
scalaVersions=2.11.12,2.12.15,2.13.6 | |||
defaultScalaVersions=2.13.6 | |||
supportedScalaVersions=2.13,2.12,2.11 |
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.
Use an explicit naming scheme for our own scala convention.
@@ -19,28 +19,12 @@ def deployedProjects = subprojects - utilProjects | |||
|
|||
configure(deployedProjects) { | |||
|
|||
def isScala = project.name.contains('scala') |
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.
This was all disabled anyway - so have removed. I've also closed JAVA-3564 as won't fix, its not possible with a single gradle run.
@@ -116,43 +116,6 @@ configure(javaProjects) { project -> | |||
} | |||
} | |||
|
|||
configure(scalaProjects) { project -> |
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.
The publishing convention now handles scala publishing.
@@ -1,5 +1,5 @@ | |||
distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-7.3-bin.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.4-bin.zip |
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.
Gradle bump
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.
Not quite done but here's a first round.
The main sanity check I did so far was to diff every MANIFEST.MF and pom.xml and look for significant differences. I found one significant one, a couple of perhaps significant ones, and a few nits.
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.
LGTM
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.
Two commits:
build.gradle
.Scala is now only configured by the project build file eg:
bson-scala\build.gradle.kts
&bson-driver\build.gradle.kts
Part of the decoupling required the creation of
buildSrc
conventions. See: https://docs.gradle.org/current/userguide/organizing_gradle_projects.html#sec:build_sourcesNote: My current plan is this will merge into a
scala3
branch and won't makemaster
until all the scala 3 work is done.JAVA-5452