-
-
Notifications
You must be signed in to change notification settings - Fork 724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplified task graph configuration #26
Comments
While graph is correct, wouldn't |
I'm not sure if I understand what you mean :/ Task("Clean")
.Does(() =>
{
});
Task("UpdateAssemblyInfo")
.WithCriteria(() => !isLocalBuild)
.IsDependentOn("Clean")
.Does(() =>
{
});
Task("Build")
.IsDependentOn("A")
.Does(() =>
{
}); What I'm proposing is another way of declaring the dependency graph: Task("Clean")
.Does(() =>
{
});
Task("UpdateAssemblyInfo")
.Does(() =>
{
});
Task("Build")
.Does(() =>
{
});
// Declare dependencies.
Graph("Clean")
.Then("Build");
.Then("OtherTask1");
.Then("OtherTask2");
if(!isLocalBuild)
{
Graph("Clean")
.Then("UpdateAssemblyInfo");
} With a model like this, you could theoretically tell Cake to run tasks is parallel: Graph("Clean")
.Then("Build");
.ThenInParallel("OtherTask1", "OtherTask2"); This is just an example. I find the This would (like I've said many times before) need some more thought 😄 |
Ahh, I think I was misunderstanding this slightly. I was assuming this as a secondary way to define targets for building. Like: Task("Clean")
.Does(() =>
{
/* Clean Things */
});
Task("UpdateAssemblyInfo")
.Does(() =>
{
/* Update CommonAssemblyInfo.cs */
});
Task("Build")
.Does(() =>
{
/* Build the project. */
});
// Declare dependencies.
if(!isLocalBuild)
{
Target("Default")
.Do("Clean");
.Then("UpdateAssemblyInfo")
.Then("Build")
.Then("Package");
}
else
{
Target("Default")
.Do("Clean")
.Then("Build");
}
RunTarget("Default"); NVM then ^_^ |
What's the status of this feature? Looks appealing to me 👍 |
I'm not sure if this should be implemented (therefore the investigate label). It seemed like a good idea at the time, but I'm not that sure that it would contribute anything except introducing more complexity. I might be wrong and I'm not 100% against it, but I think the API should be discussed a little bit more before doing anything since it affects how Cake behaves. Any good use cases for this functionality? |
I agree that it hardly adds any value to the project, but as a "syntactic sugar" - looks nice 😋 In proposed example the workflow is all at a glance, so if build order is changed it is much easier to move couple of lines up/down, rather than fixing And to me, semantically it also looks like better way to. First you define tasks as granular units of work; next - you combine them in a workflow (or multiple workflows). Specifying execution criteria for the task makes it stick to the task. So, say if you want to execute same task, but in another workflow/build configuration where that criteria doesn't apply you have 2 options: write a new task with different criteria, or update the criteria itself with some conditional logic depending to context of execution. ExampleCurrent wayTask("A")
.WithCriteria( ()=> !someCondition)
.Does(()=>
{
// some actions here
});
Task("B")
.IsDependentOn("A")
.Does(()=>
{
// some actions here
});
Task("C")
.Does(()=>
{
// some actions here
});
// this wants task A to execute only if !someCondition
Target("Build_Config_A")
.IsDependentOn("B")
.IsDependentOn("C");
// this wants task A to execute regardless of 'someCondition',
Target("Build_Config_B")
.IsDependentOn("B")
.IsDependentOn("C");
// so to be able to acheive that, will have to duplicate task but exclude condition
// or update the condition itself to tak execution context into account somehow.
// Even more, we might have a config that, wants task B to be executed, but not w/o Task A
// being executed before it
Target("Build_Config_C")
.IsDependentOn("B") // just B! not dep. on task "A"
.IsDependentOn("C"); Now a better way (in my opinion 😄 )var config = Argument<string>("config", "Build_Config_B");
Task("A")
.Does(()=>
{
// some actions here
});
Task("B")
.Does(()=>
{
// some actions here
});
Task("C")
.Does(()=>
{
// some actions here
});
Workflow("Build_Config_A").
Do("A", If (() => !someCondition))
.Then("B")
.Then("C");
Workflow("Build_Config_B")
.Do("A")
.Then("B")
.Then("C");
Workflow("Build_Config_C")
.Do("B")
.Then("C");
RunWorkflow(config); As you mentioned above, this style also gives credit to possible parallel execution (in future), and passing a some sort of "execution result" object to the next task (which is "piping" in a nutshell, some already mentioned that in other discussion) |
Great feedback. I would say that workflows as you suggest (great name suggestion btw) is something else than what's proposed in this issue. Although I like the idea. Since the simplified graph configuration i proposed above is more like a way of adding dependencies in reverse, have I understood this correctly if I say that workflows is a way to compose tasks - also in reverse - but in many different ways and isolated from other workflows? Some questions that pop up:
|
Yes. I consider "workflow" as single unique path to accomplish build.
I had troubles trying to come-up with a good use case for that, so - no, workflow should not be dependent on other workflow(s).
No again. My vision here is that task, actually should not have dependencies (so as conditions) at all. That is, there should be no I understand, that what I wrote above "hurts" a bit at first, because removing aforementioned methods is a huge breaking change, but we're just talking it out loud, anyway, right? 😉 And the conversation topic has drifted from original one, cause now I realize I speak more about alternative Cake API. What-ifWe would have an experimental namespace |
I've been thinking about this today and I cannot see that we would remove task dependencies or criteria from Cake. I appreciate the ideas, but I'm extremely reluctant to include two execution models that are not compatible with each other. I like the idea of workflows though, and I can see a benefit from them if they respect task dependencies and criteria, which would make them a new kind of mechanism for task execution. This way we could accomplish what you propose simply by not specifying dependencies or criteria for a task. This is the only option I see since we don't want to deprecate the current functionality or ignore it. I'm sorry if I sound negative, but it's so easy to add stuff and so difficult to remove them. I've already made that mistake in this project so I'm trying to avoid another one. Especially something that would affect core functionality. We could add a new issue for this and continue the discussion since this issue has been a little bit derailed 😄 |
Rather than baking a different workflow in you could simply create a
Would be
I am not sure, but if Does have an overload which accepts a task you can do parallel with:
I agree that cake should not move away from the graph approach it has now, that way the engine can add parallelism down the track without changing build scripts. |
I ran into a bug today in my project that is related to the way the dependency graph works today. It is a great point of contention for me and my team on how to write scripts because of this. For example I have a script that has the following targets: Task("restore").Does(() => { //restore code });
Task("build-sources").Does(() => { //build code });
Task("get-translations").Does(() => { // get translation code} );
Task("run-tests").Does(() => { //run tests });
Task("zip-artifacts").Does(() => { // zipping code }); I then have 2 rollup tasks to generate the graph: Target("build")
.IsDependentOn("restore")
.IsDependentOn("get-translations")
.IsDependentOn("build-sources")
.IsDependentOn("run-tests")
.IsDependentOn("zip-artifacts"); Target("cc")
.IsDependentOn("restore")
.IsDependentOn("get-translations")
.IsDependentOn("build-sources")
.IsDependentOn("run-tests"); The problem came in when somebody on my team asked "Why don't the core tasks have IsDependentOn() calls?". And, then went about adding them into the tasks. During this he added a Task("get-translations")
.IsDependentOn("build-sources")
.Does(() => { //some stuff here }); This changed the call order from: To: Which caused builds for the last 2 weeks to go out the door with stale translations. What I expected should happen was a cyclical dependency error, but I suppose cake doesn't get that info from the IsDependentOn() calls for the build target. TLDR; I would prefer a cleaner dependency graph solution to whats offered now. And, many of the ideas floating in this issue remind me of how FAKE does it. I think Cake would benefit greatly from ripping out the old system and replacing it with a clear dependency structure. |
@rbutcher We will not change this behavior. First it would be a massive breaking change and second is that FAKE is actually the tool that does this differently. The "normal" way that people coming from a Make, Rake or MSBuild background would expect is to define targets and their dependencies, not the explicit execution order. I opened this issue to provide an alternative to the default way of building the dependency graph, for simple scenarios. |
Simplified task graph configuration.
The text was updated successfully, but these errors were encountered: