Fixes issue #34: set tmpfile length in async writer #35
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
The async `poll_write()` implementation was creating a tempfile as a backing for its inner mmap, but it was failing to set the length on the file to match the incoming data. Compare with the sync implementation! This bug was exposed when the `memmap2` crate was swapped in for `memmap`. The older crate was likely more lax about this. Wrote a pair of new tests for `cacache::write_hash_sync` and `cacache::write_hash`. The async test fails without this change, as does any benchmarks run. Everything passes with it.
zkat
reviewed
Jan 24, 2023
- Removed extraneous commented-out dep from testing. - Updated Rust edition to 2021, as CI demanded. - Ran `cargo clippy --fix` to remove all the new lints.
|
https://github.com/zkat/cacache-rs/actions/runs/3991969995/jobs/6847382624#step:4:16 and it seems like it might be time to bump the MSRV |
|
I think the issue is more that edition2021 was still nightly-only in 1.54? |
|
Grump, did I misread that? I didn't touch the edition field at all until after that. Looking more closely... |
The first 2021 edition was version 1.56. I am guessing that the intent was to test on the version that was stable at the time of latest version release.
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.
The async
poll_write()implementation was creating a tempfile as a backing for its inner mmap, but it was failing to set the length on the file to match the incoming data. Compare with the sync implementation!This bug was exposed when the
memmap2crate was swapped in formemmap. The older crate was likely more lax about this.Wrote a pair of new tests for
cacache::write_hash_syncandcacache::write_hash. The async test fails without this change, as does any benchmarks run. Everything passes with it.