-
Notifications
You must be signed in to change notification settings - Fork 0
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
Resolve Gradle Configuration Caching Issues When Executing AsciidoctorTask #1
base: development-4.x
Are you sure you want to change the base?
Changes from 14 commits
97e6e3d
008d6af
2ebef2a
77ba88c
ba468e6
a3ebec9
14442ba
42e0744
acccec9
17f6274
fe09ac0
bc07f63
28147cf
47262ef
e201779
680a6df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import org.asciidoctor.gradle.base.AsciidoctorTaskFileOperations | |
import org.asciidoctor.gradle.base.AsciidoctorTaskMethods | ||
import org.asciidoctor.gradle.base.AsciidoctorTaskOutputOptions | ||
import org.asciidoctor.gradle.base.AsciidoctorTaskWorkspacePreparation | ||
import org.asciidoctor.gradle.base.SafeMode | ||
import org.asciidoctor.gradle.base.Transform | ||
import org.asciidoctor.gradle.base.internal.DefaultAsciidoctorBaseDirConfiguration | ||
import org.asciidoctor.gradle.base.internal.DefaultAsciidoctorFileOperations | ||
|
@@ -38,13 +39,16 @@ import org.asciidoctor.gradle.internal.ExecutorUtils | |
import org.asciidoctor.gradle.internal.JavaExecUtils | ||
import org.asciidoctor.gradle.remote.AsciidoctorJavaExec | ||
import org.gradle.api.Action | ||
import org.gradle.api.GradleException | ||
import org.gradle.api.artifacts.Configuration | ||
import org.gradle.api.artifacts.ConfigurationContainer | ||
import org.gradle.api.artifacts.Dependency | ||
import org.gradle.api.file.FileCollection | ||
import org.gradle.api.logging.LogLevel | ||
import org.gradle.api.provider.ListProperty | ||
import org.gradle.api.provider.MapProperty | ||
import org.gradle.api.provider.Property | ||
import org.gradle.api.provider.Provider | ||
import org.gradle.api.tasks.Classpath | ||
import org.gradle.api.tasks.Input | ||
import org.gradle.api.tasks.Internal | ||
import org.gradle.api.tasks.Nested | ||
|
@@ -60,6 +64,8 @@ import org.ysb33r.grolifant.api.core.runnable.AbstractJvmModelExecTask | |
import org.ysb33r.grolifant.api.remote.worker.WorkerAppExecutorFactory | ||
|
||
import java.util.function.Function | ||
import java.util.regex.Pattern | ||
import java.util.stream.Collectors | ||
|
||
import static org.asciidoctor.gradle.base.AsciidoctorUtils.getClassLocation | ||
import static org.asciidoctor.gradle.base.internal.AsciidoctorAttributes.evaluateProviders | ||
|
@@ -93,17 +99,53 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
public final static Severity WARN = Severity.WARN | ||
public final static Severity INFO = Severity.INFO | ||
|
||
protected final AsciidoctorJExtension asciidoctorj | ||
private ExecutionMode inProcess | ||
private Severity failureLevel = Severity.FATAL | ||
private final List<Object> asciidocConfigurations = [] | ||
private final File rootDir | ||
private final File projectDir | ||
private final File execConfigurationDataFile | ||
private final Function<List<Dependency>, Configuration> detachedConfigurationCreator | ||
private final Property<FileCollection> jvmClasspath | ||
private final List<Provider<File>> gemJarProviders = [] | ||
|
||
protected final MapProperty<String, Object> optionsProperty = project.objects.mapProperty(String, Object) | ||
protected final MapProperty<String, Object> attributesProperty = project.objects.mapProperty(String, Object) | ||
protected final ListProperty<AsciidoctorAttributeProvider> attributeProviderProperty = project.objects | ||
.listProperty(AsciidoctorAttributeProvider) | ||
protected FileCollection configurationsFileCollection | ||
protected final ListProperty<Pattern> fatalWarningsProperty = project.objects.listProperty(Pattern) | ||
protected final ListProperty<String> requiresProperty = project.objects.listProperty(String) | ||
protected final Property<LogLevel> logLevelProperty = project.objects.property(LogLevel) | ||
protected final Property<SafeMode> safeModeProperty = project.objects.property(SafeMode) | ||
protected final ListProperty<Object> docExtensionsProperty = project.objects.listProperty(Object) | ||
|
||
private final Provider<FileCollection> jrubyLessDependenciesProvider = project.provider { | ||
List<Dependency> deps = this.docExtensionsProperty.get().findAll { | ||
it instanceof Dependency | ||
} as List<Dependency> | ||
|
||
def detachedConfigurationCreator = { ConfigurationContainer c, List<Dependency> d -> | ||
final cfg = c.detachedConfiguration(d.toArray() as Dependency[]) | ||
cfg.canBeConsumed = false | ||
cfg.canBeResolved = true | ||
cfg | ||
}.curry(project.configurations) as Function<List<Dependency>, Configuration> | ||
|
||
Configuration cfg = detachedConfigurationCreator.apply(deps) | ||
asciidoctorJExtension.loadJRubyResolutionStrategy(cfg) | ||
cfg | ||
} as Provider<FileCollection> | ||
|
||
private final Provider<Map<String, Map<String, Object>>> attributesByLangProvider = project.provider { | ||
def attributesByLang = [:] | ||
languagesAsOptionals.each { lang -> | ||
if (lang.present) { | ||
attributesByLang.put(lang.get(), asciidoctorJExtension.getAttributesForLang(lang.get())) | ||
} | ||
} | ||
attributesByLang | ||
} as Provider<Map<String, Map<String, Object>>> | ||
|
||
@Delegate | ||
private final DefaultAsciidoctorFileOperations asciidoctorTaskFileOperations | ||
|
||
|
@@ -222,7 +264,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
*/ | ||
@Input | ||
Map<String, Object> getOptions() { | ||
resolveAsCacheable(asciidoctorj.options, projectOperations) | ||
resolveAsCacheable(optionsProperty.get(), projectOperations) | ||
} | ||
|
||
/** Apply a new set of Asciidoctor options, clearing any options previously set. | ||
|
@@ -234,7 +276,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
* @param m Map with new options | ||
*/ | ||
void setOptions(Map m) { | ||
asciidoctorj.options = m | ||
optionsProperty.set(m) | ||
} | ||
|
||
/** Add additional asciidoctor options | ||
|
@@ -246,7 +288,8 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
* @param m Map with new options | ||
*/ | ||
void options(Map m) { | ||
asciidoctorj.options(m) | ||
checkForAttributesInOptions(m) | ||
this.optionsProperty.get().putAll(m) | ||
} | ||
|
||
/** Returns all of the Asciidoctor options. | ||
|
@@ -256,7 +299,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
*/ | ||
@Input | ||
Map<String, Object> getAttributes() { | ||
resolveAsCacheable(asciidoctorj.attributes, projectOperations) | ||
resolveAsCacheable(attributesProperty.get(), projectOperations) | ||
} | ||
|
||
/** Apply a new set of Asciidoctor options, clearing any options previously set. | ||
|
@@ -268,7 +311,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
* @param m Map with new options | ||
*/ | ||
void setAttributes(Map m) { | ||
asciidoctorj.attributes = m | ||
attributesProperty.set(m) | ||
} | ||
|
||
/** Add additional asciidoctor options | ||
|
@@ -280,7 +323,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
* @param m Map with new options | ||
*/ | ||
void attributes(Map m) { | ||
asciidoctorj.attributes(m) | ||
attributesProperty.putAll(m) | ||
} | ||
|
||
/** Additional providers of attributes. | ||
|
@@ -292,7 +335,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
*/ | ||
@Internal | ||
List<AsciidoctorAttributeProvider> getAttributeProviders() { | ||
asciidoctorj.attributeProviders | ||
attributeProviderProperty.get().stream().collect(Collectors.toList()) | ||
} | ||
|
||
/** Returns all of the specified configurations as a collections of files. | ||
|
@@ -301,11 +344,10 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
* | ||
* @return FileCollection | ||
*/ | ||
@Classpath | ||
@SuppressWarnings('Instanceof') | ||
FileCollection getConfigurations() { | ||
FileCollection getConfigurations(AsciidoctorJExtension asciidoctorJExtension) { | ||
final precompiledExtensions = findDependenciesInExtensions() | ||
FileCollection fc = this.asciidocConfigurations.inject(asciidoctorj.configuration) { | ||
FileCollection fc = this.asciidocConfigurations.inject(asciidoctorJExtension.configuration) { | ||
FileCollection seed, Object it -> | ||
seed + projectOperations.configurations.asConfiguration(it) | ||
} | ||
|
@@ -348,8 +390,8 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
*/ | ||
@Override | ||
Set<Configuration> getReportableConfigurations() { | ||
([asciidoctorj.configuration] + projectOperations.configurations.asConfigurations(asciidocConfigurations)) | ||
.toSet() | ||
([asciidoctorJExtension.configuration] + projectOperations.configurations | ||
.asConfigurations(asciidocConfigurations)).toSet() | ||
} | ||
|
||
/** | ||
|
@@ -428,7 +470,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
entrypoint { | ||
classpath(JavaExecUtils.getJavaExecClasspath( | ||
projectOperations, | ||
configurations | ||
configurationsFileCollection | ||
)) | ||
} | ||
|
||
|
@@ -440,7 +482,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
) | ||
} else { | ||
entrypoint { | ||
classpath(configurations) | ||
classpath(configurationsFileCollection) | ||
} | ||
} | ||
super.exec() | ||
|
@@ -467,19 +509,23 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
asciidoctorTaskFileOperations | ||
) | ||
this.baseDirConfiguration = new DefaultAsciidoctorBaseDirConfiguration(project, this) | ||
this.asciidoctorj = extensions.create(AsciidoctorJExtension.NAME, AsciidoctorJExtension, this) | ||
|
||
def asciidoctorj = extensions.create(AsciidoctorJExtension.NAME, AsciidoctorJExtension, this) | ||
this.optionsProperty.set(asciidoctorj.options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we setting default values here? what does this block do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, setting default values |
||
this.safeModeProperty.set(asciidoctorj.safeMode) | ||
this.attributesProperty.set(asciidoctorj.attributes) | ||
this.attributeProviderProperty.set(asciidoctorj.attributeProviders) | ||
this.configurationsFileCollection = getConfigurations(asciidoctorj) | ||
this.fatalWarningsProperty.set(asciidoctorj.fatalWarnings) | ||
this.requiresProperty.set(asciidoctorj.requires) | ||
this.logLevelProperty.set(asciidoctorj.logLevel != null ? asciidoctorj.logLevel : LogLevel.INFO) | ||
this.docExtensionsProperty.set(asciidoctorj.docExtensions) | ||
|
||
this.projectDir = project.projectDir | ||
this.rootDir = project.rootDir | ||
this.jvmClasspath = project.objects.property(FileCollection) | ||
this.execConfigurationDataFile = getExecConfigurationDataFile(this) | ||
this.detachedConfigurationCreator = { ConfigurationContainer c, List<Dependency> deps -> | ||
final cfg = c.detachedConfiguration(deps.toArray() as Dependency[]) | ||
cfg.canBeConsumed = false | ||
cfg.canBeResolved = true | ||
cfg | ||
}.curry(project.configurations) as Function<List<Dependency>, Configuration> | ||
|
||
inputs.files(this.asciidoctorj.configuration) | ||
inputs.files { gemJarProviders }.withPathSensitivity(RELATIVE) | ||
inputs.property 'backends', { -> backends() } | ||
inputs.property 'asciidoctorj-version', { -> asciidoctorj.version } | ||
|
@@ -585,13 +631,13 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
), | ||
backendName: backendName, | ||
logDocuments: logDocuments, | ||
fatalMessagePatterns: asciidoctorj.fatalWarnings, | ||
fatalMessagePatterns: fatalWarningsProperty.get().stream().collect(Collectors.toList()), | ||
asciidoctorExtensions: serializableAsciidoctorJExtensions, | ||
requires: asciidoctorj.requires, | ||
requires: requiresProperty.get().stream().collect(Collectors.toList()), | ||
copyResources: copyResources.present && | ||
(copyResources.get().empty || backendName in copyResources.get()), | ||
executorLogLevel: ExecutorUtils.getExecutorLogLevel(asciidoctorj.logLevel), | ||
safeModeLevel: asciidoctorj.safeMode.level | ||
executorLogLevel: ExecutorUtils.getExecutorLogLevel(logLevelProperty.get()), | ||
safeModeLevel: safeModeProperty.get().level | ||
) | ||
} | ||
|
||
|
@@ -601,7 +647,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
*/ | ||
@Internal | ||
protected List<Object> getAsciidoctorJExtensions() { | ||
asciidoctorj.docExtensions | ||
docExtensionsProperty.get().stream().collect(Collectors.toList()) | ||
} | ||
|
||
@Nested | ||
|
@@ -634,6 +680,17 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
} | ||
} | ||
|
||
@Internal | ||
private AsciidoctorJExtension getAsciidoctorJExtension() { | ||
extensions.getByName(AsciidoctorJExtension.NAME) as AsciidoctorJExtension | ||
} | ||
|
||
private void checkForAttributesInOptions(Map m) { | ||
if (m.containsKey('attributes')) { | ||
throw new GradleException('Attributes found in options. Please use \'attributes\' method') | ||
} | ||
} | ||
|
||
private List<Object> getSerializableAsciidoctorJExtensions() { | ||
asciidoctorJExtensions.findAll { !(it instanceof Dependency) }.collect { | ||
getSerializableAsciidoctorJExtension(it) | ||
|
@@ -703,7 +760,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
|
||
@SuppressWarnings('Instanceof') | ||
private FileCollection findDependenciesInExtensions() { | ||
List<Dependency> deps = asciidoctorj.docExtensions.findAll { | ||
List<Dependency> deps = this.docExtensionsProperty.get().findAll { | ||
it instanceof Dependency | ||
} as List<Dependency> | ||
|
||
|
@@ -719,19 +776,19 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
if (deps.empty && closurePaths.empty) { | ||
projectOperations.fsOperations.emptyFileCollection() | ||
} else if (closurePaths.empty) { | ||
jrubyLessConfiguration(deps) | ||
jrubyLessDependenciesProvider.get() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we have to pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} else if (deps.empty) { | ||
projectOperations.fsOperations.files(closurePaths) | ||
} else { | ||
jrubyLessConfiguration(deps) + projectOperations.fsOperations.files(closurePaths) | ||
jrubyLessDependenciesProvider.get() + projectOperations.fsOperations.files(closurePaths) | ||
} | ||
} | ||
|
||
private void handleGradleClosureInstrumentation(Set<File> closurePaths) { | ||
// Jumping through hoops to make docExtensions based upon closures to work. | ||
closurePaths.add(getClassLocation(org.gradle.internal.scripts.ScriptOrigin)) | ||
if (LegacyLevel.PRE_8_4 && !LegacyLevel.PRE_7_6) { | ||
closurePaths.add(getClassLocation(org.gradle.api.GradleException)) | ||
closurePaths.add(getClassLocation(GradleException)) | ||
} | ||
if (LegacyLevel.PRE_8_4 && !LegacyLevel.PRE_8_3) { | ||
closurePaths.add(getInternalGradleLibraryLocation( | ||
|
@@ -750,26 +807,19 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe | |
} | ||
} | ||
|
||
// TODO: Try to do this without a detached configuration | ||
private FileCollection jrubyLessConfiguration(List<Dependency> deps) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just FYI, in a recent conversation with the BT team, they mentioned:
|
||
Configuration cfg = detachedConfigurationCreator.apply(deps) | ||
asciidoctorj.loadJRubyResolutionStrategy(cfg) | ||
cfg | ||
} | ||
|
||
private Map<String, Object> preparePreserialisedAttributes(final File workingSourceDir, Optional<String> lang) { | ||
prepareAttributes( | ||
projectOperations.stringTools, | ||
attributes, | ||
(lang.present ? asciidoctorj.getAttributesForLang(lang.get()) : [:]), | ||
(lang.present ? attributesByLangProvider.get().getOrDefault(lang.get(), [:]) : [:]), | ||
getTaskSpecificDefaultAttributes(workingSourceDir) as Map<String, ?>, | ||
attributeProviders, | ||
lang | ||
) | ||
} | ||
|
||
private List<Closure> findExtensionClosures() { | ||
asciidoctorj.docExtensions.findAll { | ||
this.docExtensionsProperty.get().findAll { | ||
it instanceof Closure | ||
} as List<Closure> | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these
protected
instead ofprivate
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had run into some unknown property exception issues that I thought were resolved by this, but after testing now, it works just fine with them
private
, so I've made that change.