Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2085a05
Replace eager task access and afterEvaluate patterns with lazy task r…
rpalcolea Dec 3, 2025
1270cd4
Removed Eager .get() Call from SystemPackagingTask
rpalcolea Dec 3, 2025
85618a1
Improve lazy evaluation with Provider API and configuration avoidance
rpalcolea Dec 3, 2025
0faf5ea
Extract magic number 420 to named constant DEFAULT_FILE_PERMISSION
rpalcolea Dec 3, 2025
3ee34dc
Extract setgid bit magic number 02000 to named constant
rpalcolea Dec 3, 2025
986045a
Optimize duplicate method call in Rpm addParentDirs mapping
rpalcolea Dec 3, 2025
4d30f43
Simplify verbose ternary operators with Elvis operator in directory h…
rpalcolea Dec 3, 2025
2cbbbf8
Convert getByName() to named() for lazy task resolution
rpalcolea Dec 3, 2025
41f20f2
Extract Unix permission mask constants for improved readability
rpalcolea Dec 3, 2025
408edac
Leverage Groovy truthiness for collection emptiness checks
rpalcolea Dec 3, 2025
a6fd9d1
Extract duplicate trigger handling into helper method
rpalcolea Dec 3, 2025
34b8884
Use TaskProvider for task dependencies instead of String names
rpalcolea Dec 3, 2025
863ff58
Replace verbose Action<T> syntax with idiomatic Groovy closures
rpalcolea Dec 3, 2025
96fc7c1
Simplify Callable<String> to closure in Provider API
rpalcolea Dec 3, 2025
7d28015
Convert SystemPackagingExtension to concrete Property fields
rpalcolea Dec 3, 2025
b6a5753
Fix Property to primitive type conversions in convention mappings
rpalcolea Dec 3, 2025
6830d2b
Fix Property conversions in Deb and Rpm convention mappings
rpalcolea Dec 3, 2025
3706042
Fix ListProperty and MapProperty conversions in Deb and Rpm tasks
rpalcolea Dec 3, 2025
45da4c1
Add @Internal annotation to ObjectFactory getter
rpalcolea Dec 3, 2025
590a4d0
Fix Property finalization in daemon plugin
rpalcolea Dec 3, 2025
b9675b2
Phase 1: Remove deprecated ConventionMapping API
rpalcolea Dec 3, 2025
69a99c6
Phase 2: Eliminate Task.project references
rpalcolea Dec 3, 2025
0fe1444
Phase 3: Fix eager Property access
rpalcolea Dec 3, 2025
b9dbecf
Enable configuration cache support for gradle-ospackage-plugin
rpalcolea Dec 3, 2025
463614e
Clean up: Remove commented notCompatibleWithConfigurationCache lines
rpalcolea Dec 3, 2025
e1d633c
Enable full configuration cache support
rpalcolea Dec 3, 2025
7c90267
Use providers.fileContents() for config-cache-safe File handling
rpalcolea Dec 4, 2025
14ba1f2
Fix validation tracking and test configuration cache compatibility
rpalcolea Dec 4, 2025
a05b187
Replace deprecated conventionMapping with Property API
rpalcolea Dec 4, 2025
701fcfc
Add comprehensive configuration cache tests for Debian packaging
rpalcolea Dec 4, 2025
bf09fb3
Fix Property API usage in OspackageApplicationDaemonPlugin
rpalcolea Dec 4, 2025
f825db5
Fix configuration avoidance in OspackageDaemonPlugin
rpalcolea Dec 4, 2025
628e32e
Fix eager task realization in OspackageApplicationDaemonPlugin
rpalcolea Dec 4, 2025
e605948
Fix DaemonTemplateTask outputs for configuration cache compatibility
rpalcolea Dec 4, 2025
a01759a
Fix configuration cache tests to check actual functionality
rpalcolea Dec 4, 2025
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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ out
*.iml
*.iws
.idea
.claude/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

.claude is intended to be checked in, so maybe:

*.local.json
*.local.md

https://code.claude.com/docs/en/memory#determine-memory-type

4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ plugins {
id 'java-library'
}

description 'Provides a task similar to Tar and Zip for constructing RPM and DEB package files.'
description = 'Provides a task similar to Tar and Zip for constructing RPM and DEB package files.'

compileGroovy.groovyOptions.configurationScript = file('src/groovyCompile/groovycConfig.groovy')

Expand Down Expand Up @@ -57,7 +57,7 @@ dependencies {
exclude group: 'org.codehaus.plexus', module: 'plexus-utils'
}

implementation 'com.bmuschko:gradle-docker-plugin:3.2.1'
implementation 'com.bmuschko:gradle-docker-plugin:10.0.0'
testImplementation 'com.github.stefanbirkner:system-rules:1.19.0'
}

