Questions about custom gradle task property annotations

Help/Discuss
1

We have developed several custom gradle tasks and we have a debate about whether we need/should annotate properties that are configured in a gradle build script.

Here is an example:

public abstract class OmlMergeTask extends DefaultTask {

    public Collection<File> inputZipPaths;

    public void setInputZipPaths(Collection<File> files) {
    	inputZipPaths = files;
   		getInputFiles().from(files);
    }

    public Collection<File> inputFolderPaths = null;

    public void setInputFolderPaths(Collection<File> files) {
    	inputFolderPaths = files;
  		getInputFiles().from(files);
    }

    public Collection<File> inputCatalogPaths;

    public void setInputCatalogPaths(Collection<File> files) {
    	inputCatalogPaths = files;
  		getInputFiles().from(files);
    }

    @Incremental
    @InputFiles
    public abstract ConfigurableFileCollection getInputFiles();
  1. With Gradle 6.5.1, we can have a mix of properties with and without Gradle annotations.

Is this practice going to be supported in future versions of Gradle?

  1. It seems to me that the purpose of having defined gradle annotations is precisely to facilitate annotating every configurable property explicitly. Is this a matter of pedantic style, recommended best practice or necessity (e.g. gradle 7)?

  2. If we wanted to explicitly annotate inputZipPaths and inputFolderPaths, it is debatable which annotation is appropriate.

@Input seems to make sense except that there is a legitimate argument that these are used to calculate genuine inputs.

@Internal seems appropriate except that the doc says that the property isn’t taken into account for up-to-date checking. Yet in this case, the value certainly matters for up-to-date checking in that if the value were to change, we’d want the task to run even if the calculated input files were somehow to be the same. So this suggests that @Input is more appropriate.

Suggestions?

  • Nicolas.
2

No, except for private properties, each and every property needs an annotation or you get a complaint or later a failure.
This is to protect from adding a property and just forgetting to annotate it, leading to wrong up-to-date checks and faulty cache entries.
Each and every property needs to be annotated, so that someone thought about what annotation it should be, even if it turns out to be @Internal.

It is not only about configurable properties.
It is about all properties.
I for example have a task where one property is configurable and configures an input file.
But this input file can import other files which thus also are input files.
So I have a property that is not configurable, but that calculates these additional input files so that the up-to-date checks and build cache work properly.

As said above, afaik the requirement to annotate all properties is to safe-guard against forgetting to annotate a property and thus disturb up-to-date checks and caching. It is to protect you from shooting your own foot. And as it is enforced by Gradle 7 as the Gradle 6 warning tells, it is also not really relevant whether it is pedantic, recommended or whatever, you just have to do it. :slight_smile:

Not really debatable. :slight_smile:

@Input does not make sense and if I remember correctly will even throw a hard error at you.
@Input is not meant for file-based inputs, it is for other inputs that are serializable, like String inputs for example, or boolean inputs.

If you actually mean @InputFiles, then yes, this is the approprate annotation.
Those files are inputs to your task, if their configured value changes, or if some file in those configured directories changes, then your task should be out-of-date, shouldn’t it?
So they are clearly @InputFiles and should be annotated as such.

If for whatever reason (for example to be compatible with earlier releases where it needed to be separte) you would have these three input properties but they are treated together the same and you could also just have one property where all of them are configured, then you could indeed mark them @Internal as you already have them considered by annotating getInputFiles() with @InputFiles. But you actually don’t use getInputFiles() anywhere. Instead you use the single properties. It looks to me like you just have this getInputFiles() property to collect the input files for the up-to-date check. But imho this makes little sense, because if you remove a directory from one of the single properties and add it to the other, the getInputFiles() would have the same content and thus up-to-date and cache think the task does not need to re-run, while it has to rerun as you use the single properties for different parameter arguments.

Also it does not make much sense to annotate getInputFiles() with @Incremental when you then do not use it to implement incremental task execution so that only added / modified / removed files are processed. So the proper solution is probably to annotate the three properties with @InputFiles and remove the getInputFiles() method.