Methods part of incremental build of Copy, Zip, Jar and similar tasks should be rejected in execution phase

Help/Discuss
1

We have a huge number of Gradle modules and many Jar tasks mostly creating “fat JARs”. This is causing the configuration phase to take quite a lot of time.

So a colleague of mine came up with an “optimization” to move the from() and manifest() method calls into doFirst() method:

task copyStuff(type: Jar) {
    doFirst {
        from configurations.xyz
        from zipTree(some_custom_jar)

        manifest {
            attributes {
                "Class-Path": configurations.runtime.collect { it.toURI() } .join(' ')
            }
        }
        ...
    }
}

This is bad for many reasons:

  1. the manifest content is not taken into account for incremental builds
  2. the changes/additions/removals of any from() method call are not taken into account for incremental builds
  3. if the task would be part of the default jar definition, then some files will be part of incremental build (class files from main sourceSet) but any custom files not, causing hidden consistency mismatch

Generally speaking allowing methods part of incremental build to be called in execution phase might cause many issues and should be rejected.

I was wondering if such behavior could be rejected by throwing an exception during execution phase.

2

Hi Martin,

the above code should already fail with Gradle 4.0. You can try out Gradle 4.0 M2 and give it a spin. We already print a deprecation message in the Gradle 3.x series.

I think you should be able to have both - fast configuration time and correct incremental build.
The easiest solution for your problem would be to add a different task which configures copyStuff - a so called configure task. So you would end up with something like


task copyStuff(type: Jar) {
    dependsOn 'configureCopyStuff'
}

task configureCopyStuff {
    doFirst {
        copyStuff.from configurations.xyz
        copyStuff.from zipTree(some_custom_jar)

        copyStuff.manifest {
            attributes {
                "Class-Path": configurations.runtime.collect { it.toURI() } .join(' ')
            }
        }
        ...
    }
}
3

Can you explain why this should work? I am not following.

From what I can see:

  1. at configuration time only the dependency between copyStuff and configureCopyStuff is created
  2. no from() block is called at configuration time, so the incremental build would not work correctly

Also when I execute the sampled build script in Gradle 3.5, no warning is printed and it works without errors in Gradle 4.0-milestone 2.

4

The configuration task has the same sort of problems that you mentioned above (namely, missing dependencies).

From your example above, the only expensive part is

configurations.runtime.collect { it.toURI() } .join(’ ')

The calls to from() are OK and correct. They’re lazily evaluated already, so putting these into doFirst is unnecessary and could lead to problems. I think we should try to prevent that.

In this particular case, what we’re missing is an easy way to lazily evaluate manifest attributes. We call toString on any attribute value, so we should be able to fake it. I haven’t tried this, but something like this may work:

class ClasspathFromConfiguration {
    Configuration configuration

    ClasspathFromConfiguration(Configuration configuration) {
        this.configuration = configuration
    }

    String toString() {
        configuration.collect { it.toURI() } .join(' ')
    }
}

task copyStuff(type: Jar) {
    dependsOn configurations.runtime

    from configurations.xyz
    from zipTree(some_custom_jar)

    manifest {
        attributes {
            "Class-Path": new ClasspathFromConfiguration(configurations.runtime)
        }
    }
}
5

I had the same problem with lazy evaluation of classpath for manifest attributes. I found this solution and after further search, it think it should be possible to use lazy string evaluation like this (lazy expressions):

manifest {
        attributes {
            "${ -> configuration.collect { it.toURI() } .join(' ') }"
        }
}