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

New baseline-class-uniqueness.lock file allows incremental adoption #1145

Merged
merged 19 commits into from
Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,35 @@ in `.baseline/copyright/*.txt` and the RegexpHeader checkstyle configuration in


## com.palantir.baseline-class-uniqueness
Run `./gradlew checkRuntimeClassUniqueness` to scan all jars on the `runtime` classpath for identically named classes.
This task will run automatically as part of `./gradlew build`. To run the task on other configurations, use the
`check<Xyz>ClassUniqueness` task for the `xyz` configuration.
When applied to a java project, this inspects all the jars in your `runtimeClasspath` configuration and records any conflicts to a `baseline-class-uniqueness.lock` file. For example:

```
# Danger! Multiple jars contain identically named classes. This may cause different behaviour depending on classpath ordering.
# Run ./gradlew checkClassUniqueness --write-locks to update this file

## runtimeClasspath
[jakarta.annotation:jakarta.annotation-api, javax.annotation:javax.annotation-api]
- javax.annotation.Resource$AuthenticationType
[jakarta.ws.rs:jakarta.ws.rs-api, javax.ws.rs:javax.ws.rs-api]
- javax.ws.rs.BadRequestException
- javax.ws.rs.ClientErrorException
- javax.ws.rs.ForbiddenException
- javax.ws.rs.InternalServerErrorException
- javax.ws.rs.NotAcceptableException
- javax.ws.rs.NotAllowedException
- javax.ws.rs.NotAuthorizedException
- javax.ws.rs.NotFoundException
- javax.ws.rs.NotSupportedException
- javax.ws.rs.Priorities
```

This task can also be used to analyze other configurations in addition to `runtimeClasspath`, e.g.:

```gradle
checkClassUniqueness {
configurations.add project.configurations.myConf
}
```

