Skip to content

Commit

Permalink
Remove memoization of Grgit
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ajoberstar committed Jun 6, 2018
1 parent 569413a commit f5add2e
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 123 deletions.
3 changes: 0 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 0 additions & 6 deletions gradle/dependency-locks/compile.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/compileClasspath.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/default.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/runtime.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/runtimeClasspath.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/testCompile.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/testCompileClasspath.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/testRuntime.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
6 changes: 0 additions & 6 deletions gradle/dependency-locks/testRuntimeClasspath.lockfile
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -39,38 +35,33 @@ public void apply(Project project) {

extension.getRepoDir().set(project.getLayout().getBuildDirectory().dir("gitPublish"));

Supplier<Grgit> grgitSupplier = Suppliers.memoize(() -> {
return findExistingRepo(project, extension).orElseGet(() -> freshRepo(project, extension));
});

Provider<Grgit> 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);

// 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<Grgit> 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.");
Expand All @@ -79,7 +70,7 @@ private Task createCopyTask(Project project, GitPublishExtension extension) {
});
}

private Task createCommitTask(Project project, GitPublishExtension extension, Provider<Grgit> grgitProvider) {
private GitPublishCommit createCommitTask(Project project, GitPublishExtension extension, Provider<Grgit> grgitProvider) {
return project.getTasks().create(COMMIT_TASK, GitPublishCommit.class, task -> {
task.setGroup("publishing");
task.setDescription("Commits changes to be published to git.");
Expand All @@ -88,7 +79,7 @@ private Task createCommitTask(Project project, GitPublishExtension extension, Pr
});
}

private Task createPushTask(Project project, GitPublishExtension extension, Provider<Grgit> grgitProvider) {
private GitPublishPush createPushTask(Project project, GitPublishExtension extension, Provider<Grgit> grgitProvider) {
return project.getTasks().create(PUSH_TASK, GitPublishPush.class, task -> {
task.setGroup("publishing");
task.setDescription("Pushes changes to git.");
Expand All @@ -97,43 +88,6 @@ private Task createPushTask(Project project, GitPublishExtension extension, Prov
});
}

private Optional<Grgit> 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"))
Expand Down
Loading

0 comments on commit f5add2e

Please sign in to comment.