Expand Down
2 changes: 1 addition & 1 deletion gradle.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Manual edits can break the build and are not advised.
# This file is expected to be part of source control.
cglib:cglib-nodep:3.2.2=integTestRuntimeClasspath,testRuntimeClasspath
com.bmuschko:gradle-docker-plugin:3.2.1=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
com.bmuschko:gradle-docker-plugin:10.0.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
com.github.luben:zstd-jni:1.5.6-3=integTestRuntimeClasspath,runtimeClasspath,testRuntimeClasspath
com.github.stefanbirkner:system-rules:1.19.0=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
com.netflix.nebula:nebula-test:11.0.0=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import groovy.transform.CompileDynamic
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.plugins.ApplicationPlugin
import org.gradle.api.tasks.application.CreateStartScripts
import org.gradle.api.plugins.JavaApplication

/**
* Combine the nebula-ospackage-application with the nebula-ospackage-daemon plugin. As with the nebula-ospackage-application,
Expand All @@ -46,24 +46,23 @@ class OspackageApplicationDaemonPlugin implements Plugin<Project> {
project.plugins.apply(OspackageApplicationPlugin)
def ospackageApplicationExtension = project.extensions.getByType(OspackageApplicationExtension)

CreateStartScripts startScripts = (CreateStartScripts) project.tasks.getByName(ApplicationPlugin.TASK_START_SCRIPTS_NAME)

project.plugins.apply(OspackageDaemonPlugin)

// Mechanism for user to configure daemon further
List<Closure> daemonConfiguration = []
setApplicationDaemon(project, daemonConfiguration)

// TODO Convention mapping on definition instead of afterEvaluate
// Keep afterEvaluate to wait for user configuration of applicationName
project.afterEvaluate {
// TODO Sanitize name
def name = startScripts.applicationName
// Use strongly-typed application extension instead of eager task realization
JavaApplication appExtension = project.extensions.getByType(JavaApplication)
def name = appExtension.applicationName ?: project.name

// Add daemon to project
DaemonExtension daemonExt = project.extensions.getByType(DaemonExtension)
def definition = daemonExt.daemon { DaemonDefinition daemonDefinition ->
daemonDefinition.setDaemonName(name)
daemonDefinition.setCommand("${ospackageApplicationExtension.prefix}/${name}/bin/${name}".toString())
daemonDefinition.setCommand("${ospackageApplicationExtension.prefix.get()}/${name}/bin/${name}".toString())
}

daemonConfiguration.each { confClosure ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@

package com.netflix.gradle.plugins.application

class OspackageApplicationExtension {
String prefix
import org.gradle.api.provider.Property

String distribution
interface OspackageApplicationExtension {
Property<String> getPrefix()
Property<String> getDistribution()
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import org.gradle.api.Action
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.internal.IConventionAware
import org.gradle.api.plugins.ApplicationPlugin

/**
Expand All @@ -45,36 +44,41 @@ class OspackageApplicationPlugin implements Plugin<Project> {
OspackageApplicationExtension extension

@Override
@CompileDynamic
void apply(Project project) {
extension = project.extensions.create('ospackage_application', OspackageApplicationExtension)
def conventionMapping = ((IConventionAware) extension).conventionMapping
conventionMapping.map('prefix') { '/opt' }
conventionMapping.map('distribution') { '' }

// Set convention (default) values using Property API

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd have Claude strip these comments that are obvious.

extension.prefix.convention('/opt')
extension.distribution.convention('')

project.plugins.apply(ApplicationPlugin)
project.plugins.apply(SystemPackagingPlugin)

project.afterEvaluate {
final installTask = project.tasks.getByName("install${extension.distribution.capitalize()}Dist")
def packagingExt = project.extensions.getByType(ProjectPackagingExtension)
packagingExt.from {
installTask.outputs.files.singleFile.parent
}
def packagingExt = project.extensions.getByType(ProjectPackagingExtension)

packagingExt.into(extension.getPrefix())
// Configure packaging extension - use provider for lazy evaluation
packagingExt.from(project.provider {
def distributionName = extension.distribution.getOrElse('')
def installTask = project.tasks.named("install${distributionName.capitalize()}Dist")
installTask.get().outputs.files.singleFile.parent
})

linkInstallToPackageTask(project, Deb, installTask)
linkInstallToPackageTask(project, Rpm, installTask)
}
// Pass the Property directly to into()
packagingExt.into(extension.prefix.map { it })

// Link install task to package tasks lazily
linkInstallToPackageTasks(project, Deb)
linkInstallToPackageTasks(project, Rpm)
}

@CompileDynamic
private <T extends Class> void linkInstallToPackageTask(Project project, T type, Task installTask) {
project.tasks.withType(type).configureEach(new Action<SystemPackagingTask>() {
@Override
void execute(SystemPackagingTask task) {
task.dependsOn(installTask)
}
})
private <T extends Class> void linkInstallToPackageTasks(Project project, T type) {
project.tasks.withType(type).configureEach { SystemPackagingTask task ->
// Use TaskProvider for type-safe lazy task dependency
def distributionName = extension.distribution.getOrElse('')
def installTaskName = "install${distributionName.capitalize()}Dist"
task.dependsOn(project.tasks.named(installTaskName))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,16 @@ class OspackageApplicationSpringBootPlugin implements Plugin<Project> {
void apply(Project project) {
project.plugins.apply(OspackageApplicationPlugin)

// Validate Spring Boot plugin at configuration time using plugins.withId
// This automatically waits for plugin application
boolean springBootFound = false
project.plugins.withId("org.springframework.boot") {
springBootFound = true
}

// Only validate after other plugins have a chance to apply
project.afterEvaluate {
if (!project.plugins.hasPlugin('org.springframework.boot')) {
if (!springBootFound) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Believe this can be project.plugins.withId(...) {, you don't need to do that suffle with springBootFound. withId evaluates immediately if it's already applied IIRC.

project.logger.error("The '{}' plugin requires the '{}' plugin.",
"com.netflix.nebula.ospackage-application-spring-boot",
"org.springframework.boot")
Expand Down Expand Up @@ -137,8 +145,10 @@ class OspackageApplicationSpringBootPlugin implements Plugin<Project> {
main {
contents {
into('lib') {
project.getConfigurations().getByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME).files.findAll { file ->
file.getName() != project.tasks.getByName(JavaPlugin.JAR_TASK_NAME).outputs.files.singleFile.name
def jarTaskProvider = project.tasks.named(JavaPlugin.JAR_TASK_NAME)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can use filter here and avoid calling files() so we're not eagerly resolving the configuration.

def runtimeClasspath = project.configurations.named(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME)
runtimeClasspath.get().files.findAll { file ->
file.getName() != jarTaskProvider.get().outputs.files.singleFile.name
}.each { file ->
exclude file.name
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,52 @@

package com.netflix.gradle.plugins.daemon

import org.gradle.api.internal.ConventionTask
import org.gradle.api.DefaultTask
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFile
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Optional
import org.gradle.api.tasks.OutputFiles
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction
import org.gradle.work.DisableCachingByDefault

/**
* Monster class that does everything.
*/
@DisableCachingByDefault
class DaemonTemplateTask extends ConventionTask {

DaemonTemplateTask() {
notCompatibleWithConfigurationCache("nebula.ospackage does not support configuration cache")
}
abstract class DaemonTemplateTask extends DefaultTask {

@Internal
Map<String, String> context
abstract MapProperty<String, Object> getContext()

@Internal
Collection<String> templates
abstract ListProperty<String> getTemplates()

@OutputDirectory
abstract DirectoryProperty getDestDir()

@Internal
File destDir
abstract Property<String> getTemplatesFolder()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be a DirectoryProperty or a straight Directory too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good question! I kept it as Property<String> because I think templatesFolder serves 2 uses cases:

