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

Provisioner can provide dependencies without their transitives #297

Merged
merged 5 commits into from
Sep 13, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ You might be looking for:

### Version 1.15.0-SNAPSHOT - TBD (javadoc [lib](https://diffplug.github.io/spotless/javadoc/spotless-lib/snapshot/) [lib-extra](https://diffplug.github.io/spotless/javadoc/spotless-lib-extra/snapshot/), [snapshot repo](https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/))

* Extended dependency provisioner to exclude transitives on request ([#297](https://github.com/diffplug/spotless/pull/297)).This prevents unnecessary downloads of unused transitive dependencies for Eclipse based formatter steps.
* Updated default groovy-eclipse from 4.8.0 to 4.8.1 ([#288](https://github.com/diffplug/spotless/pull/288)). New version is based on [Groovy-Eclipse 3.0.0](https://github.com/groovy/groovy-eclipse/wiki/3.0.0-Release-Notes).
* Integrated Eclipse WTP formatter ([#290](https://github.com/diffplug/spotless/pull/290))
* Updated JSR305 annotation from 3.0.0 to 3.0.2 ([#274](https://github.com/diffplug/spotless/pull/274))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public static class State implements Serializable {

/** State constructor expects that all passed items are not modified afterwards */
protected State(String formatterStepExt, Provisioner jarProvisioner, List<String> dependencies, Iterable<File> settingsFiles) throws IOException {
this.jarState = JarState.from(dependencies, jarProvisioner);
this.jarState = JarState.withoutTransitives(dependencies, jarProvisioner);
this.settingsFiles = FileSignature.signAsList(settingsFiles);
this.formatterStepExt = formatterStepExt;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ org.eclipse.platform:org.eclipse.equinox.app:1.3.500
org.eclipse.platform:org.eclipse.equinox.common:3.10.0
org.eclipse.platform:org.eclipse.equinox.preferences:3.7.100
org.eclipse.platform:org.eclipse.equinox.registry:3.8.0
#Spotless currently loads all transitive dependencies.
#jface requires platform specific JARs (not used by formatter), which are not hosted via M2.
#org.eclipse.platform:org.eclipse.jface.text:3.13.0
#org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.jface.text:3.13.0
org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.osgi.services:3.7.0
org.eclipse.platform:org.eclipse.osgi:3.13.0
org.eclipse.platform:org.eclipse.text:3.6.300
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ org.eclipse.platform:org.eclipse.equinox.app:1.3.500
org.eclipse.platform:org.eclipse.equinox.common:3.10.0
org.eclipse.platform:org.eclipse.equinox.preferences:3.7.100
org.eclipse.platform:org.eclipse.equinox.registry:3.8.0
#Spotless currently loads all transitive dependencies.
#jface requires platform specific JARs (not used by formatter), which are not hosted via M2.
#org.eclipse.platform:org.eclipse.jface.text:3.13.0
#org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.jface.text:3.13.0
org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.osgi:3.13.0
org.eclipse.platform:org.eclipse.text:3.6.300
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ org.eclipse.platform:org.eclipse.equinox.app:1.3.500
org.eclipse.platform:org.eclipse.equinox.common:3.10.0
org.eclipse.platform:org.eclipse.equinox.preferences:3.7.100
org.eclipse.platform:org.eclipse.equinox.registry:3.8.0
#Spotless currently loads all transitive dependencies.
#jface requires platform specific JARs (not used by formatter), which are not hosted via M2.
#org.eclipse.platform:org.eclipse.jface.text:3.13.0
#org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.jface.text:3.13.0
org.eclipse.platform:org.eclipse.jface:3.14.0
org.eclipse.platform:org.eclipse.osgi:3.13.0
org.eclipse.platform:org.eclipse.text:3.6.300
17 changes: 15 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/JarState.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,37 @@ public final class JarState implements Serializable {
@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
private final transient Set<File> jars;

@Deprecated // internal
public JarState(String mavenCoordinate, FileSignature fileSignature, Set<File> jars) {
this(Arrays.asList(mavenCoordinate), fileSignature, jars);
}

@Deprecated // internal
public JarState(Collection<String> mavenCoordinates, FileSignature fileSignature, Set<File> jars) {
this.mavenCoordinates = new TreeSet<String>(mavenCoordinates);
this.fileSignature = fileSignature;
this.jars = jars;
}

/** Provisions the given maven coordinate and its transitive dependencies. */
public static JarState from(String mavenCoordinate, Provisioner provisioner) throws IOException {
return from(Collections.singletonList(mavenCoordinate), provisioner);
}

/** Provisions the given maven coordinates and their transitive dependencies. */
public static JarState from(Collection<String> mavenCoordinates, Provisioner provisioner) throws IOException {
Objects.requireNonNull(provisioner, "provisioner");
return provisionWithTransitives(true, mavenCoordinates, provisioner);
}

/** Provisions the given maven coordinates without their transitive dependencies. */
public static JarState withoutTransitives(Collection<String> mavenCoordinates, Provisioner provisioner) throws IOException {
return provisionWithTransitives(false, mavenCoordinates, provisioner);
}

private static JarState provisionWithTransitives(boolean withTransitives, Collection<String> mavenCoordinates, Provisioner provisioner) throws IOException {
Objects.requireNonNull(mavenCoordinates, "mavenCoordinates");
Set<File> jars = provisioner.provisionWithDependencies(mavenCoordinates);
Objects.requireNonNull(provisioner, "provisioner");
Set<File> jars = provisioner.provisionWithTransitives(withTransitives, mavenCoordinates);
if (jars.isEmpty()) {
throw new NoSuchElementException("Resolved to an empty result: " + mavenCoordinates.stream().collect(Collectors.joining(", ")));
}
Expand Down
29 changes: 21 additions & 8 deletions lib/src/main/java/com/diffplug/spotless/Provisioner.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,30 @@
* Spotless' dependencies minimal.
*/
public interface Provisioner {

/** Method interface has been extended to {@link Provisioner#provide}. */
@Deprecated
public default Set<File> provisionWithDependencies(Collection<String> mavenCoordinates) {
return provisionWithTransitives(true, mavenCoordinates);
}

/** Method interface has been extended to {@link Provisioner#provide}. */
@Deprecated
public default Set<File> provisionWithDependencies(String... mavenCoordinates) {
return provisionWithDependencies(Arrays.asList(mavenCoordinates));
}

/**
* Given a set of maven coordinates, returns a set of jars which
* include all of the specified coordinates and their dependencies.
* Given a set of maven coordinates, returns a set of jars which include all
* of the specified coordinates and optionally their transitive dependencies.
*/
public Set<File> provisionWithDependencies(Collection<String> mavenCoordinates);
public default Set<File> provisionWithTransitives(boolean withTransitives, String... mavenCoordinates) {
return provisionWithTransitives(withTransitives, Arrays.asList(mavenCoordinates));
}

/**
* Given a set of maven coordinates, returns a set of jars which
* include all of the specified coordinates and their dependencies.
* Given a set of maven coordinates, returns a set of jars which include all
* of the specified coordinates and optionally their transitive dependencies.
*/
public default Set<File> provisionWithDependencies(String... mavenCoordinates) {
return provisionWithDependencies(Arrays.asList(mavenCoordinates));
}
public Set<File> provisionWithTransitives(boolean withTransitives, Collection<String> mavenCoordinates);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ private GradleProvisioner() {}

public static Provisioner fromProject(Project project) {
Objects.requireNonNull(project);
return mavenCoords -> {
return (withTransitives, mavenCoords) -> {
try {
Dependency[] deps = mavenCoords.stream()
.map(project.getBuildscript().getDependencies()::create)
.toArray(Dependency[]::new);
Configuration config = project.getRootProject().getBuildscript().getConfigurations().detachedConfiguration(deps);
config.setDescription(mavenCoords.toString());
config.setTransitive(withTransitives);
return config.resolve();
} catch (Exception e) {
logger.log(Level.SEVERE,
Expand All @@ -53,4 +54,5 @@ public static Provisioner fromProject(Project project) {
}

private static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName());

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@

public class ArtifactResolver {

private static final DependencyFilter ACCEPT_ALL_FILTER = (node, parents) -> true;
private static final DependencyFilter ACCEPT_ALL = (node, parents) -> true;
private static final DependencyFilter FILTER_TRANSITIVES = (node, parents) -> parents.size() <= 1;

private final RepositorySystem repositorySystem;
private final RepositorySystemSession session;
Expand All @@ -56,19 +57,28 @@ public ArtifactResolver(RepositorySystem repositorySystem, RepositorySystemSessi
this.log = Objects.requireNonNull(log);
}

/** Use {@link ArtifactResolver#resolve(Collection) instead.} */
/** Use {@link ArtifactResolver#resolve(boolean, Collection)) instead.} */
@Deprecated
public Set<File> resolve(String mavenCoordinate) {
return resolve(Arrays.asList(mavenCoordinate));
return resolve(true, Arrays.asList(mavenCoordinate));
}

public Set<File> resolve(Collection<String> mavenCoordinate) {
List<Dependency> dependencies = mavenCoordinate.stream()
/**
* Given a set of maven coordinates, returns a set of jars which include all
* of the specified coordinates and optionally their transitive dependencies.
*/
public Set<File> resolve(boolean withTransitives, Collection<String> mavenCoordinates) {
List<Dependency> dependencies = mavenCoordinates.stream()
.map(coordinateString -> new DefaultArtifact(coordinateString))
.map(artifact -> new Dependency(artifact, null))
.collect(toList());
CollectRequest collectRequest = new CollectRequest((Dependency) null, dependencies, repositories);
DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, ACCEPT_ALL_FILTER);
CollectRequest collectRequest = new CollectRequest(dependencies, null, repositories);
DependencyRequest dependencyRequest;
if (withTransitives) {
dependencyRequest = new DependencyRequest(collectRequest, ACCEPT_ALL);
} else {
dependencyRequest = new DependencyRequest(collectRequest, FILTER_TRANSITIVES);
}

DependencyResult dependencyResult = resolveDependencies(dependencyRequest);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,4 @@ private MavenProvisioner() {}
public static Provisioner create(ArtifactResolver artifactResolver) {
return artifactResolver::resolve;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
public class MavenProvisionerTest extends MavenIntegrationTest {

@Test
public void testMultipleDependencies() throws Exception {
public void testMultipleDependenciesExcludingTransitives() throws Exception {
writePomWithJavaSteps(
"<eclipse>",
" <version>4.8.0</version>",
Expand All @@ -30,7 +30,7 @@ public void testMultipleDependencies() throws Exception {
}

@Test
public void testSingleDependency() throws Exception {
public void testSingleDependencyIncludingTransitives() throws Exception {
writePomWithJavaSteps(
"<googleJavaFormat>",
" <version>1.2</version>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ private static Supplier<Provisioner> createLazyWithRepositories(Consumer<Reposit
return Suppliers.memoize(() -> {
Project project = ProjectBuilder.builder().build();
repoConfig.accept(project.getRepositories());
return mavenCoords -> {
return (withTransitives, mavenCoords) -> {
Dependency[] deps = mavenCoords.stream()
.map(project.getDependencies()::create)
.toArray(Dependency[]::new);
Configuration config = project.getConfigurations().detachedConfiguration(deps);
config.setTransitive(withTransitives);
config.setDescription(mavenCoords.toString());
try {
return config.resolve();
Expand Down Expand Up @@ -85,11 +86,11 @@ private static Provisioner caching(String name, Supplier<Provisioner> input) {
} else {
cached = new HashMap<>();
}
return mavenCoords -> {
return (withTransitives, mavenCoords) -> {
Box<Boolean> wasChanged = Box.of(false);
ImmutableSet<File> result = cached.computeIfAbsent(ImmutableSet.copyOf(mavenCoords), coords -> {
wasChanged.set(true);
return ImmutableSet.copyOf(input.get().provisionWithDependencies(coords));
return ImmutableSet.copyOf(input.get().provisionWithTransitives(withTransitives, coords));
});
if (wasChanged.get()) {
try (ObjectOutputStream outputStream = new ObjectOutputStream(Files.asByteSink(cacheFile).openBufferedStream())) {
Expand Down
16 changes: 14 additions & 2 deletions testlib/src/test/java/com/diffplug/spotless/ProvisionerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,25 @@

public class ProvisionerTest {
@Test
public void testManipulation() {
Provisioner provisioner = deps -> deps.stream().map(File::new).collect(Collectors.toSet());
@Deprecated
public void testManipulationDeprecated() {
Provisioner provisioner = (withTransitives, deps) -> deps.stream().map(File::new).collect(Collectors.toSet());
Assertions.assertThat(provisioner.provisionWithDependencies("a"))
.containsExactlyInAnyOrder(new File("a"));
Assertions.assertThat(provisioner.provisionWithDependencies("a", "a"))
.containsExactlyInAnyOrder(new File("a"));
Assertions.assertThat(provisioner.provisionWithDependencies(Arrays.asList("a", "a")))
.containsExactlyInAnyOrder(new File("a"));
}

@Test
public void testManipulation() {
Provisioner provisioner = (withTransitives, deps) -> deps.stream().map(File::new).collect(Collectors.toSet());
Assertions.assertThat(provisioner.provisionWithTransitives(true, "a"))
.containsExactlyInAnyOrder(new File("a"));
Assertions.assertThat(provisioner.provisionWithTransitives(true, "a", "a"))
.containsExactlyInAnyOrder(new File("a"));
Assertions.assertThat(provisioner.provisionWithTransitives(true, Arrays.asList("a", "a")))
.containsExactlyInAnyOrder(new File("a"));
}
}