Add Scala support

[Arthurm1]

Implements buildTarget/scalacOptions. Also returns ScalaBuildTarget instead of JvmBuildTarget if Scala has been configured for that target and passes back scala as a supported language.

I haven't separated out the implementation per language. I think a refactor would be better done in a single PR (not one where a new language is added). I'm not sure it's currently worth refactoring. Maybe when more languages are added?

Creating the scala compiler args using ZincScalaCompilerArgumentsGenerator is a bit verbose because of the class it requires to extract the setup from. It only actually needs 2 methods to be implemented. Another option would be to copy the code directly from ZincScalaCompilerArgumentsGenerator into SourceSetsModelBuilder. I thought this would be easier to maintain if the original is changed.

[jdneo]

I'm not sure it's currently worth refactoring. Maybe when more languages are added?

To me, I would like to have a refactoring at current stage. Because:

• The current GradleSourceSet.java has become a large object with a lot of fields.
• If we defer the refactoring to the third language comes, things will be even more complex than now.

I'm sorry that this is a debt that I missed when implementing.

About the refactoring, I'm thinking that

• We can have an extensibility that allows third-party plugin to provide their language build targets.
• For the current embedded plugin, it can have different model builders taking care of their own language models.

We can do the second refactoring first to unblock the scala support.

The rough idea in my mind is:

• the plugin will register the model builder according to the languageIds of the client capability.
• The Gradle source set holds the common shared data.
• The Gradle source set has an extension that holds language specific data. (like Scala version)

WDYT?

[Arthurm1]

the plugin will register the model builder according to the languageIds of the client capability. The Gradle source set holds the common shared data. The Gradle source set has an extension that holds language specific data. (like Scala version)

It sounds OK. I'm happy to code around your design, I'm just don't know whether that's the right direction as I haven't seen enough different language setups in Gradle to make a decision on splitting the design across languages.

One big advantage here is that I doubt anyone will be using this project as a library (it doesn't make sense to me to do that) so I think you can change the design/API as and when you like without affecting downstream projects.

Just some thoughts, not blockers... 1) A single Build Target can have multiple language types so whatever these extensions are, the common gradle source set would need to be mapped to extensions 1->many, not 1->1. E.g scala can mix and match java and scala code within the same gradle project configuration. It would need both Scala extension info and Java extension info. So I wouldn't like to see the extension classes inherit from the common info class.

2) We might want to build the common set regardless of whether the languages are supported by the BSP client. One example I can think of is that a multi-project might have mostly Java subprojects but have the test projects written in Kotlin. A Java only client could still run these tests without knowing anything about the Kotlin language and debug into the Java code. The BSP compile/run/test/debug requests are all language agnostic.

3) Would this work nicely for Android? I've written some code to extract Gradle Android info before here. Annoyingly it ignores the Gradle sourceset structures and defines its own. So the common shared data class might not actually include sourcesets (but would include dependencies/modules/names etc). Would the Android extract be registered by language? I'm not sure that makes sense as the extracted sourcesets are still Java/Scala/Kotlin. Unless the new structure allows multiple ModelBuilders per language. AFAIK - you have to register your ToolingModelBuilder by the class it produces so I guess you'd have basic wrapper classes e.g. AndroidCommon, AndroidJava, AndroidScala, AndroidKotlin which just wrap the extension classes written for those languages?

[jdneo]

Thank you @Arthurm1 for your input.

Here is my plan: I'll first make sure those opened PR get merged recently. At the meantime, I'll find some time think thoroughly about how to do the refactoring. We can continue the discussion here.

[jdneo]

Annoyingly it ignores the Gradle sourceset structures and defines its own. So the common shared data class might not actually include sourcesets

I'm not very familiar with Android, which kind of property of the source set is not aligned when it comes to Android projects?

[jdneo]

@Arthurm1 I now have a feeling that, maybe the BSP related types should be constructed at the plugin side, and the server is just simply get them from plugins and return them when different requests come.

Currently the server needs to know the definition of the source set and parse them to build target, etc... This makes trouble as you've mentioned, the concept of source set does not exist for Android, not to say other languages.

[Arthurm1]

I now have a feeling that, maybe the BSP related types should be constructed at the plugin side

Maybe - I don't think that's a bad idea - I'm just not sure yet.

Android does have the concept of sourcesets - it just has it's own hierarchy. So instead of org.gradle.api.tasks.SourceSet it has com.android.build.api.dsl.AndroidSourceSet

What I do think is that this data might have to be cached. Have you tried the plugin on a big project? e.g. Gradle itself. It takes about a minute to respond to build/initialize (which I think should be instant and is slow because the source sets are queried in response to this request but not yet needed). Then, after a compile, the source sets are all queried again. It would be good to delay the building of the sourcesets until the client requests them and do they need to be built again after every compile?

[jdneo]

About the perf issue, let's discuss at https://github.com/microsoft/build-server-for-gradle/issues/118.

Android does have the concept of sourcesets - it just has it's own hierarchy. So instead of org.gradle.api.tasks.SourceSet it has com.android.build.api.dsl.AndroidSourceSet

Does that mean that for android project, we need a different way to fetch the information of the source set, but the properties of the source set are the same?

[Arthurm1]

Does that mean that for android project, we need a different way to fetch the information of the source set, but the properties of the source set are the same?

I think so.

[jdneo]

@Arthurm1 I have got some ideas about the refactoring. I'll make a draft these days and let you review it.

[jdneo]

@Arthurm1, Sorry bro for the long time hanging. Would you like to continue the work based on the latest code base?

[Arthurm1]

@jdneo In the first commit, I've refactored for the new multi-language setup. The only thing of note is I've moved compileClasspath back into GradleSourceSet as it seems to be common across all languages.

In the second commit, I've refactored the extension conversion. I've managed to make it so the conversion to a JavaExtension or ScalaExtension is done once in the DefaultGradleSourceSet constructor. It still uses reflection sadly.

The only requirement after that is the casting of Object to JavaExtension or ScalaExtension. I've pushed this into a new SupportedLanguage interface so that it's not possible to mismatch language and extension like Conversions.toJavaExtension(gradleSourceSet.getExtensions().get(SupportedLanguages.SCALA));

See what you think

[Arthurm1]

@jdneo If you're happy with this refactoring then I'll refactor the Kotlin PR