Allow compilation of build scripts against a different Gradle API version #20373

wolfs · 2022-04-05T11:45:29Z

This PRs explores what would be necessary to compile build scripts against a different Gradle API. It allows passing the Gradle API version via -Dorg.gradle.api.version=6.9 to the Gradle installation. Note that you also need a repository which contains an artifact for the Gradle API of this version with the coordinates org.gradle:gradle-api:<version>. You can specify the URL of the repository via -Dgradle.api.repository.url=....

Currently this works for Kotlin and Groovy build scripts. It also should work for Kotlin pre-compiled scripts and Kotlin and Groovy build logic. Though it doesn't work for Groovy pre-compiled build scripts. I suppose we should be able to make this work with Groovy pre-compiled build scripts as well, though I did run into some classloader problems there.

You can also declare a dependency in your build script on the Gradle API with a version by using gradleApi(<version>) instead of gradleApi().

Exploration doc: https://docs.google.com/document/d/1JB75X_Lt5BlubszqD0l10rHPqL89B2yd-yD39qizyDA/edit

subprojects/core-api/src/main/java/org/gradle/api/artifacts/dsl/DependencyHandler.java

lptr · 2022-04-08T09:04:08Z

subprojects/core-api/src/main/java/org/gradle/api/artifacts/dsl/DependencyHandler.java

