cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.91k stars 730 forks source link

Conditional task dependencies #514

Open jrnail23 opened 8 years ago

jrnail23 commented 8 years ago

I was just trying to figure out a way to do conditional dependencies between tasks, particularly as a means to get incremental build behavior.

It's not directly supported through the task DSL, but I figured I'd see if anyone has ideas on how I might achieve this. If not, maybe this would be a nice enhancement :smile:

Here's a simplified example of what I'm trying to do:

Let's say I have a Build task and a Package task. I only want to run the Build task if my bin is empty (or older than the source files, or whatever). Otherwise, I'll just package the artifacts that are already in the bin.

Obviously I can set up a WithCriteria on the Build task, but I don't really want to the Build task to be responsible for that, as I don't always want that rule to be applied every time the Build task is invoked -- I'd rather apply that condition contextually.

I actually expected that if a Task's WithCriteria isn't satisfied, that the tasks it depends on would not be invoked. That doesn't seem to be the case, though. The dependencies always get invoked, and only the task with the unsatisfied Criteria gets skipped.

Any thoughts?

patriksvensson commented 8 years ago

Ok, I've been thinking about your problem a little bit. Not sure if I understand it completely, but otherwise you should just stop me :smile:

Consider the following task graph, where C have a dependency on B that have a dependency on A. So if C is my target then tasks A, B and C will be executed in that specific order.

C -> B -> A

You said in your question:

I actually expected that if a Task's WithCriteria isn't satisfied, that the tasks it depends on would not be invoked. That doesn't seem to be the case, though. The dependencies always get invoked, and only the task with the unsatisfied Criteria gets skipped.

If I understand you correctly, you want A and B to be skipped if C's criteria isn't fulfilled? I don't think that is a good idea.

A and B do their thing in isolation, and just because C determines it doesn't have to be run, I don't think it should dictate whether or not B or A runs.

The other way around would be OK I think, e.g. if A is skipped, then B and C should be skipped as well, but then B and C knows that they're dependent on something that might never be run.

We could introduce something for this like WithPropagatingCriteria or something like that, but I think we should think about this some more before adding something. Changes like these are not easy to undo when people start relying on them so I would like us to be super sure about how we want stuff like this to behave.

Actually, when I think about it, I think that we should try to document the graph traversal and how different modifications to the tasks influence execution.

gep13 commented 8 years ago

@jrnail23 did you have any comments on @patriksvensson comments above? Is this still an issue? Thanks!

jrnail23 commented 8 years ago

@gep13, I'm just now catching up on old github notifications, so sorry for the slow reply. I've been out of the loop for quite a long time, and not using Cake anymore since moving to an all-javascript shop. Please feel free to close the issue, but I don't actually agree with @patriksvensson's assessment:

If I understand you correctly, you want A and B to be skipped if C's criteria isn't fulfilled? I don't think that is a good idea.

A and B do their thing in isolation, and just because C determines it doesn't have to be run, I don't think it should dictate whether or not B or A runs.

My thinking was that the nature of a task dependency graph suggests that A's explicit purpose is to satisfy B. If B isn't needed, you don't run A. (and so on with B & C). I'm assuming @patriksvensson has a fundamentally different opinion on this point, and that's quite OK.

And it's also quite possible that this sort of model only applies to certain kinds of logical tasks, and you guys may have some cases in mind that just don't fit the concept I'm presenting here.

I think the key capability I was looking for was doing incremental builds, where any unnecessary processing is completely avoided. Martin Fowler's Rake article presents this concept pretty well. Now that I'm reviewing it, maybe what I'm talking about is more in line with his discussion of file tasks.

Hopefully that adds something constructive to the discussion. 😄

BTW, good to see Cake still thriving, and I hope all is well with you guys!

liquidvapour commented 7 years ago

Hi,

We have just started using Cake and I agree with @jrnail23.

Took us one day to hit this issue. Would you accept a pull request that 'fixed' this?

