feat(devenv-tasks): support dependency pruning with execIf and execIfModified#2673
feat(devenv-tasks): support dependency pruning with execIf and execIfModified#2673allonhadaya-maven wants to merge 2 commits into
Conversation
| let env = self | ||
| .prepare_env(outputs, shell_env) | ||
| .wrap_err("Failed to prepare task environment")?; |
There was a problem hiding this comment.
The initialization of env has been lifted out of the status_after block, and is being reused for task execution below. Is it sound to reuse the same instance of env instead of creating separate instances like we were previously?
|
We should keep current behavior and add |
@domenkozar thanks for taking a look at the proposal, and providing the above guidance! Please correct me if I'm wrong, but what I'm understanding is that maintaining API stability is your primary concern with the proposal? That sounds reasonable to me.
I gave this a good deal of thought, and wanted to run some ideas by you: So one little detail I wasn't clear on in your feedback was whether "top-level" task refers to the dependent task or the dependency task. To make this next part concrete, I'll reference the "Motivating Example" and discuss both possible interpretations: Option A: "top-level task" means
|
|
Thanks for the thorough analysis of the options! I think there is a simpler path that avoids the breaking change entirely: keep The core feature here is skip propagation, which is valuable. But it does not require changing
This way the feature is purely additive, no breaking change, no migration friction, and users opt in to the pruning benefit by using the new option for checks that do not need dependency outputs. What do you think? |
322f643 to
0b6e44d
Compare
…Modified This change defines a new task option `execIf` and extracts `execIfModified` into a first pass that runs in reverse-topological order, from dependents to dependencies. Tasks whose dependents are all skipped are pruned; short-circuiting evaluation of their checks.
1d42d66 to
aae3629
Compare
|
@domenkozar thanks for your followup guidance! PR is ready for review along the lines you suggested 🙏 |
I'll take a look at this soon. |
|
Thanks @domenkozar! This branch is based on pretty old |
This change extracts task
statusandexec-if-modifiedchecks into a first pass that runs in reverse-topological order, from dependents to dependencies. Tasks whose dependents are all skipped are pruned from the DAG. Tasks whose dependents are all skipped short-circuiting evaluation of their checks.Breaking change: Status checks no longer receive
DEVENV_TASKS_OUTPUTS. This allows them to run before their task's dependencies. They still receiveDEVENV_TASK_INPUT, shell env, and per-task env vars.Motivating Example
Before
$ devenv shell ✓ Configuring shell 32ms └ ✓ Evaluating shell cached 25ms ✓ Loading tasks 2ms └ ✓ Evaluating devenv.config.task.config cached 1ms ✓ Running tasks 10.1s └ ✓ devenv:enterShell 50ms └ ✓ devenv:files:cleanup 24ms └ ✓ demo:conditional-task skipped 34ms └ ✓ demo:expensive-task 1 lines → Expensive Done 10.0s └ ✓ devenv:enterTest skipped 0msAfter
devenv shell ✓ Configuring shell 42ms └ ✓ Evaluating shell cached 33ms ✓ Loading tasks 3ms └ ✓ Evaluating devenv.config.task.config cached 0ms ✓ Running tasks 74ms └ ✓ devenv:enterShell 21ms └ ✓ devenv:files:cleanup 20ms └ ✓ demo:conditional-task skipped 0ms └ ✓ demo:expensive-task skipped 0ms └ ✓ devenv:enterTest skipped 0ms