Skip to content

Commit

Permalink
fix(core): Fixes loading file on windows
Browse files Browse the repository at this point in the history
When we were loading the file, we were locking it and then opening another
file handle to read it. The test was loading it readonly rather than
using a shared lock, so this didn't catch the edge case

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
  • Loading branch information
thomastaylor312 committed Nov 4, 2024
1 parent 873e77a commit 82d40fd
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 19 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ resolver = "2"

[workspace.package]
edition = "2021"
version = "0.8.2"
version = "0.8.3"
authors = ["The Wasmtime Project Developers"]
license = "Apache-2.0 WITH LLVM-exception"

Expand Down Expand Up @@ -36,9 +36,9 @@ tracing-subscriber = { version = "0.3.18", default-features = false, features =
"fmt",
"env-filter",
] }
wasm-pkg-common = { version = "0.8.2", path = "crates/wasm-pkg-common" }
wasm-pkg-client = { version = "0.8.2", path = "crates/wasm-pkg-client" }
wasm-pkg-common = { version = "0.8.3", path = "crates/wasm-pkg-common" }
wasm-pkg-client = { version = "0.8.3", path = "crates/wasm-pkg-client" }
wasm-metadata = "0.219"
wit-component = "0.219"
wit-parser = "0.219"
wasm-pkg-core = { version = "0.8.2", path = "crates/wasm-pkg-core" }
wasm-pkg-core = { version = "0.8.3", path = "crates/wasm-pkg-core" }
26 changes: 17 additions & 9 deletions crates/wasm-pkg-core/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use semver::{Version, VersionReq};
use serde::{Deserialize, Serialize};
use tokio::{
fs::{File, OpenOptions},
io::{AsyncSeekExt, AsyncWriteExt},
io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt},
};
use wasm_pkg_client::{ContentDigest, PackageRef};

Expand Down Expand Up @@ -70,12 +70,14 @@ impl LockFile {
/// Loads a lock file from the given path. If readonly is set to false, then an exclusive lock
/// will be acquired on the file. This function will block until the lock is acquired.
pub async fn load_from_path(path: impl AsRef<Path>, readonly: bool) -> Result<Self> {
let locker = if readonly {
let mut locker = if readonly {
Locker::open_ro(path.as_ref()).await
} else {
Locker::open_rw(path.as_ref()).await
}?;
let contents = tokio::fs::read_to_string(path)
let mut contents = String::new();
locker
.read_to_string(&mut contents)
.await
.context("unable to load lock file from path")?;
let lock_file: LockFileIntermediate =
Expand All @@ -87,6 +89,13 @@ impl LockFile {
lock_file.version
));
}
// Rewind the file after reading just to be safe. We already do this before writing, but
// just in case we add any future logic, we can reset the file here so as to not cause
// issues
locker
.rewind()
.await
.context("Unable to reset file after reading")?;
Ok(lock_file.into_lock_file(locker))
}

Expand Down Expand Up @@ -131,25 +140,24 @@ impl LockFile {
pub async fn write(&mut self) -> Result<()> {
let contents = toml::to_string_pretty(self)?;
// Truncate the file before writing to it
self.locker.file.rewind().await.with_context(|| {
self.locker.rewind().await.with_context(|| {
format!(
"unable to rewind lock file at path {}",
self.locker.path.display()
)
})?;
self.locker.file.set_len(0).await.with_context(|| {
self.locker.set_len(0).await.with_context(|| {
format!(
"unable to truncate lock file at path {}",
self.locker.path.display()
)
})?;

self.locker.file.write_all(
self.locker.write_all(
b"# This file is automatically generated.\n# It is not intended for manual editing.\n",
)
.await.with_context(|| format!("unable to write lock file to path {}", self.locker.path.display()))?;
self.locker
.file
.write_all(contents.as_bytes())
.await
.with_context(|| {
Expand All @@ -160,7 +168,7 @@ impl LockFile {
})?;
// Make sure to flush and sync just to be sure the file doesn't drop and the lock is
// released too early
self.locker.file.sync_all().await.with_context(|| {
self.locker.sync_all().await.with_context(|| {
format!(
"unable to write lock file to path {}",
self.locker.path.display()
Expand Down Expand Up @@ -860,7 +868,7 @@ mod tests {

// Now read the lock file again and make sure everything is correct (and we can lock it
// properly)
let lock = LockFile::load_from_path(&path, true)
let lock = LockFile::load_from_path(&path, false)
.await
.expect("Shouldn't fail when loading lock file");
assert_eq!(
Expand Down

0 comments on commit 82d40fd

Please sign in to comment.