Identify and promote first-class support for mapping between extension and task properties about gradle

[ajoberstar]

As we won't promote the software model for the JVM domain, we'll need a different solution.

I'm hoping you mean "won't promote it yet"? Just want to make sure software model is still the direction.

[philmcardle]

Apologies, I'm 100% sure this isn't the appropriate place to ask this, but it's an open question for us too - was @ajoberstar's question answered anywhere?

The radical differences between the JVM domain and the software model are a cause of near-constant complexity for someone using Gradle for native, so it would be good to know which way Gradle is going.

[bmuschko]

I added a spec for this work. Feedback welcome!

@ajoberstar This work won't touch the software model. Convention mapping has been treated almost like a public API by plugin developers in the past years. I have done it, I am sure you have done it. There's a good reason for it: there were simply no better options. With the software model you can effectively avoid the evaluation order issue (addressed by convention mapping) by defining model rules.

[ajoberstar]

@bmuschko That makes sense to me, and I like the focus on getting some issues to round out the public APIs. My original question was more of an aside, and I don't know that it's quite been answered. Since this came up recently on the forum too, maybe that's a better place to continue if you don't mind taking a look.

[bmuschko]

@ajoberstar Thanks for raising the discussion. It's a valid concern for any plugin developer. Let's continue the discussion in the forum.

[adammurdoch]

I think there are a few options for making the 'provider' approach more convenient.

We can make some progress towards a more managed model and remove a bunch of boilerplate by allowing Gradle to manage the state for a property and to provide getter and setter implementations.

For example:

abstract class MyTask extends DefaultTask {
    @Input
    abstract String getSomeProp();
    abstract void setSomeProp(String value);
    abstract void setSomeProp(Provider<String> value);
}

Gradle can provide implementations for these methods. The current decoration that mixes in convention mapping basically already does this.

For backwards compatibility, the generated methods would still be convention mapping aware. So you can still do myTask.conventMapping.someProp = { "123" } so that myTask.someProp == "123".

No task types are currently abstract but can be extended. So it would be a breaking change to add abstract methods to an existing task type or to add abstract to existing task methods. We could use the approach we use to inject services via a getter, so that the task defines a stub method that is later overridden by the decoration. The downside of this approach is that a call to super.getSomeProp() in a task subtype will not be intercepted. Another option is that these methods might just delegate to conveniences provided by DefaultTask, and we change them to abstract methods in a major release.

We might get rid of the need for an additional setter that accepts Provider<T> by including an API on task that allows you to set a property value given a property name and a Provider:

abstract class MyTask extends DefaultTask {
    @Input
    abstract String getSomeProp();
    abstract void setSomeProp(String value);
}

// statically typed language
myTask.setProperty("someProp", someProvider);

// dynamically typed language
myTask.someProp = someProvider;

The point here is to avoid mixing a bunch of different concerns into the model (the task type).

For nested data structures, we should provide some kind of 'create a decorated instance of this type' feature.

One option would be to add a public API for Instantiator, so that the constructor for a class can be injected with an Instantiator and create whatever nested objects it needs, calling the setters to attach this. This, along with the managed state from above, would be applied recursively so that the desired object graph can be assembled.

Another, not mutually exclusive option, would be for Gradle to automatically create instances of things that it knows how to create: collection types, DomainObjectContainer types, types with an @Inject constructor, interfaces, etc.

[adammurdoch]

Digging a little further into the cases where abstract methods cannot be used:

The first option might look something like this:

class MyTask extends DefaultTask {
    @Input @Inject
    String getSomeProp() { throw new UnsupportedOperationException(); }
    @Inject
    void setSomeProp(String value) { throw new UnsupportedOperationException(); }
}

Gradle would override the getter and setter, as it does now, but would also manage the state of the property instead of delegating to the getter and setter.

However, this would be broken:

class MyCustomTask extends MyTask {
    void setSomeProp(String value) {
        value = ... some calculation ...
        super.setSomeProp(value);
   }
}

The second option might look something like this:

interface StateHolder<T> extends Provider<T> {
    void set(T value); // uses the provided value
    void set(Provider<? extends T> provider); // delegates to the provider
}

class MyTask extends DefaultTask {
    // an implementation is provided by Gradle and injected
    @Inject
    private final StateHolder<String> someProp;

