Use persisted build information to automatically clean old outputs #39654
Conversation
Codecov Report
@@ Coverage Diff @@
## master #39654 +/- ##
==========================================
+ Coverage 55.61% 58.34% +2.73%
==========================================
Files 194 194
Lines 18726 18742 +16
==========================================
+ Hits 10414 10935 +521
+ Misses 8312 7807 -505
Continue to review full report at Codecov.
|
| } | ||
| } | ||
| return canSkip; | ||
| return _ChangeResult(previousOutputs, canSkip); |
There was a problem hiding this comment.
Consider caching the decoded stamp file json, and adding getters to the Target whose values are cached after being computed the first time. So, in this case, instead of returning _ChangeResult(...), it would go back to returning a bool, but Target would have a separate getter previousOutputs that is computed once using the decoded json that may or may not have already been cached itself.
There was a problem hiding this comment.
I think that to be ergonomic this needs some refactoring. The Target should not be used as the runtime representation of the build graph, that should be delegated to another interface which is mutable, allowing my to cache these values. That would potentially simplify the requirements for Source vs files too - Target could allow configuring sources, while the runtime representation could use Files.
There was a problem hiding this comment.
Updated this to have a runtime component which caches the outputs/inputs/stamp and deals only with files.
As a consequence, I'll need to deprecate/remove all the functionality that allowed for completely dynamic outputs (like output wildcards), but the tradeoff is worth it - I can now detect that a rule has changed only when the outputs change.
This ended up blocking the build assets with flutter assemble PR, since changing the output directory would not in of itself trigger a rebuild. After this PR it does.
| // Malformed stamp file, not safe to skip. | ||
| _dirty = true; | ||
| } else { | ||
| final Map<String, Object> values = json.decode(content); |
There was a problem hiding this comment.
If the json is non-empty but corrupted somehow, then anything from here down could throw one of several exceptions. Maybe catch and set _dirty to true there too.
There was a problem hiding this comment.
Updated this to be safe, added test, unnested
Description
We already have the information about the prior dag cached. When computing updates, uses the existing map of current output files to remove any old files that are no longer a part of the output.
There might be an edge case where a file is removed from the output of one target and added to the output of another, but I think that will be handled by comparing the filehashes (though I need more validation here anyway).
Removes manual deletion for asset and macOS rule.
Tests