Commit 4cc7091
committed
fix: ensure
Overwriting `task.dependencies` with the graph edges doesn't play nicely
with `get_filtered_taskgraph` (e.g `--tasks <regex>`) because in this
case the full graph has already been trimmed. The result is that when
using `--tasks`, the dependencies displayed will be a subset of the real
dependencies. This is very misleading and can easily lead one to false
conclusions while debugging. Recently this caused me to waste an hour of
my time.
I traced this comment back through history and it's been like this since
it was originally implemented. Likely the intent was that if we a have a
subset of the graph, then we should modify the task definition to not
display any dependencies that aren't part of it.
But I disagree with this. I'd argue that `Task.dependencies` is a very
important part of the task definition, and not at all relevant to the
higher level DAG that the task happens to be a part of. I believe
mutating the task definition just because we did some filtering is a
bug.
Testing the full graph from the Firefox repo, I can confirm this change
results in identical graphs (when not using `--tasks`).taskgraph full -J --tasks <regex> shows all dependencies1 parent 5c65170 commit 4cc7091
3 files changed
Lines changed: 29 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
48 | 47 | | |
49 | 48 | | |
50 | 49 | | |
51 | 50 | | |
52 | | - | |
53 | | - | |
54 | 51 | | |
55 | 52 | | |
56 | 53 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
165 | 165 | | |
166 | 166 | | |
167 | 167 | | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
168 | 188 | | |
169 | 189 | | |
170 | 190 | | |
| |||
226 | 246 | | |
227 | 247 | | |
228 | 248 | | |
229 | | - | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
230 | 256 | | |
231 | 257 | | |
232 | 258 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
30 | | - | |
31 | | - | |
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
| |||
56 | 55 | | |
57 | 56 | | |
58 | 57 | | |
59 | | - | |
| 58 | + | |
60 | 59 | | |
61 | 60 | | |
62 | 61 | | |
| |||
0 commit comments