    @Input
    String getSomeProp() { return someProp.get(); }

    // setters delegate to methods on the injected value
    void setSomeProp(String value) { someProp.set(value); }
    void setSomeProp(Provider<String> provider) { someProp.set(provider); }
}

There'd be some integration with the convention mapping infrastructure for backwards compatibility. We know which state object or field is associated with which property in both cases.

[adammurdoch]

Nested objects might work something like this:

abstract class SomeReport {
    abstract String getSomeProp();
    abstract void setSomeProp(String value);
}

// Using an instantiator
abstract class MyTask extends DefaultTask {
    private final SomeReport report;

    @Inject
    MyTask(Instantiator instantiator) {
        report = instantiator.newInstance(SomeReport.class);
    }

    @Nested
    public SomeReport getReport() { return report; }
}

// Injected via a field
abstract class MyTask extends DefaultTask {
    @Inject
    private final SomeReport report;

    @Nested
    SomeReport getReport() { return report; }
}

// Injected via a getter
abstract class MyTask extends DefaultTask {
    @Inject @Nested
    abstract SomeReport getReport();
}

A task might also want to hide the implementation type of a nested object:

// Using an instantiator
abstract class MyTask extends DefaultTask {
    private final SomeReportInternal report;

    @Inject
    MyTask(Instantiator instantiator) {
        report = instantiator.newInstance(SomeReportInteral.class);
    }

    @Nested
    public SomeReport getReport() { return report; }
}

// Injected via a field
abstract class MyTask extends DefaultTask {
    @Inject
    private final SomeReportInternal report;

    @Nested
    SomeReport getReport() { return report; }
}

// Injected via a getter
abstract class MyTask extends DefaultTask {
    @Inject @Nested
    abstract SomeReportInternal getReportInternal(); // would need to be package protected

    public SomeReport getReport() { return getReportInternal(); }
}

[adammurdoch]

Something to note is that these capabilities would not be specific to Task subtypes, but to any decorated type. In particular, this would mean that they are available for extension types.

[adammurdoch]

For unit testing these types, I think ProjectBuilder would work fine:

def project= ProjectBuilder.build()
def myTask = project.createTask("myTask", MyTask);

// do stuff

[bmuschko]

Thanks for your feedback, @adammurdoch.

Based on the feedback I got from other team members and you it seems like we feel comfortable with moving forward with the provider approach. Do you agree?

Your comments above focus on the convenience aspect of the implementation. I am wondering if we could just release the provider implementation in multiple phases. Phase 1 would not provide the aspect of convenience. I am proposing the approach for two reasons:

• Delivering a public API that solves the problem of providing lazily evaluated values will instantly make the life of plugin developers so much better. We'd also be able to get feedback from end users.
• I often times hear that users of Gradle would like to see less magic behavior. Generating getters/setter removes boilerplate code but it can also confuse users. Some people would rather be under full control than let Gradle sprinkle magic.

In practice I do not see people mixing convention mapping and provider approach in their plugins. I'd expect external plugins to fully switch over to the new approach and release a new version. The interoperability aspect is probably more useful for internal plugins which will migrate to the provider approach over time.

You also mentioned making the Instantiator a public API. That's definitely planned but as a separate story: #728.

[adammurdoch]

Yeah, I agree. We could (and should) certainly do this in stages.

I'm not seeing generated getters and setters as magic. It's pretty explicit that someone else has to provide the implementation. In this regard it's quite a bit better than convention mapping.

At this stage generated getters and setters are an important part of the performance strategy, as they allow us to cache state, reset state and make things immutable or parallel safe. Probably also part of the software model migration story as well. This pattern is intended to become pervasive in all domain object types.

In practice I do not see people mixing convention mapping and provider approach in their plugins

This would instead be important for people that reuse tasks types from other plugins, or who add or override mappings for tasks created by other plugins. Interop with convention mapping means that the plugin author can switch styles without affecting downstream users. And downstream users can switch styles without being blocked on the plugin author releasing a new version.

[bmuschko]

PR: #1452

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

[stale]

This issue has been automatically closed due to inactivity. If you can reproduce this on a recent version of Gradle or if you have a good use case for this feature, please feel free to reopen the issue with steps to reproduce, a quick explanation of your use case or a high-quality pull request.