  1. It's primarily a classpath resource path (default: /com/netflix/gradle/plugins/daemon)
  2. It can also be a custom file system path (absolute or relative)

And then the TemplateHelper.getTemplateContent() uses getResourceAsStream() to load from classpath so the directory property wouldn't work here


@Internal
String templatesFolder
abstract Property<File> getProjectDirectory()

@TaskAction
def template() {
TemplateHelper templateHelper = new TemplateHelper(getDestDir(), getTemplatesFolder(), project)
getTemplates().collect { String templateName ->
templateHelper.generateFile(templateName, getContext())
}
DaemonTemplateTask() {
// Capture project directory during configuration
projectDirectory.convention(project.projectDir)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

}

@Internal
Collection<File> getTemplatesOutput() {
return templates.collect {
new File(destDir, it)
@TaskAction
def template() {
TemplateHelper templateHelper = new TemplateHelper(
destDir.get().asFile,
templatesFolder.get(),
projectDirectory.get()
)
templates.get().collect { String templateName ->
templateHelper.generateFile(templateName, context.get())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class OspackageDaemonPlugin implements Plugin<Project> {
String cleanedName = daemonName.replaceAll("\\W", "").capitalize()


File outputDir = new File(project.layout.buildDirectory.getAsFile().get(), "daemon/${cleanedName}/${task.name}")
def outputDirProvider = project.layout.buildDirectory.dir("daemon/${cleanedName}/${task.name}")

String defaultInitDScriptLocationTemplate = isRedhat ? "/etc/rc.d/init.d/\${daemonName}" : "/etc/init.d/\${daemonName}"
Map<String, String> templatesWithFileOutput = [
Expand All @@ -101,33 +101,54 @@ class OspackageDaemonPlugin implements Plugin<Project> {
'initd': defaultDefinition.initDScriptLocation ?: defaultInitDScriptLocationTemplate
]

DaemonTemplateTask templateTask = project.tasks.create("${task.name}${cleanedName}Daemon".toString(), DaemonTemplateTask)
templateTask.conventionMapping.map('destDir') { outputDir }
templateTask.conventionMapping.map('templatesFolder') { daemonTemplatesConfigExtension.folder ?: DEFAULT_TEMPLATES_FOLDER }
templateTask.conventionMapping.map('context') {
Map<String,Object> context = toContext(defaults, definition)
context.daemonName = daemonName
context.isRedhat = isRedhat
context.installCmd = definition.installCmd ?: LegacyInstallCmd.create(context)
context
def templateTaskProvider = project.tasks.register("${task.name}${cleanedName}Daemon", DaemonTemplateTask) {
// Use Property API instead of conventionMapping
it.destDir.convention(outputDirProvider)
it.templatesFolder.convention(daemonTemplatesConfigExtension.folder ?: DEFAULT_TEMPLATES_FOLDER)
it.context.convention(project.provider {
Map<String,Object> context = toContext(defaults, definition)
context.daemonName = daemonName
context.isRedhat = isRedhat
context.installCmd = definition.installCmd ?: LegacyInstallCmd.create(context)
context
})
it.templates.convention(templatesWithFileOutput.keySet() + POST_INSTALL_TEMPLATE)
}
templateTask.conventionMapping.map('templates') { templatesWithFileOutput.keySet() + POST_INSTALL_TEMPLATE }

task.dependsOn(templateTask)
task.dependsOn(templateTaskProvider)
templatesWithFileOutput.each { String templateName, String destPathTemplate ->
File rendered = new File(outputDir, templateName) // To be created by task, ok that it's not around yet
String destPath = getDestPath(destPathTemplate, templateTask)
// Gradle CopySpec can't set the name of a file on the fly, we need to do a rename.
int slashIdx = destPath.lastIndexOf('/')
String destDir = destPath.substring(0,slashIdx)
String destFile = destPath.substring(slashIdx+1)
configureTask(task, rendered, destDir, destFile)
// Use lazy providers to avoid eager task realization
def renderedFileProvider = outputDirProvider.map { dir ->
new File(dir.asFile, templateName)
}

def destPathProvider = templateTaskProvider.flatMap { templateTask ->
project.provider {
getDestPath(destPathTemplate, templateTask)
}
}

// Configure task with lazy providers
configureTaskLazily(task, renderedFileProvider, destPathProvider)
}

task.doFirst {
File postInstallCommand = new File(outputDir, POST_INSTALL_TEMPLATE)
task.postInstall(postInstallCommand.text)
}
// Add postInstall content from generated template
// Use providers.fileContents() which is configuration-cache-safe
def postInstallFileProvider = project.layout.buildDirectory.file(
"daemon/${cleanedName}/${task.name}/${POST_INSTALL_TEMPLATE}"
)

// Use fileContents provider which properly handles file reading for config cache
def postInstallContentProvider = project.providers.fileContents(postInstallFileProvider)
.asText
.orElse('')

// Add the file content to postInstallCommands
task.exten.postInstallCommands.addAll(
postInstallContentProvider.map { String content ->
content?.trim() ? [content] : []
}
)
}
}
}
Expand All @@ -142,14 +163,33 @@ class OspackageDaemonPlugin implements Plugin<Project> {
}
}

@CompileDynamic
private void configureTaskLazily(SystemPackagingTask task, def renderedFileProvider, def destPathProvider) {
task.from(renderedFileProvider) {
// Use closures for lazy evaluation during copy execution
into({
String destPath = destPathProvider.get()
int slashIdx = destPath.lastIndexOf('/')
destPath.substring(0, slashIdx)
})
rename({ String filename ->
String destPath = destPathProvider.get()
int slashIdx = destPath.lastIndexOf('/')
destPath.substring(slashIdx + 1)
})
FilePermissionUtil.setFilePermission(it, 0555)
user 'root'
}
}

@CompileDynamic
private void addDaemonToProject(Project project, Closure closure) {
project.ext.daemon = closure
}

private String getDestPath(String destPathTemplate, DaemonTemplateTask templateTask) {
GStringTemplateEngine engine = new GStringTemplateEngine()
def destPath = engine.createTemplate(destPathTemplate).make(templateTask.getContext()).toString()
def destPath = engine.createTemplate(destPathTemplate).make(templateTask.getContext().get()).toString()
destPath
}

Expand Down
Loading