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

Handle gradle 7 removing default configurations #721

Merged
merged 8 commits into from
Jul 14, 2021
Merged

Conversation

ferozco
Copy link
Contributor

@ferozco ferozco commented May 20, 2021

Before this PR

Closes #700. Side note, its kinda funny that a gradle 7 issue is issue 700

After this PR

==COMMIT_MSG==
Handle gradle 7 removing default configurations
==COMMIT_MSG==

Possible downsides?

Don't think so, we are now eagerly looking for the configurations but it should be safe since we ensure that the java plugin has already been applied and hence the configurations exist

Increasing the minimum required Gradle version from 5.x to 6.1.

@policy-bot policy-bot bot requested a review from jkozlowski May 20, 2021 19:59
@carterkozak
Copy link
Contributor

Can we add a gradle 7 integration test?

@stale
Copy link

stale bot commented Jun 26, 2021

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Jun 26, 2021
@fawind fawind added long-lived and removed stale labels Jul 7, 2021
* The full configuration name follows the naming scheme of "$taskBaseName + capitalize($configurationName)"
* see {@link org.gradle.api.internal.tasks.DefaultSourceSet#configurationNameOf}.
*/
private static final ImmutableList<String> DEPRECATED_SOURCESET_SUFFIXES = ImmutableList.of("Compile", "Runtime");
Copy link
Contributor

Choose a reason for hiding this comment

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

The getCompileConfigurationName and getRuntimeConfigurationName method no longer exist with Gradle 7 and the method for building the configuration name based on the base config is private (ref).

Matching on the configuration name suffix for backwards compatibility but for Gradle 7 projects, this shouldn't match anything and can be removed in the future.

plugins {
id '${PLUGIN_NAME}'
}
allprojects {
apply plugin: 'com.palantir.configuration-resolver'
tasks.register("resolveConfigurations", {
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked too deeply into why, but on Gradle 7, we fail to resolve the gradle-configuration-resolver-plugin. However given that the plugin is archived, I pulled the relevant task out into directly into this test and removed the dependency.

@@ -103,7 +103,7 @@

public class VersionsLockPlugin implements Plugin<Project> {
private static final Logger log = Logging.getLogger(VersionsLockPlugin.class);
static final GradleVersion MINIMUM_GRADLE_VERSION = GradleVersion.version("5.3");
static final GradleVersion MINIMUM_GRADLE_VERSION = GradleVersion.version("6.1");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is imposed by nebula-dependency-recommender:10.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

We only need nebula-dependency-recommender for tests, right? In theory, we could replace it with our own gradle code that does the same stuff.

That said, I think it's fine to kill off Gradle 5 support now that 7 is out (probably want to major rev it).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's just for tests so not strictly raising the minimum gradle version for the actual plugin.

@fawind fawind changed the title Fix: Handle gradle 7 removing default configurations Handle gradle 7 removing default configurations Jul 8, 2021
JavaPlugin.RUNTIME_CONFIGURATION_NAME)
.map(project.getConfigurations()::named)
.forEach(confProvider -> confProvider.configure(conf -> {
// TODO(fwindheuser): Remove compile and runtime after stating to build with Gradle 7+
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should enforce this by adding a precondition that MINIMUM_GRADLE_VERSION < 7?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this precondition would get triggered when a consuming repository builds with Gradle 7? I think for now, we want to enable users to use either Gradle 6 or 7.

@fawind fawind requested a review from CRogers July 12, 2021 10:34
@bulldozer-bot bulldozer-bot bot merged commit 40942d1 into develop Jul 14, 2021
@bulldozer-bot bulldozer-bot bot deleted the fo/gradle-7 branch July 14, 2021 13:23
@svc-autorelease
Copy link
Collaborator

Failed to load project - please reach out to #dev-foundry-infra or check Aries to debug

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

Successfully merging this pull request may close these issues.

Gradle 7 compatibility
6 participants