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.

Sign up for GitHub

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

Open
wants to merge 16 commits into
base: development-4.x
Choose a base branch
from
Open

Resolve Gradle Configuration Caching Issues When Executing AsciidoctorTask #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
97e6e3d
Remove asciidoctorj from AbstractAsciidoctorTask
tylerbertrand Apr 12, 2024
008d6af
Add optionsProperty and attributesProperty to AbstractAsciidoctorTask…
tylerbertrand Apr 12, 2024
2ebef2a
Replace references to extension in task
tylerbertrand Apr 15, 2024
77ba88c
Solve CC issues - still not rendering document
tylerbertrand May 8, 2024
ba468e6
Set safemodeProperty and logLevelProperty
tylerbertrand May 8, 2024
a3ebec9
Beginning cleanup
tylerbertrand May 14, 2024
14442ba
Fixed detachedConfigurationCreator config cache issue
tylerbertrand May 20, 2024
42e0744
Removed commented-out detachedConfigurationCreator field
tylerbertrand Jun 19, 2024
acccec9
Remove classpath annotation, as the asciidoctorj extension didn't hav…
tylerbertrand Jun 20, 2024
17f6274
Remove commented-out code. It was replaced
tylerbertrand Jun 20, 2024
fe09ac0
Remove commented-out detachedConfigurationCreator. It has been moved …
tylerbertrand Jun 20, 2024
bc07f63
Add back asciidoctorj.configuration to the results of getReportableCo…
tylerbertrand Jun 20, 2024
28147cf
Make attributeProviderProperty protected
tylerbertrand Jun 20, 2024
47262ef
Fix codenarc issues
tylerbertrand Jun 20, 2024
e201779
Set version to 4.0.2-local
tylerbertrand Apr 12, 2024
680a6df
Update properties that were moved from AsciidoctorJExtension to Abstr…
tylerbertrand Jul 15, 2024
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
2 changes: 1 addition & 1 deletion gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version = 4.0.2
version = 4.0.2-local
group = org.asciidoctor

copyrightYear = 2013-2024
Expand Down
136 changes: 93 additions & 43 deletions jvm/src/main/groovy/org/asciidoctor/gradle/jvm/AbstractAsciidoctorTask.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 = []

private final MapProperty<String, Object> optionsProperty = project.objects.mapProperty(String, Object)
private final MapProperty<String, Object> attributesProperty = project.objects.mapProperty(String, Object)
private final ListProperty<AsciidoctorAttributeProvider> attributeProviderProperty = project.objects
.listProperty(AsciidoctorAttributeProvider)
private FileCollection configurationsFileCollection
private final ListProperty<Pattern> fatalWarningsProperty = project.objects.listProperty(Pattern)
private final ListProperty<String> requiresProperty = project.objects.listProperty(String)
private final Property<LogLevel> logLevelProperty = project.objects.property(LogLevel)
private final Property<SafeMode> safeModeProperty = project.objects.property(SafeMode)
private 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

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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()
}

/**
Expand Down Expand Up @@ -428,7 +470,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe
entrypoint {
classpath(JavaExecUtils.getJavaExecClasspath(
projectOperations,
configurations
configurationsFileCollection
))
}

Expand All @@ -440,7 +482,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe
)
} else {
entrypoint {
classpath(configurations)
classpath(configurationsFileCollection)
}
}
super.exec()
Expand All @@ -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)
Copy link

Choose a reason for hiding this comment

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

are we setting default values here? what does this block do?

Copy link
Owner Author

Choose a reason for hiding this comment

The 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 }
Expand Down Expand Up @@ -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
)
}

Expand All @@ -601,7 +647,7 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe
*/
@Internal
protected List<Object> getAsciidoctorJExtensions() {
asciidoctorj.docExtensions
docExtensionsProperty.get().stream().collect(Collectors.toList())
}

@Nested
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>

Expand All @@ -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()
Copy link

Choose a reason for hiding this comment

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

do we have to pass deps to this?

Copy link
Owner Author

Choose a reason for hiding this comment

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

deps doesn't need to be passed in here as it has been moved to within the provider

} 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(
Expand All @@ -750,26 +807,19 @@ class AbstractAsciidoctorTask extends AbstractJvmModelExecTask<AsciidoctorJvmExe
}
}

// TODO: Try to do this without a detached configuration
private FileCollection jrubyLessConfiguration(List<Dependency> deps) {
Copy link

@cdsap cdsap Jun 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

just FYI, in a recent conversation with the BT team, they mentioned:

detached configurations are probably not what you want. "detached" means that almost everything that 
comes from project state is separate.  We were going to deprecate detached configurations from extending
 project configurations,

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>
}
Expand Down
Loading