Add CRC mode to make CRC checks opt-in by default#12706
Add CRC mode to make CRC checks opt-in by default#12706samypr100 wants to merge 2 commits intoastral-sh:mainfrom
Conversation
|
LGTM. |
|
We should probably make this a global setting so it shows up in settings, etc. But I know that's more work, can be a TODO. |
| Enforce, | ||
| /// Warn on CRC mismatch, but continue. | ||
| Lax, |
There was a problem hiding this comment.
I'm curious why not "Error" and "Warn" here which feel more canonical to me?
There was a problem hiding this comment.
(which would also be "Ignore" instead of "None", I think)
There was a problem hiding this comment.
I wonder if we'd then want a different name? UV_CRC_CHECK = error | warn | ignore? I feel less strongly about that.
There was a problem hiding this comment.
Happy to do the change once I'm back to personal laptop
|
I'm gonna put up a PR for an alternative approach. |
|
I'm going to close this in favour of #12722, but if that fix proves insufficient this is a more robust hammer we should revisit. |
|
Thanks for hoping on this anyway Sam. |
4 participants
Summary
This is a POC quick fix as proposed in #12618 (comment)
This adds a
UV_CRC_MODEenv var that allowsenforceorlaxto be set or default tononewhich does nothing.Avoided reworking out the interfaces as that led to a larger change across uv-extract and uv-metadata, but longer term that's probably a better solution.
Looking at the dependency graph, uv-distribution-filename was sort-off the best place for CRC Mode, but I'd be happy to move it.Test Plan
Updated existing crc test.