If you discover multiple jars on your classpath contain clashing classes, you should ideally try to fix them upstream and then depend on the fixed version. If this is not feasible, you may be able to tell Gradle to [use a substituted dependency instead](https://docs.gradle.org/current/userguide/customizing_dependency_resolution_behavior.html#sec:module_substitution):

Expand All @@ -244,7 +270,6 @@ configurations.all {
}
```


## com.palantir.baseline-circleci
Automatically applies the following plugins:

Expand Down
8 changes: 8 additions & 0 deletions changelog/@unreleased/pr-1145.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
type: feature
feature:
description: The `com.palantir.baseline-class-uniqueness` plugin now records conflicts
in a `baseline-class-uniqueness.lock` file. This should be checked-in to git and
makes it easier to incrementally improve projects, rather than requiring a big-bang
migration.
links:
- https://github.com/palantir/gradle-baseline/pull/1145
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,30 @@

package com.palantir.baseline.plugins;

import com.palantir.baseline.plugins.rules.BaselineClassUniquenessRule;
import com.palantir.baseline.tasks.CheckClassUniquenessLockTask;
import org.gradle.api.Project;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.language.base.plugins.LifecycleBasePlugin;

/**
* This plugin is similar to https://github.com/nebula-plugins/gradle-lint-plugin/wiki/Duplicate-Classes-Rule
* but goes one step further and actually hashes any identically named classfiles to figure out if they're
* <i>completely</i> identical (and therefore safely interchangeable).
* This plugin is similar to https://github.com/nebula-plugins/gradle-lint-plugin/wiki/Duplicate-Classes-Rule but goes
* one step further and actually hashes any identically named classfiles to figure out if they're <i>completely</i>
* identical (and therefore safely interchangeable).
*
* The task only fails if it finds classes which have the same name but different implementations.
* <p>The task only fails if it finds classes which have the same name but different implementations.
*/
public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin {
@Override
public final void apply(Project project) {
BaselineClassUniquenessRule rule = new BaselineClassUniquenessRule(project);

project.getTasks().addRule(rule);
TaskProvider<CheckClassUniquenessLockTask> lockTask =
project.getTasks().register("checkClassUniqueness", CheckClassUniquenessLockTask.class);
project.getPlugins().apply(LifecycleBasePlugin.class);
project.getTasks().getByName(LifecycleBasePlugin.CHECK_TASK_NAME).dependsOn(lockTask);

project.getPlugins().withId("java", plugin -> {
String checkRuntimeClasspathTask = "checkRuntimeClasspathClassUniqueness";
rule.apply(checkRuntimeClasspathTask);
project.getTasks().getByName("check")
.dependsOn(project.getTasks().getByName(checkRuntimeClasspathTask));
lockTask.configure(t -> t.configurations.add(
iamdanfox marked this conversation as resolved.
Show resolved Hide resolved
iamdanfox marked this conversation as resolved.
Show resolved Hide resolved
project.getConfigurations().getByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME)));
});
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/*
* (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.baseline.tasks;

import com.google.common.collect.ImmutableList;
import java.io.File;
import java.util.Collection;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.gradle.api.DefaultTask;
import org.gradle.api.GradleException;
import org.gradle.api.Task;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.ModuleVersionIdentifier;
import org.gradle.api.provider.SetProperty;
import org.gradle.api.specs.Spec;
import org.gradle.api.tasks.CacheableTask;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.TaskAction;
import org.gradle.util.GFileUtils;

@CacheableTask
public class CheckClassUniquenessLockTask extends DefaultTask {

private static final String HEADER = "# Danger! Multiple jars contain identically named classes. This may "
+ "cause different behaviour depending on classpath ordering.\n"
+ "# Run ./gradlew checkClassUniqueness --write-locks to update this file\n\n";

// not marking this as an Input, because we want to re-run if the *contents* of a configuration changes
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the contents only change if a dependency on the configuration changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configurations are not Serializable, so I can't just mark a list of them as an @Input.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can take the configuration as an input by annotating it with @classpath

Copy link
Contributor Author

@iamdanfox iamdanfox Jan 7, 2020

Choose a reason for hiding this comment

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

I think that @classpath annotation would work if I had just a single configuration, but in order to preserve the ability to analyze multiple configurations, I don't have just one single configuration here but a bunch of them?

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of storing a set of configurations could you create a single configuration which extends from all of the provided configurations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno, it seems that has a slight risk of false positives... e.g. if we're analyzing two configurations foo and bar and we move a jar from foo -> bar. In the big clobbered configuration there would be no change, but actually I would expect the lockfile to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok no big deal

@SuppressWarnings("VisibilityModifier")
public final SetProperty<Configuration> configurations;

private final File lockFile;

public CheckClassUniquenessLockTask() {
this.configurations = getProject().getObjects().setProperty(Configuration.class);
this.lockFile = getProject().file("baseline-class-uniqueness.lock");
onlyIf(new Spec<Task>() {
@Override
public boolean isSatisfiedBy(Task task) {
return !configurations.get().isEmpty();
}
});
}

/**
* This method exists purely for up-to-dateness purposes - we want to re-run if the contents of a configuration
* changes.
*/
@Input
public final Map<String, ImmutableList<String>> getContentsOfAllConfigurations() {
return configurations.get().stream().collect(Collectors.toMap(Configuration::getName, configuration -> {
return configuration.getIncoming().getResolutionResult().getAllComponents().stream()
.map(resolvedComponentResult -> Objects.toString(resolvedComponentResult.getModuleVersion()))
.collect(ImmutableList.toImmutableList()); // Gradle requires this to be Serializable
}));
}

@OutputFile
public final File getLockFile() {
return lockFile;
}

@TaskAction
public final void doIt() {
Map<String, Optional<String>> resultsByConfiguration = configurations.get().stream()
.collect(Collectors.toMap(Configuration::getName, configuration -> {
ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(getProject().getLogger());
analyzer.analyzeConfiguration(configuration);
Collection<Set<ModuleVersionIdentifier>> problemJars = analyzer.getDifferingProblemJars();

if (problemJars.isEmpty()) {
return Optional.empty();
}

StringBuilder stringBuilder = new StringBuilder();
// TODO(dfox): ensure we iterate through problemJars in a stable order
for (Set<ModuleVersionIdentifier> clashingJars : problemJars) {
stringBuilder
.append(clashingJars.stream()
.map(mvi -> mvi.getGroup() + ":" + mvi.getName())
.sorted()
.collect(Collectors.joining(", ", "[", "]")))
.append('\n');

analyzer.getDifferingSharedClassesInProblemJars(clashingJars).stream()
.sorted()
.forEach(className -> {
stringBuilder.append(" - ");
stringBuilder.append(className);
stringBuilder.append('\n');
});
}
return Optional.of(stringBuilder.toString());
}));

boolean conflictsFound = resultsByConfiguration.values().stream().anyMatch(Optional::isPresent);
if (!conflictsFound) {
// this is desirable because if means if people apply the plugin to lots of projects which are already
// compliant, they don't get loads of noisy lockfiles created.
ensureLockfileDoesNotExist();
} else {
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append(HEADER);
// TODO(dfox): make configuration order stable!
resultsByConfiguration.forEach((configuration, contents) -> {
if (contents.isPresent()) {
stringBuilder.append("## ").append(configuration).append("\n");
stringBuilder.append(contents.get());
}
});
ensureLockfileContains(stringBuilder.toString());
}
}

private void ensureLockfileContains(String expected) {
if (getProject().getGradle().getStartParameter().isWriteDependencyLocks()) {
GFileUtils.writeFile(expected, lockFile);
getLogger().lifecycle("Updated {}", getProject().getRootDir().toPath().relativize(lockFile.toPath()));
return;
}

if (!lockFile.exists()) {
throw new GradleException("baseline-class-uniqueness detected multiple jars containing identically named "
+ "classes. Please resolve these problems, or run `./gradlew checkClassUniqueness "
+ "--write-locks` to accept them:\n\n" + expected);
}

String onDisk = GFileUtils.readFile(lockFile);
if (!onDisk.equals(expected)) {
throw new GradleException(lockFile
+ " is out of date, please run `./gradlew "
+ "checkClassUniqueness --write-locks` to update this file");
}
}

private void ensureLockfileDoesNotExist() {
if (lockFile.exists()) {
if (getProject().getGradle().getStartParameter().isWriteDependencyLocks()) {
GFileUtils.deleteQuietly(lockFile);
getLogger().lifecycle("Deleted {}", getProject().getRootDir().toPath().relativize(lockFile.toPath()));
} else {
throw new GradleException(lockFile + " should not exist (as no problems were found).");
}
}
}
}
Loading