Skip to content
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

Open
wants to merge 4 commits into
base: scala3
Choose a base branch
from
Open

Java 5452 #1378

wants to merge 4 commits into from

Conversation

rozza
Copy link
Member

@rozza rozza commented May 3, 2024

Two commits:

  1. Bump gradle to the latest 7.6 release.
  2. Decouple scala multi version plugin and remove Scala from the root 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_sources

Note: My current plan is this will merge into a scala3 branch and won't make master until all the scala 3 work is done.

rozza added 2 commits May 3, 2024 10:40
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"
Copy link
Member Author

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") }
Copy link
Member Author

@rozza rozza May 3, 2024

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
Copy link
Member Author

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])
Copy link
Member Author

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"))
Copy link
Member Author

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());
Copy link
Member Author

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) {
Copy link
Member Author

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)
Copy link
Member Author

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"
Copy link
Member Author

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 {
Copy link
Member Author

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")) } } }
Copy link
Member Author

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 {
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

@rozza rozza May 3, 2024

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
Copy link
Member Author

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')
Copy link
Member Author

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(
Copy link
Member Author

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")
Copy link
Member Author

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"))
Copy link
Member Author

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
Copy link
Member Author

@rozza rozza May 3, 2024

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')
Copy link
Member Author

@rozza rozza May 3, 2024

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 ->
Copy link
Member Author

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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gradle bump

@rozza rozza requested review from a team, vbabanin and jyemin and removed request for a team May 3, 2024 11:18
@rozza rozza changed the base branch from master to scala3 May 3, 2024 11:19
@rozza rozza marked this pull request as ready for review May 7, 2024 12:35
Copy link
Contributor

@jyemin jyemin left a 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.

bson-scala/build.gradle.kts Show resolved Hide resolved
driver-scala/build.gradle.kts Outdated Show resolved Hide resolved
bson-scala/build.gradle.kts Outdated Show resolved Hide resolved
driver-scala/build.gradle.kts Outdated Show resolved Hide resolved
buildSrc/src/main/kotlin/config/publishing.kt Outdated Show resolved Hide resolved
buildSrc/src/main/kotlin/config/publishing.kt Show resolved Hide resolved
gradle/libs.versions.toml Show resolved Hide resolved
settings.gradle.kts Show resolved Hide resolved
.evergreen/publish.sh Show resolved Hide resolved
bson/src/test/unit/util/JsonPoweredTestHelper.java Outdated Show resolved Hide resolved
@rozza rozza requested a review from jyemin May 9, 2024 10:59
@rozza
Copy link
Member Author

rozza commented May 10, 2024

@rozza rozza requested a review from jyemin May 10, 2024 13:57
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants