Don't cache failed generators #737
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When FuseSoC runs generators, the output is cached if the generator re- quests so. While this is generally a good idea, the cached output must not be used in subsequent runs if the generator fails with a non-zero exit code. In that case, the generator should be re-invoked in the next run to not need to user to manually run `fusesoc gen clean` to fix a faulty cache entry. This commit adds a test for that exact behavior, which shows, that the expectation outlined above is not upheld. Instead a failed generator run is still cached and thus never re-executed until its input changes.
If the generator invocation failed for any reason, its output directory is removed. While this is bad for debugging failing generators, it at least prevents the next FuseSoC-run to assume, that the generator was successful and hence a failed generator is retried on the next FuseSoC run. Since the stderr of the executed command is printed to the user, the harm for debugging is not that large, so this should be fine. As an alternative, one could store a file `.success` (or similar) in the generator directory and only use the cache directory if that file is present there.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When FuseSoC runs generators, the output is cached if the generator requests so. While this is generally a good idea, the cached output must not be used in subsequent runs if the generator fails with a non-zero exit code. In that case, the generator should be re-invoked in the next run to not need to user to manually run
fusesoc gen cleanto fix a faulty cache entry. Therefore this pull request adds in a check, if the generator call was successful and if it isn't the caching is prevented.The first commit adds a test for a failing generator called in two subsequent FuseSoC-runs, expecting it to be called twice. The second commit changes the code calling generators to check for errors, taking action and re-raising that error. Currently the aforementioned action is to delete the entire working directory of the generator, since this will fail the current check for a cached generator result:
fusesoc/fusesoc/edalizer.py
Line 657 in 1403437
As an alternative, one could store a file
.success(or similar) in the generator directory and only use the cache directory if that file is present there. I'm happy to implement this instead, if deemed better.