Delaying zip task configuration in plugin (SOLVED)

plugins

(Mark Fisher) #1

I’ve got a simple task working in a build to delay configuration of a zip task until the run phase like this:

task delayedConfigSetter {
	doLast {
		tasks.createStaticAssetArchive {
			destinationDir project.buildEnv.artefactDir
			from "${project.buildEnv.projectDir}/foo/bar/assets"
		}
	}
}

task createStaticAssetArchive(type: Zip) {
	dependsOn loadBuildEnv, delayedConfigSetter
	archiveName 'staticAssets.zip'
}

I need to turn this into a plugin task, and I’m trying to simplify it so there’s no delayedConfigSetter type hack, instead the dynamic values are delayed to the action phase of the task.

I’ve tried the following, but it’s not firing the action, which is the bit I think I need to fix.

class CreateArchiveInArtefact extends Zip {
	@Input String projectPath

	CreateArchiveInArtefact() {
		dependsOn project.tasks.loadBuildEnv
		// ... various exclude blocks etc
	}

	@TaskAction action() {
		def fromFile = project.file("${project.buildEnv.projectDir}/${projectPath}")
		from fromFile
		destinationDir = project.buildEnv.artefactDir
		copy()
	}
}

And then a simpler configuration (which I can also create many of) is:

createStaticAssetArchive {
	archiveName 'staticAssets.zip'
	projectPath 'foo/bar/assets'
}

I see in the build info log that my task is being skipped: “as it has no source files and no previous output files”

I tried adding “outputs.upToDateWhen { false }” to the class instantiation, but it doesn’t change the skipped status of the task.

What’s the correct way to extend a zip task and delay the configuration to the action phase so I can programatically set it like I did in the original “delayedConfigSetter” task? After all, that’s running its action to set the tasks values, so I’d like to keep it all simple and in one reusable task.

Thanks for any help in this.


(Stefan Wolf) #2

Hi Mark,

the Zip task has different input and output properties which are annotated so that it can determine if it is up to date or not. The up to date checks run before the task action is executed - that is the point, since we want to avoid executing the task action.
In your task the action is modifying the input and the output properties of the Zip task - but at this point up to date checks already finished. And since the Zip task has not been configured, there are no files to zip - so the task is marked as up to date.
In order to achieve the desired effect consider calling from in the setter of projectPath.

Cheers,
Stefan


(Mark Fisher) #3

Thanks Stefan.

EDIT: discovered this is creating 2 archives, one in the project’s master dir (destinationDir unset), and second when the destinationDir is set correctly in action() and copy() called.

That’s certainly got it working, i’ve changed it to the following and the action is now firing.

@Input List<String> projectPaths
void setProjectPaths(List<String> projectPaths) {
	this.projectPaths = projectPaths
	List<File> files = projectPaths.collect { project.file("${project.buildEnv.projectDir}/$it") }
	from files
}
// ...
@TaskAction action() {
	destinationDir = project.buildEnv.artefactDir
	copy()
}

I was honestly expecting this to fail, as I didn’t think my ‘project.buildEnv’ variable was being set early enough for the set method, but it appears it is, so all is good.

Thanks again for the help.


(Stefan Wolf) #4

One thing which could still be improved is that you can actually add a closure as a from clause to the copy spec. This closure will be evaluated lazily.
So you could for example try to do:

CreateArchiveInArtefact() {
    from {
        projectPaths.collect { project.file("${project.buildEnv.projectDir}/$it")) }    
    }
}

This would remove the need for overwriting the setter and would not cause duplicate from clauses when calling the setter more than once. Note that I did not try it out, so it could be that it needs some tuning.


(Mark Fisher) #5

I had to change my approach as I needed to specify an ‘into’ for each projectPath, and switched to creating a copySpec specifying ‘from’ and ‘into’, and including it using ‘with’:

void setProjectPaths(List<String> projectPaths) {
	this.projectPaths = projectPaths

	projectPaths.each { String p ->
		File f = project.file("${project.buildEnv.projectDir}/$p")
		with ProjectUtil.dirCopySpec(project, f)
	}
}
// ProjectUtil.groovy:
static CopySpec dirCopySpec(Project project, File dir) {
	project.copySpec {
		// creates a dir of same name as the directory being copied, e.g. "/foo/assets/*" copied into "assets/"
		into dir.name
		from dir
	}
}

I tried your suggestion of moving the from block into the constructor, and although this worked as is, I couldn’t figure out how to do a lazy ‘into’ in this way, so the archive was effectively flattened and no good. Also the archive is being generated in wrong dir, more on this below…

However, another issue I’ve discovered is that the zip is being generated twice, the first is generated presumably by the superclass (I put a sleep in action() at top and it’s already created at this point) using a null destinationDir (so created in the project’s master directory), and the second version when the copy() method is called in action() after setting destinationDir. Because destinationDir is a value I don’t know until the tasks dependencies are run, I can’t set it in the constructor, it’s another value that needs to be evaluated lazily.

Do you know why the superclass is creating a version before I call copy()?


(James Justinic) #6

Task actions are usually cumulative. That copy() method from the AbstractCopyTask has a @TaskAction annotation as well, so both your action() which calls copy() and the original copy() action added by the superclass are executed. The task design here expects that you will customize the configuration in subclasses, but not do your own orchestration of the mechanics. In a case where there’s really no other option (I’m not convinced you’re there yet), if you change your action() method to override copy() and call super.copy() instead, you should only have the copy() performed once.

This might help if you want to see exactly what actions you have:

this.actions.each {
    def action = it.action
    logger.debug "${action}: ${action.hasProperty('method') ? action.method : ''}"
}

(Mark Fisher) #7

Thanks for the comments, I shall try this out tomorrow.

The key problem in my own design is that the value project.buildEnv.artefactDir is calculated in a task generated in a rule. I spent a considerable amount of time today trying to understand the order of events.

In my case I have:

  • plugins initialised
  • classes for tasks created <-- value not yet set
  • rules match task being run and set variable i need
  • tasks run

What I’ve done for now is add an interface with a method to say the value is now set, and in my rules i call that method on all tasks that implement that interface that need the value. This happens just before the tasks run, so is ideal for now and removes the workaround I had that removed the erroneous zip generated by the action in the super class (thanks for the confirmation on this btw).

I’ll try your suggestion of overriding out tomorrow and see if it fares better, as then I’ll be able to remove the interface mechanism I currently have to get around the issue.


(Mark Fisher) #8

@jjustinic - thanks for your comments, overriding the copy method worked perfectly. For reference of anyone googling this problem, my final code looks like:

class CreateArchiveInArtefact extends Zip {
	@Input List<String> projectPaths

	void setProjectPaths(List<String> projectPaths) {
		this.projectPaths = projectPaths

		projectPaths.each { String p ->
			File f = project.file("${project.buildEnv.projectDir}/$p")
			with ProjectUtil.dirCopySpec(project, f)
		}
	}

	@Override
	protected void copy() {
		destinationDir = project.buildEnv.artefactDir
		super.copy()
	}

}

With the utility class:

class ProjectUtil {
	static CopySpec dirCopySpec(Project project, File dir) {
		project.copySpec {
			into dir.name
			from dir
			exclude '**/.svn'
			// etc.
		}
	}
}

I’ve marked this reply as the ‘solution’, but again thanks to both @Stefan_Wolf and @jjustinic for their comments that arrived at the solution.