On 3 Sep 2014, at 9:45 am, Luke Daley <luke.da...@gradleware.com> wrote:
>
>
> On 3 September 2014 at 5:52:44 am, Adam Murdoch (adam.murd...@gradleware.com)
> wrote:
>
>>
>> On 3 Sep 2014, at 2:29 am, Marcin Erdmann <marcin.erdm...@proxerd.pl> wrote:
>>
>>> While working on the new configuration model Luke and I have noticed that
>>> we are not locking task container after the configuration phase so the
>>> following does not throw an error (even though it essentially has no useful
>>> effect):
>>>
>>> task foo {
>>> doFirst {
>>> tasks.create("bar")
>>> }
>>> }
>>>
>>> As implemented at the moment tasks references are added to the new
>>> configuration model based on the task container contents at the end of the
>>> configuration phase therefore it probably makes sense to prevent any more
>>> modifications to the container after that point in time.
>>
>> It makes sense to prevent (or warn about) pointless creation of tasks. We do
>> have to be a little careful about when we lock the task container for a
>> given project, for backwards compatibility reasons.
>>
>> Currently, any build script can define a task in any other project, so we
>> can’t lock until after every project build script has been executed and the
>> afterEvaluate() events fired. We could certainly deprecate this behaviour,
>> but we still need to support it.
>
> The story that triggered this is about defining the lifecycle WRT the
> “configuration phase” and the new model rule stuff. At the moment we
> interleave the two, which we want to now change.
>
> What I want to do is register the tasks into the model space _just_ before
> the task graph is built, and do this via iteration instead of .all().
>
You should make sure you don’t trigger the placeholder actions when doing this.
These could be translated across to creation rules without being triggered.
> This is cleaner, and avoids some awkwardness with task removal/replacement
> during the configuration phase. If we do this, we should ensure that no tasks
> are yet to be created. So, it’s at this point I want to “lock” the task
> container.
>
The problem is that it’s too early. There are some examples below.
From a usability point of view, we need to avoid 2 things:
1. Tasks that are created that can never be executed.
2. Tasks that are created after some predicate that they match has been closed
in the rules.
They’re really 2 separate problems, and should be solved in separate ways.
The first one would be solved by closing the task container for a project after
the dag has been finalised for that project (however that dag happens to have
been built).
The second one would be solved by listening for changes to the task container.
>
>
> This leaves only two “holes” for tasks to still be added: during task
> dependency traversal (a Buildable input could create a task in its
> getTaskDependency() imll)
>
In configure-on-demand mode, the target projects are configured during task
dependency traversal. The logic for the target project could, but is not
supposed to, create tasks in the current project.
An example of task creation during task dependency traversal would be tasks
created by the old rules added via NamedDomainObjectCollection.addRule():
clean.dependsOn(“cleanJavaCompile”)
> and during execution time proper. I can’t think of any valid reasons to do
> either of those. I suspect that creating tasks during the creation of the
> task graph is going to lead to CMEs at worst, and is undefined at best.
> Probably the same for creating tasks at execution time.
>
> Three options as I see them:
>
> 1. Once we start building the task graph, have all task containers warn that
> adding tasks at this time is deprecated but still try and add it
>
> 2. Once we start building the task graph, have all task containers warn that
> adding tasks at this time is not support it and discard the task
>
> 3. Once we start building the task graph, have all task containers error if
> someone tries to add a task
>
> Given how edge case this is, I’m for jumping straight to 3 as a breaking
> change.
>
I think we need to lock on a per-project basis, otherwise configure-on-demand
is going to be broken.
--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
CTO Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com
