[Cache] Add integrity check using checksums. #10064
Conversation
Relates to CocoaPods#10053, but adds built in integrity checking instead of adding a separate command. This is a first version intended to show the effect. Will probably have to do some refactoring, to make sure that we have a lock while performing integrity checks.
Here's an example of your CHANGELOG entry: * [Cache] Add integrity check using checksums.
[codiophile](https://github.com/codiophile)
[#issue_number](https://github.com/CocoaPods/CocoaPods/issues/issue_number)note: There are two invisible spaces after the entry's text. Generated by 🚫 Danger |
| @@ -156,6 +157,34 @@ def self.valid_lock?(file, filename) | |||
|
|
|||
| private | |||
|
|
|||
| def checksum_for_directory(directory) | |||
There was a problem hiding this comment.
how "expensive" (slow) can this get? should we provide a cache keyed by directory?
There was a problem hiding this comment.
how "expensive" (slow) can this get?
Well, it didn't have any noticeable impact on test execution time, so it seems fairly cheap. I would assume that the main bottleneck for MD5 is the I/O and not CPU. Considering that we are already doing other I/O operations (like downloading and copying) in connection with this, I think it is fair to assume that the files being checked are already cached in RAM. I suppose, in theory, you could have a problem with pods that are too big to fit in RAM, but pods are generally small and RAM is generally plentiful, so in practice, that should never be a problem.
should we provide a cache keyed by
directory?
I'm afraid I don't understand the question. :)
|
@codiophile can you rebase? we actually converted to GH actions on latest master. |
Hi @dnkoutso, I just saw this. I've been swamped with work lately. I will have a look when I find some time. |
|
@codiophile friendly ping if you'd like to rebase this one just to ensure everything still passes. |
|
@igor-makarov does this look good to you? |
| md5.hexdigest | ||
| end | ||
|
|
||
| def validate_cache(path) |
There was a problem hiding this comment.
this seems to be returning a bool, perhaps rename to cache_valid? instead?
| @@ -281,12 +310,14 @@ def in_tmpdir(&blk) | |||
| # | |||
| def copy_and_clean(source, destination, spec) | |||
| specs_by_platform = group_subspecs_by_platform(spec) | |||
| Pod::Installer::PodSourcePreparer.new(spec, source).prepare! | |||
There was a problem hiding this comment.
what is the reason to move those out of the lock file block?
Relates to #10053, but adds built in integrity checking instead of adding a separate command.
This is a first version intended to show the effect. Will probably have to do some refactoring, to make sure that we have a lock while performing integrity checks.