+    /**
+     * Creates a dependency on the API of the current version of Gradle.
+     *
+     * @return The dependency.


It would be good to add some Javadoc on the expected format of the parameter. Are snapshots accepted?

I think we need to mention that only older versions are accepted, right? What happens if I give it a newer or non-existent version?

Yeah, probably. Though I wouldn't go into polishing the Javadoc for this spike.

lptr · 2022-04-08T09:05:42Z

subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java

+        return new GroovyScriptClasspathProvider(
+            classLoaderScopeRegistry.getCoreAndPluginsScope(),
+            () -> {
+                String version = System.getProperty("org.gradle.api.version");


We should qualify what API version this is. I'd go with org.gradle.api.target-version or something.

Yeah, I think we should qualify the property name. It feels like target doesn't really capture it. For me, target sounds more about the target runtime. How about org.gradle.api.source-version, since this is the version of the Gradle API we use for sources.

lptr · 2022-04-08T09:07:05Z

...plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java

    private void applyDependencies(Project project) {
        DependencyHandler dependencies = project.getDependencies();
-        dependencies.add(API_CONFIGURATION, dependencies.gradleApi());
+        String apiVersion = System.getProperty("org.gradle.api.version");


We query this property in six different places by name. Instead it would be good to have a shared mechanism to expose this as an Optional<VersionNumber> instead. An @Inject-able service would be nice.

I extracted the common logic to a single class. I didn't make it a service, yet, since I am not sure what scope this would need to go.

lptr · 2022-04-08T09:15:40Z

...plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java

+                repository.setUrl(repositoryUrl);
+            });
+            dependencies.add(API_CONFIGURATION, dependencies.gradleApi(apiVersion));
+        }


I think it would be good if the user could declare the API version they want to use via a gradleApi("6.9") call in the build script of the plugin. The above mechanism does not allow that.

Yes, this can be improved. Note that with this solution you can already specify a different version of the gradle API by adding a constraint to the configuration, since the gradleApi(version) is a regular dependency and participates in dependency resolution. For the final solution, I am not sure what the right thing from dependency management is. defaultDependencies sounds it may be something in the right direction.

lptr · 2022-04-08T09:16:31Z

...c/main/java/org/gradle/initialization/buildsrc/GroovyBuildSrcProjectConfigurationAction.java

+                repository.setUrl(repositoryUrl);
+            });
+            dependencies.add(JavaPlugin.API_CONFIGURATION_NAME, dependencies.gradleApi(apiVersion));
+        }


This code is repeated, and should be abstracted/shared.

I extracted the common code.

lptr · 2022-04-08T09:19:56Z

subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java

+                    ? gradleApisFromCurrentGradle(dependencyFactory)
+                    : gradleApisFromRepository(resolutionServices, dependencyFactory, version);
+            },
+            () -> ((SelfResolvingDependency) dependencyFactory.createDependency(DependencyFactory.ClassPathNotation.LOCAL_GROOVY)).resolve());


Looks like we are using local Groovy regardless of the Gradle version, but that will change. Shouldn't we resolve the Groovy version via the repository instead if the target API version is specified?

I wouldn't resolve a different Groovy/Kotlin version per API. If we'd do this, then we'd also need to upgrade the Kotlin/Groovy code and use a different compiler as well. I don't see why we can't compile the old code with the newer Kotlin/Groovy version.

I think that reasoning makes sense. Let's mention it here in a comment.

lptr · 2022-04-08T09:21:30Z

...c/main/java/org/gradle/api/internal/artifacts/dsl/dependencies/DefaultDependencyHandler.java


    @Override
    public Dependency gradleApi() {
        return dependencyFactory.createDependency(DependencyFactory.ClassPathNotation.GRADLE_API);


Should we qualify the name of this constant now? Like, CURRENT_GRADLE_API? Or BUNDLED_GRALDE_API?

I wouldn't do this now. I would like to get rid of this notation and always use a real dependency.

...l/src/integTest/kotlin/org/gradle/kotlin/dsl/integration/KotlinBuildScriptIntegrationTest.kt

subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/accessors/AccessorFragments.kt

…l/DependencyHandler.java Co-authored-by: Lóránt Pintér <[email protected]>

asodja · 2022-04-12T14:22:22Z

...ects/core/src/integTest/groovy/org/gradle/groovy/scripts/GroovyImportsIntegrationTest.groovy

        succeeds("help")
    }

+    @IgnoreRest


Is @IgnoreRest here for prototyping reasons?

Yeah, I removed it now.

The evaluation of the source Gradle API version needs to be lazy.

depending on the source Gradle API version.

asodja · 2022-04-19T15:11:23Z

...cts/core/src/main/java/org/gradle/groovy/scripts/internal/GroovyScriptClasspathProvider.java

+                                url.getProtocol(),
+                                url.getHost(),
+                                url.getPort(),
+                                url.getFile().replace(" ", "%20")


Do we need to encode any other character here?

I copied that from the Kotlin version of this class which seems to work. I suppose we should have only one classpath provider and not two separate implementations doing mostly the same things.

asodja · 2022-04-19T15:16:48Z

subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/accessors/tasks/PrintAccessors.kt

+    abstract val sourceGradleApiVersion: Property<String>
+
+    init {
+        sourceGradleApiVersion.set(GradleApiVersionProvider.getGradleApiSourceVersion().orElse(null))


There are two init blocks in this class. Is that ok?

asodja · 2022-04-19T15:36:03Z

subprojects/core/src/main/java/org/gradle/api/internal/artifacts/GradleApiVersionProvider.java

+        });
+    }
+
+    public static void addToConfiguration(Configuration configuration, DependencyHandler repositoryHandler) {


It's maybe not that important here, but could we return dependency here and the caller would do whatever it wants? Similar also for the repository.

For this case:

GradleApiVersionProvider.addToConfiguration(project.getConfigurations().getByName(API_CONFIGURATION), dependencies); GradleApiVersionProvider.addGradleSourceApiRepository(project.getRepositories());

It feels a bit weird to me that we accept something as a parameter and then we add something to it, instead of that we provide a dependency/repository and the caller would do with it whatever it wants.

asodja · 2022-04-19T15:42:18Z

subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/accessors/AccessorFragments.kt

        if (config.hasDeclarationDeprecations()) publicFunctionWithAnnotationsFlags to config.getDeclarationDeprecationBlock()
        else publicFunctionFlags to ""
+    val sourceApiGradleVersion = GradleVersion.version(
+        GradleApiVersionProvider.getGradleApiSourceVersion().orElse(GradleVersion.current().version)


It seems GradleApiVersionProvider.getGradleApiSourceVersion().orElse(GradleVersion.current().version) is repeated multiple times, would it make sense to have methods on GradleApiVersionProvider provider like
GradleApiVersionProvider.getGradleApiSourceVersionOrCurrent()?

asodja · 2022-04-19T15:51:37Z

subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/accessors/AccessorsClassPath.kt

 import org.gradle.api.artifacts.dsl.DependencyHandler
 import org.gradle.api.provider.Provider
-import org.gradle.api.provider.ProviderConvertible
+${if (supportsProviderConvertible) "import org.gradle.api.provider.ProviderConvertible" else ""}


Dang, I hope ifs for different source versions won't be too annoying in the future.

For this particular case, can we maybe just do: org.gradle.api.provider.* or are there too many types in that package?

lptr · 2022-06-14T13:10:29Z

This has been merged into #20818.

wolfs added the @execution label Apr 5, 2022

wolfs self-assigned this Apr 5, 2022

wolfs force-pushed the wolfs/provider/compile-groovy-scripts-against-gradle-api branch from 1cb5d97 to aa9a6be Compare April 5, 2022 12:17

wolfs added 7 commits April 5, 2022 14:18

Allow declaring a gradleApi with a version

a252643

Allow compiling Kotlin build scripts against a different Gradle API

2528beb

Allow specifying the Gradle API version via a system property

9f68fe1

Allow specifying the repository for the Gradle API

79ce33f

Create version specific accessors for configurations

aca870a

Compile Groovy scripts against Gradle API

8611b87

Setup right gradle API for build Src

600bab3

wolfs force-pushed the wolfs/provider/compile-groovy-scripts-against-gradle-api branch from aa9a6be to 600bab3 Compare April 5, 2022 12:18

lptr reviewed Apr 8, 2022

View reviewed changes

Update subprojects/core-api/src/main/java/org/gradle/api/artifacts/ds…

35a9481

…l/DependencyHandler.java Co-authored-by: Lóránt Pintér <[email protected]>

asodja reviewed Apr 12, 2022

View reviewed changes

wolfs added 3 commits April 13, 2022 11:37

Extract common logic

2999a18

Revert changes to tests

b5f3617

Rename system property

113105d

wolfs requested review from asodja and lptr April 13, 2022 14:14

wolfs added 6 commits April 13, 2022 16:16

Add incubating/since to gradleApi method

463ccc7

Add source version as inputs for accessor generation

667aa7f

Fix accessor import generation

0819c7f

The evaluation of the source Gradle API version needs to be lazy.

Add source Gradle API version as input to accessor generation

c3c6317

Use separate output directories for accessors

bab3dda

depending on the source Gradle API version.

Remove println

990e76e

wolfs mentioned this pull request Apr 19, 2022

Add some build logic to demonstrate source pinning gradle/provider-api-migration-testbed#4

Closed

asodja approved these changes Apr 19, 2022

View reviewed changes

lptr removed their request for review April 20, 2022 09:45

lptr added the in:api-evolution Bytecode upgrades, multi-gradle-version support, gradle-api label Jun 14, 2022

lptr closed this Jun 14, 2022

lptr deleted the wolfs/provider/compile-groovy-scripts-against-gradle-api branch June 14, 2022 13:10

Allow compilation of build scripts against a different Gradle API version #20373

Allow compilation of build scripts against a different Gradle API version #20373

Uh oh!

Conversation

wolfs commented Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lptr commented Jun 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wolfs commented Apr 5, 2022 •

edited

Loading