patriksvensson commented 7 years ago

@liquidvapour See my previous reply to the original thread (12, Nov, 2015). This is by design.

pascalberger commented 7 years ago

Having the same issue.

If I understand you correctly, you want A and B to be skipped if C's criteria isn't fulfilled? I don't think that is a good idea.

A and B do their thing in isolation, and just because C determines it doesn't have to be run, I don't think it should dictate whether or not B or A runs.

IMHO there should be a way to have a conditional dependency. Which doesn't mean that B or A won't run, but that B or A won't run just for C. If another task depends on B or A they still should be run.

Our use case is that we have one task which runs a gulp script and one task which restores npm packages. Running gulp script depends on having npm packages restored, but it doesn't make sense to restore npm packages, when it is clear that we won't run the gulp script. Since the npm restore tasks is of generic use I won't repeat the criteria there which is on the task running the gulp script.

devlead commented 7 years ago

Before dependecy graph is changed in any way, "WeakDependencies" perhaps could be something introduced as a alias/method to executed before RunTarget basically it could check for task existence by iterating over the ICakeEngine.Tasks and if both exist add the dependency.

jonstelly commented 6 years ago

Reviving an old topic, but this is exactly what I'm running into. I can appreciate the logic for the current behavior and don't think changing it is right, but finding some way to express this would be helpful. For me:

if A.IsDependentOn(B) and B.WithCriteria(false).IsDependentOn(C).IsDependentOn(D)

I'm only running C and D to support B, so if B isn't going to run, I don't need or want to run C or D. If I wanted them to run always, I would just have A depend on C and D.

Some overload of something like:

IsDependentOn(string taskName, bool condition)
IsDependentOn(string taskName, Func<bool> condition)

would be a big help and help organize the conditions better... For the reasoning of why I wouldn't mark C or D WithCriteria, I may want to run those tasks explicitly ./build.ps1 -Target C. So the criteria isn't associated with Task C or D in my mind, but with task B.

For a more concrete example, a build script with an argument to also package. This is the best option I've found to support this today, and imagine 4+ Package sub-targets, it starts getting unwieldy:

var target = Argument("target", "Default");
var package = Argument("package", target.StartsWith("Package"));

Task("Default")
    .IsDependentOn("Build")
    .IsDependentOn("Package");

Task("Build")
    .Does(() => {
        //...
    });

Task("Package")
    .WithCriteria(package)
    .IsDependentOn("Package.Server")
    .IsDependentOn("Package.Client");

Task("Package.Server")
    .WithCriteria(package)
    .Does(() => {
        //...
    });

Task("Package.Client")
    .WithCriteria(package)
    .Does(() => {
        //...
    });

With an overload for IsDependentOn:

var target = Argument("target", "Default");
var package = Argument("package", false);

Task("Default")
    .IsDependentOn("Build")
    .IsDependentOn("Package", package);

Task("Build")
    .Does(() => {
        //...
    });

Task("Package")
    .IsDependentOn("Package.Server")
    .IsDependentOn("Package.Client");

Task("Package.Server")
    .Does(() => {
        //...
    });

Task("Package.Client")
    .Does(() => {
        //...
    });

Alternatively, I've started playing around with some -Target tomfoolery:

./build.ps1 -Target 'Build;Package'

and then:

var targets = Argument("target", "Default").Split(';');

//...

foreach(var target in targets) { 
    RunTarget(target);
}

The problem here is that multiple RunTarget() calls means that if two targets share a dependency, the dependency runs multiple times. An option there could be something like a RunTargets(targets) call that can combine multiple targets into a single tree, taking duplicate dependencies into account.

@devlead and @patriksvensson would you be open to take a pull request for either of these? The IsDependentOn overload seems really easy to implement so I'd be happy to go give that a shot.

reisenberger commented 6 years ago

To solve a scenario similar to that outlined by @jonstelly - a whole group of tasks grouped under the umbrella "package", with one bool controlling go/no-go for that group - I did the following, for now:

Task("_ConditionallyPackage")
    .WithCriteria(() => package)
    .Does(() => RunTarget("_Package"));

In wider context:

Task("Build")
    .IsDependentOn("_BuildAndTest")
    .IsDependentOn("_ConditionallyPackage");

Task("_BuildAndTest")
    .IsDependentOn("__Clean")
    .IsDependentOn("__RestoreNugetPackages")
    .IsDependentOn("__BuildSolutions")
    .IsDependentOn("__RunTests");

Task("_Package")
    .IsDependentOn("__CopyOutputToNugetFolder")
    .IsDependentOn("__CreateNugetPackage");

Task("_ConditionallyPackage")
    .WithCriteria(() => package) // bool value package calculated somewhere earlier ...
    .Does(() => RunTarget("_Package"));

Not offering this as a recommendation, just an example of another workround. I agree that some native syntax would be better.

( @patriksvensson , it was good to meet you at tretton37 in November :smile: )

patriksvensson commented 6 years ago

@reisenberger Stumbled across this issue by accident. Somehow missed your message when you wrote it. It was on my birthday though, so I might have been preoccupied 😄

That is an excellent idea and something I've been thinking about as well. It was good meeting you too!

rdavisunr commented 6 years ago

@reisenberger - This looks like a passable workaround, but it seems to cause Setup to run again (if it was defined).

jden123 commented 5 years ago

I've faced with the same case. I see that some of my tasks group smaller tasks and define a flow. In this case it's useful to cancel child tasks if main task WithCriteria isn't satisfied.

I think we can have a switch like SkipDependentTasksOnCriteriaFail to change behavior.

Task("Main")
    .WithCriteria(false)
    .SkipDependentTasksOnCriteriaFail()
    .IsDependentOn("Subtask1")
    .IsDependentOn("Subtask2");
devlead commented 5 years ago

@jden123 As criteria can be lazy and dependent on child tasks result, it can lead to unforeseen consequences, complexities, SkipDependentTasksOnCriteriaFail would essentially have to evaluate criteria, which would change behavior. Maybe ok, but something to be aware of.

Now that we have typed context, criteria could easily be shared between tasks, that might be a possible workaround to have intent in a shared sate. https://cakebuild.net/blog/2018/05/cake-v0.28.0-released i.e.

Task("Subtask1")
    .WithCriteria<MyBuildData>(
        (context, data) => data.MainCriteria && data.SubtaskCriteria
        );

If main is the entry task perhaps better to validate everything in setup and fail fast there.

string target = Argument("target", "Main");

Setup(setupContext =>
{
    if (setupContext.TargetTask.Name == "Main" &&
        !setupContext.HasArgument("MainCriteria"))
    {
        throw new Exception("Aborting target \"Main\", but \"--MainCriteria\" not specified");
    }
});

Task("Subtask1")
    .Does(()=>{});

Task("Subtask2")
    .Does(()=>{});

Task("Main")
    .IsDependentOn("Subtask1")
    .IsDependentOn("Subtask2");

RunTarget("Main");

then cake .\typedcontextfail.cake --MainCriteria will succeed but just cake .\typedcontextfail.cake will fail. image

jden123 commented 5 years ago

@devlead, thank you for your feedback. If subtask can lead to main task condition fail that can add complexity.

In my case I have task "tree" where small tasks(actions) are grouped into big task that are group into the biggest :)

Task -> Task1(condition 1) -> Subtask11
                           -> Subtask12

     -> Task2(condition 2) -> Subtask21
                           -> Subtask22

Now, as a workaround, I build task based on criterias like

var task = Task("Task");

if (doTask1)
{
    task.IsDependentOn("Task1");
}

if (doTask2)
{
    task.IsDependentOn("Task2");
}
FrankRay78 commented 1 year ago

Please assign this issue to me @devlead, I'd like to work on it.