From f5add2e6b070056827ebd93666a277a937f47242 Mon Sep 17 00:00:00 2001 From: Andrew Oberstar Date: Tue, 5 Jun 2018 21:06:26 -0500 Subject: [PATCH] Remove memoization of Grgit When switching to providers, there was the issue that each task that called the provider would potentially open a new Grgit instance, which weren't going to be closed properly. With 1.0.0, we switched to use a Guava memoized Supplier, which was intended to deal with that issue. It's not entirely clear if it is causing the issue, but a couple people have noticed getting a NoRemoteRepositoryException: origin: not found. This change leverages a different aspect of Providers where I can have the Reset task be the one to open the Grgit instance, and just set the opened instance in the Property, which I can then pass to the other tasks. That way the Property's value isn't based on a Provider that will be re-evaluated for each task. Even if this doesn't address the issue, it's probably a decent improvement as it drops the Guava dependency. This will hopefully address #51. --- build.gradle | 3 - gradle/dependency-locks/compile.lockfile | 6 -- .../compileClasspath.lockfile | 6 -- gradle/dependency-locks/default.lockfile | 6 -- gradle/dependency-locks/runtime.lockfile | 6 -- .../runtimeClasspath.lockfile | 6 -- gradle/dependency-locks/testCompile.lockfile | 6 -- .../testCompileClasspath.lockfile | 6 -- gradle/dependency-locks/testRuntime.lockfile | 6 -- .../testRuntimeClasspath.lockfile | 6 -- .../gradle/git/publish/GitPublishPlugin.java | 66 +++------------ .../git/publish/tasks/GitPublishReset.java | 82 ++++++++++++++++--- 12 files changed, 82 insertions(+), 123 deletions(-) diff --git a/build.gradle b/build.gradle index 7d71d0a..85d7c8f 100644 --- a/build.gradle +++ b/build.gradle @@ -25,9 +25,6 @@ dependencies { compile 'org.ajoberstar:grgit:[2.0.0,)' compatTestCompile 'org.ajoberstar:grgit:[2.0.0,)' - // util - compile 'com.google.guava:guava:[25.1-jre,)' - // testing compatTestCompile gradleTestKit() compatTestCompile 'org.spockframework:spock-core:1.1-groovy-2.4' diff --git a/gradle/dependency-locks/compile.lockfile b/gradle/dependency-locks/compile.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/compile.lockfile +++ b/gradle/dependency-locks/compile.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/compileClasspath.lockfile b/gradle/dependency-locks/compileClasspath.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/compileClasspath.lockfile +++ b/gradle/dependency-locks/compileClasspath.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/default.lockfile b/gradle/dependency-locks/default.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/default.lockfile +++ b/gradle/dependency-locks/default.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/runtime.lockfile b/gradle/dependency-locks/runtime.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/runtime.lockfile +++ b/gradle/dependency-locks/runtime.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/runtimeClasspath.lockfile b/gradle/dependency-locks/runtimeClasspath.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/runtimeClasspath.lockfile +++ b/gradle/dependency-locks/runtimeClasspath.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/testCompile.lockfile b/gradle/dependency-locks/testCompile.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/testCompile.lockfile +++ b/gradle/dependency-locks/testCompile.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/testCompileClasspath.lockfile b/gradle/dependency-locks/testCompileClasspath.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/testCompileClasspath.lockfile +++ b/gradle/dependency-locks/testCompileClasspath.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/testRuntime.lockfile b/gradle/dependency-locks/testRuntime.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/testRuntime.lockfile +++ b/gradle/dependency-locks/testRuntime.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/gradle/dependency-locks/testRuntimeClasspath.lockfile b/gradle/dependency-locks/testRuntimeClasspath.lockfile index 0bab974..d7137ee 100644 --- a/gradle/dependency-locks/testRuntimeClasspath.lockfile +++ b/gradle/dependency-locks/testRuntimeClasspath.lockfile @@ -1,10 +1,6 @@ # This is a Gradle generated file for dependency locking. # Manual edits can break the build and are not advised. # This file is expected to be part of source control. -com.google.code.findbugs:jsr305:3.0.2 -com.google.errorprone:error_prone_annotations:2.1.3 -com.google.guava:guava:25.1-jre -com.google.j2objc:j2objc-annotations:1.1 com.googlecode.javaewah:JavaEWAH:1.1.6 com.jcraft:jsch.agentproxy.core:0.0.9 com.jcraft:jsch.agentproxy.jsch:0.0.9 @@ -21,8 +17,6 @@ net.java.dev.jna:jna:4.1.0 org.ajoberstar:grgit:2.2.1 org.apache.httpcomponents:httpclient:4.5.2 org.apache.httpcomponents:httpcore:4.4.4 -org.checkerframework:checker-qual:2.0.0 -org.codehaus.mojo:animal-sniffer-annotations:1.14 org.eclipse.jgit:org.eclipse.jgit.ui:4.11.0.201803080745-r org.eclipse.jgit:org.eclipse.jgit:4.11.0.201803080745-r org.slf4j:slf4j-api:1.7.2 diff --git a/src/main/java/org/ajoberstar/gradle/git/publish/GitPublishPlugin.java b/src/main/java/org/ajoberstar/gradle/git/publish/GitPublishPlugin.java index a76bc92..271d6d0 100644 --- a/src/main/java/org/ajoberstar/gradle/git/publish/GitPublishPlugin.java +++ b/src/main/java/org/ajoberstar/gradle/git/publish/GitPublishPlugin.java @@ -1,15 +1,11 @@ package org.ajoberstar.gradle.git.publish; -import java.net.URISyntaxException; import java.util.Optional; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import org.ajoberstar.gradle.git.publish.tasks.GitPublishCommit; import org.ajoberstar.gradle.git.publish.tasks.GitPublishPush; import org.ajoberstar.gradle.git.publish.tasks.GitPublishReset; import org.ajoberstar.grgit.Grgit; -import org.eclipse.jgit.transport.URIish; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; @@ -39,16 +35,10 @@ public void apply(Project project) { extension.getRepoDir().set(project.getLayout().getBuildDirectory().dir("gitPublish")); - Supplier grgitSupplier = Suppliers.memoize(() -> { - return findExistingRepo(project, extension).orElseGet(() -> freshRepo(project, extension)); - }); - - Provider grgitProvider = project.provider(grgitSupplier::get); - - Task reset = createResetTask(project, extension, grgitProvider); + GitPublishReset reset = createResetTask(project, extension); Task copy = createCopyTask(project, extension); - Task commit = createCommitTask(project, extension, grgitProvider); - Task push = createPushTask(project, extension, grgitProvider); + Task commit = createCommitTask(project, extension, reset.getGrgit()); + Task push = createPushTask(project, extension, reset.getGrgit()); push.dependsOn(commit); commit.dependsOn(copy); copy.dependsOn(reset); @@ -56,21 +46,22 @@ public void apply(Project project) { // always close the repo at the end of the build project.getGradle().buildFinished(result -> { project.getLogger().info("Closing Git publish repo: {}", extension.getRepoDir().get()); - grgitProvider.get().close(); + reset.getGrgit().get().close(); }); } - private Task createResetTask(Project project, GitPublishExtension extension, Provider grgitProvider) { + private GitPublishReset createResetTask(Project project, GitPublishExtension extension) { return project.getTasks().create(RESET_TASK, GitPublishReset.class, task -> { task.setGroup("publishing"); task.setDescription("Prepares a git repo for new content to be generated."); - task.getGrgit().set(grgitProvider); + task.getRepoDirectory().set(extension.getRepoDir()); + task.getRepoUri().set(extension.getRepoUri()); task.getBranch().set(extension.getBranch()); task.setPreserve(extension.getPreserve()); }); } - private Task createCopyTask(Project project, GitPublishExtension extension) { + private Copy createCopyTask(Project project, GitPublishExtension extension) { return project.getTasks().create(COPY_TASK, Copy.class, task -> { task.setGroup("publishing"); task.setDescription("Copy contents to be published to git."); @@ -79,7 +70,7 @@ private Task createCopyTask(Project project, GitPublishExtension extension) { }); } - private Task createCommitTask(Project project, GitPublishExtension extension, Provider grgitProvider) { + private GitPublishCommit createCommitTask(Project project, GitPublishExtension extension, Provider grgitProvider) { return project.getTasks().create(COMMIT_TASK, GitPublishCommit.class, task -> { task.setGroup("publishing"); task.setDescription("Commits changes to be published to git."); @@ -88,7 +79,7 @@ private Task createCommitTask(Project project, GitPublishExtension extension, Pr }); } - private Task createPushTask(Project project, GitPublishExtension extension, Provider grgitProvider) { + private GitPublishPush createPushTask(Project project, GitPublishExtension extension, Provider grgitProvider) { return project.getTasks().create(PUSH_TASK, GitPublishPush.class, task -> { task.setGroup("publishing"); task.setDescription("Pushes changes to git."); @@ -97,43 +88,6 @@ private Task createPushTask(Project project, GitPublishExtension extension, Prov }); } - private Optional findExistingRepo(Project project, GitPublishExtension extension) { - try { - return Optional.of(Grgit.open(op -> op.setDir(extension.getRepoDir().get().getAsFile()))) - .filter(repo -> { - try { - String originUri = getOriginUri(repo); - // need to use the URIish to normalize them and ensure we support all Git compatible URI-ishs (URL - // is too limiting) - boolean valid = new URIish(extension.getRepoUri().get()).equals(new URIish(originUri)) && extension.getBranch().get().equals(repo.getBranch().current().getName()); - if (!valid) { - repo.close(); - } - return valid; - } catch (URISyntaxException e) { - throw new RuntimeException("Invalid URI.", e); - } - }); - } catch (Exception e) { - // missing, invalid, or corrupt repo - project.getLogger().debug("Failed to find existing Git publish repository.", e); - return Optional.empty(); - } - } - - private Grgit freshRepo(Project project, GitPublishExtension extension) { - project.delete(extension.getRepoDir().get().getAsFile()); - - Grgit repo = Grgit.init(op -> { - op.setDir(extension.getRepoDir().get().getAsFile()); - }); - repo.getRemote().add(op -> { - op.setName("origin"); - op.setUrl(extension.getRepoUri().get()); - }); - return repo; - } - private String getOriginUri(Grgit grgit) { return grgit.getRemote().list().stream() .filter(remote -> remote.getName().equals("origin")) diff --git a/src/main/java/org/ajoberstar/gradle/git/publish/tasks/GitPublishReset.java b/src/main/java/org/ajoberstar/gradle/git/publish/tasks/GitPublishReset.java index da90b78..eb7bc90 100644 --- a/src/main/java/org/ajoberstar/gradle/git/publish/tasks/GitPublishReset.java +++ b/src/main/java/org/ajoberstar/gradle/git/publish/tasks/GitPublishReset.java @@ -1,11 +1,12 @@ package org.ajoberstar.gradle.git.publish.tasks; -import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.URISyntaxException; import java.nio.file.Files; import java.util.Arrays; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -13,24 +14,33 @@ import org.ajoberstar.grgit.Grgit; import org.ajoberstar.grgit.Ref; +import org.eclipse.jgit.transport.URIish; import org.gradle.api.DefaultTask; +import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.FileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; +import org.gradle.api.file.ProjectLayout; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; -import org.gradle.api.tasks.*; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Internal; +import org.gradle.api.tasks.OutputDirectory; +import org.gradle.api.tasks.TaskAction; import org.gradle.api.tasks.util.PatternFilterable; public class GitPublishReset extends DefaultTask { private final Property grgit; + private final DirectoryProperty repoDirectory; + private final Property repoUri; private final Property branch; - private PatternFilterable preserve; @Inject - public GitPublishReset(ObjectFactory objectFactory) { + public GitPublishReset(ProjectLayout layout, ObjectFactory objectFactory) { this.grgit = objectFactory.property(Grgit.class); + this.repoDirectory = layout.directoryProperty(); + this.repoUri = objectFactory.property(String.class); this.branch = objectFactory.property(String.class); // always consider this task out of date @@ -42,6 +52,16 @@ public Property getGrgit() { return grgit; } + @OutputDirectory + public DirectoryProperty getRepoDirectory() { + return repoDirectory; + } + + @Input + public Property getRepoUri() { + return repoUri; + } + @Input public Property getBranch() { return branch; @@ -56,14 +76,11 @@ public void setPreserve(PatternFilterable preserve) { this.preserve = preserve; } - @OutputDirectory - public File getRepoDirectory() { - return getGrgit().get().getRepository().getRootDir(); - } - @TaskAction public void reset() { - Grgit git = getGrgit().get(); + Grgit git = findExistingRepo().orElseGet(() -> freshRepo()); + grgit.set(git); + String pubBranch = getBranch().get(); Map remoteBranches = git.lsremote(op -> { @@ -133,4 +150,49 @@ public void visitFile(FileVisitDetails fileVisitDetails) { op.setUpdate(true); }); } + + private Optional findExistingRepo() { + try { + return Optional.of(Grgit.open(op -> op.setDir(repoDirectory.get().getAsFile()))) + .filter(repo -> { + try { + String originUri = getOriginUri(repo); + // need to use the URIish to normalize them and ensure we support all Git compatible URI-ishs (URL + // is too limiting) + boolean valid = new URIish(repoUri.get()).equals(new URIish(originUri)) && branch.get().equals(repo.getBranch().current().getName()); + if (!valid) { + repo.close(); + } + return valid; + } catch (URISyntaxException e) { + throw new RuntimeException("Invalid URI.", e); + } + }); + } catch (Exception e) { + // missing, invalid, or corrupt repo + getProject().getLogger().debug("Failed to find existing Git publish repository.", e); + return Optional.empty(); + } + } + + private Grgit freshRepo() { + getProject().delete(repoDirectory.get().getAsFile()); + + Grgit repo = Grgit.init(op -> { + op.setDir(repoDirectory.get().getAsFile()); + }); + repo.getRemote().add(op -> { + op.setName("origin"); + op.setUrl(repoUri.get()); + }); + return repo; + } + + private String getOriginUri(Grgit grgit) { + return grgit.getRemote().list().stream() + .filter(remote -> remote.getName().equals("origin")) + .map(remote -> remote.getUrl()) + .findAny() + .orElse(null); + } }