Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit test for verifying that caching works when running packages twice #3512

Merged
merged 23 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
037dcd3
Implement failing test case to debug issues with wasmer run caching
fschutt Jan 20, 2023
6be874f
Fix CI unit test
fschutt Jan 20, 2023
f201f5c
nit: GIT_HASH_SHORT should show 7 chars, not 5
fschutt Jan 20, 2023
436cd79
Fix comments
fschutt Jan 20, 2023
e627ec1
Debug test_run_customlambda not working
fschutt Jan 20, 2023
99b0cc7
Debug test_run_customlambda not working
fschutt Jan 20, 2023
958da9d
Fix merge errors
fschutt Jan 20, 2023
3ea4ef2
Fix make lint
fschutt Jan 20, 2023
ebdf681
Fix CI tests
fschutt Jan 20, 2023
b4600c5
Debug error of try_unpack_targz
fschutt Jan 20, 2023
5e53391
Merge branch 'master' into fix-caching-issue-customlambda
fschutt Jan 20, 2023
ceb6541
Add more debugging to .gz / .xz untar process
fschutt Jan 20, 2023
1dc903d
Debug tar gz unpacking
fschutt Jan 20, 2023
ce575e2
Debug .tar.gz unpacking
fschutt Jan 20, 2023
28b901d
Fix cargo fmt
fschutt Jan 20, 2023
ca4b976
Normalize file paths when unpacking .tar.gz on Windows
fschutt Jan 23, 2023
c073cfe
Add more debug info
fschutt Jan 23, 2023
7b4ea13
Fix issue with ar.unpack()
fschutt Jan 23, 2023
5779a7f
Fix unpack_with_parent function
fschutt Jan 23, 2023
0b08bf3
Disable test_run_customlambda on Windows, add tracking issue comment
fschutt Jan 26, 2023
e3db893
Merge branch 'master' into fix-caching-issue-customlambda
fschutt Jan 26, 2023
580b643
Merge branch 'master' into fix-caching-issue-customlambda
fschutt Jan 26, 2023
add6663
Merge branch 'master' into fix-caching-issue-customlambda
fschutt Jan 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ pub fn main() {
if git_hash.len() > 5 {
println!(
"cargo:rustc-env=WASMER_BUILD_GIT_HASH_SHORT={}",
&git_hash[..5]
&git_hash[..7]
);
} else {
println!("cargo:rustc-env=WASMER_BUILD_GIT_HASH_SHORT=??????");
println!("cargo:rustc-env=WASMER_BUILD_GIT_HASH_SHORT=???????");
}

let utc: DateTime<Utc> = Utc::now();
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,5 @@ pub fn get_cache_dir() -> PathBuf {
}

pub(crate) fn normalize_path(s: &str) -> String {
s.strip_prefix(r"\\?\").unwrap_or(s).to_string()
wasmer_registry::utils::normalize_path(s)
}
81 changes: 67 additions & 14 deletions lib/registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::fmt;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::time::Duration;
use tar::EntryType;
use url::Url;

pub mod config;
Expand All @@ -26,6 +27,7 @@ pub mod publish;
pub mod queries;
pub mod utils;

use crate::utils::normalize_path;
pub use crate::{
config::{format_graphql, WasmerConfig},
package::Package,
Expand Down Expand Up @@ -415,8 +417,14 @@ pub fn try_unpack_targz<P: AsRef<Path>>(
target_path: P,
strip_toplevel: bool,
) -> Result<PathBuf, anyhow::Error> {
let target_targz_path = target_targz_path.as_ref();
let target_path = target_path.as_ref();
let target_targz_path = target_targz_path.as_ref().to_string_lossy().to_string();
let target_targz_path = crate::utils::normalize_path(&target_targz_path);
let target_targz_path = Path::new(&target_targz_path);

let target_path = target_path.as_ref().to_string_lossy().to_string();
let target_path = crate::utils::normalize_path(&target_path);
let target_path = Path::new(&target_path);

let open_file = || {
std::fs::File::open(&target_targz_path)
.map_err(|e| anyhow::anyhow!("failed to open {}: {e}", target_targz_path.display()))
Expand All @@ -425,14 +433,20 @@ pub fn try_unpack_targz<P: AsRef<Path>>(
let try_decode_gz = || {
let file = open_file()?;
let gz_decoded = flate2::read::GzDecoder::new(&file);
let mut ar = tar::Archive::new(gz_decoded);
let ar = tar::Archive::new(gz_decoded);
if strip_toplevel {
unpack_sans_parent(ar, target_path).map_err(|e| {
anyhow::anyhow!("failed to unpack {}: {e}", target_targz_path.display())
anyhow::anyhow!(
"failed to unpack (gz_ar_unpack_sans_parent) {}: {e}",
target_targz_path.display()
)
})
} else {
ar.unpack(target_path).map_err(|e| {
anyhow::anyhow!("failed to unpack {}: {e}", target_targz_path.display())
unpack_with_parent(ar, target_path).map_err(|e| {
anyhow::anyhow!(
"failed to unpack (gz_ar_unpack) {}: {e}",
target_targz_path.display()
)
})
}
};
Expand All @@ -442,25 +456,37 @@ pub fn try_unpack_targz<P: AsRef<Path>>(
let mut decomp: Vec<u8> = Vec::new();
let mut bufread = std::io::BufReader::new(&file);
lzma_rs::xz_decompress(&mut bufread, &mut decomp).map_err(|e| {
anyhow::anyhow!("failed to unpack {}: {e}", target_targz_path.display())
anyhow::anyhow!(
"failed to unpack (try_decode_xz) {}: {e}",
target_targz_path.display()
)
})?;

let cursor = std::io::Cursor::new(decomp);
let mut ar = tar::Archive::new(cursor);
if strip_toplevel {
unpack_sans_parent(ar, target_path).map_err(|e| {
anyhow::anyhow!("failed to unpack {}: {e}", target_targz_path.display())
anyhow::anyhow!(
"failed to unpack (sans parent) {}: {e}",
target_targz_path.display()
)
})
} else {
ar.unpack(target_path).map_err(|e| {
anyhow::anyhow!("failed to unpack {}: {e}", target_targz_path.display())
ar.unpack(&target_path).map_err(|e| {
anyhow::anyhow!(
"failed to unpack (with parent) {}: {e}",
target_targz_path.display()
)
})
}
};

try_decode_gz().or_else(|_| try_decode_xz())?;
try_decode_gz().or_else(|e1| {
try_decode_xz()
.map_err(|e2| anyhow::anyhow!("could not decode gz: {e1}, could not decode xz: {e2}"))
})?;

Ok(target_targz_path.to_path_buf())
Ok(Path::new(&target_targz_path).to_path_buf())
}

/// Whether the top-level directory should be stripped
Expand Down Expand Up @@ -494,12 +520,39 @@ pub fn download_and_unpack_targz(
Ok(target_path.to_path_buf())
}

pub fn unpack_with_parent<R>(mut archive: tar::Archive<R>, dst: &Path) -> Result<(), anyhow::Error>
where
R: std::io::Read,
{
use std::path::Component::Normal;

let dst_normalized = normalize_path(dst.to_string_lossy().as_ref());

for entry in archive.entries()? {
let mut entry = entry?;
let path: PathBuf = entry
.path()?
.components()
.skip(1) // strip top-level directory
.filter(|c| matches!(c, Normal(_))) // prevent traversal attacks
.collect();
if entry.header().entry_type().is_file() {
entry.unpack_in(&dst_normalized)?;
} else if entry.header().entry_type() == EntryType::Directory {
std::fs::create_dir_all(&Path::new(&dst_normalized).join(&path))?;
}
}
Ok(())
}

pub fn unpack_sans_parent<R>(mut archive: tar::Archive<R>, dst: &Path) -> std::io::Result<()>
where
R: std::io::Read,
{
use std::path::Component::Normal;

let dst_normalized = normalize_path(dst.to_string_lossy().as_ref());

for entry in archive.entries()? {
let mut entry = entry?;
let path: PathBuf = entry
Expand All @@ -508,7 +561,7 @@ where
.skip(1) // strip top-level directory
.filter(|c| matches!(c, Normal(_))) // prevent traversal attacks
.collect();
entry.unpack(dst.join(path))?;
entry.unpack(Path::new(&dst_normalized).join(path))?;
}
Ok(())
}
Expand Down Expand Up @@ -538,7 +591,7 @@ pub fn install_package(wasmer_dir: &Path, url: &Url) -> Result<PathBuf, anyhow::
unpacked_targz_path.as_path(),
false,
)
.with_context(|| anyhow::anyhow!("Could not unpack file downloaded from {url}"))?;
.map_err(|e| anyhow::anyhow!("Could not unpack file downloaded from {url}: {e}"))?;

// read {unpacked}/wasmer.toml to get the name + version number
let toml_parsed = LocalPackage::read_toml(&unpacked_targz_path)
Expand Down
4 changes: 4 additions & 0 deletions lib/registry/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ pub fn get_username_registry_token(registry: &str, token: &str) -> anyhow::Resul
let response: who_am_i_query::ResponseData = execute_query(registry, token, &q)?;
Ok(response.viewer.map(|viewer| viewer.username))
}

pub fn normalize_path(s: &str) -> String {
s.strip_prefix(r"\\?\").unwrap_or(s).to_string()
}
69 changes: 68 additions & 1 deletion tests/integration/cli/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,70 @@ fn test_no_start_wat_path() -> PathBuf {
Path::new(ASSET_PATH).join("no_start.wat")
}

/// Ignored on Windows because running vendored packages does not work
/// since Windows does not allow `::` characters in filenames (every other OS does)
///
/// The syntax for vendored package atoms has to be reworked for this to be fixed, see
/// https://github.com/wasmerio/wasmer/issues/3535
#[cfg_attr(target_os = "windows", ignore)]
#[test]
fn test_run_customlambda() -> anyhow::Result<()> {
let bindir = String::from_utf8(
Command::new(get_wasmer_path())
.arg("config")
.arg("--bindir")
.output()
.expect("failed to run wasmer config --bindir")
.stdout,
)
.expect("wasmer config --bindir stdout failed");

// /Users/fs/.wasmer/bin
let checkouts_path = Path::new(&bindir)
.parent()
.expect("--bindir: no parent")
.join("checkouts");
println!("checkouts path: {}", checkouts_path.display());
let _ = std::fs::remove_dir_all(&checkouts_path);

let output = Command::new(get_wasmer_path())
.arg("run")
.arg("https://wapm.io/ciuser/customlambda")
// TODO: this argument should not be necessary later
// see https://github.com/wasmerio/wasmer/issues/3514
.arg("customlambda.py")
.arg("55")
.output()?;

let stdout_output = std::str::from_utf8(&output.stdout).unwrap();
let stderr_output = std::str::from_utf8(&output.stderr).unwrap();

println!("first run:");
println!("stdout: {stdout_output}");
println!("stderr: {stderr_output}");
assert_eq!(stdout_output, "139583862445\n");

// Run again to verify the caching
let output = Command::new(get_wasmer_path())
.arg("run")
.arg("https://wapm.io/ciuser/customlambda")
// TODO: this argument should not be necessary later
// see https://github.com/wasmerio/wasmer/issues/3514
.arg("customlambda.py")
.arg("55")
.output()?;

let stdout_output = std::str::from_utf8(&output.stdout).unwrap();
let stderr_output = std::str::from_utf8(&output.stderr).unwrap();

println!("second run:");
println!("stdout: {stdout_output}");
println!("stderr: {stderr_output}");
assert_eq!(stdout_output, "139583862445\n");

Ok(())
}

fn assert_tarball_is_present_local(target: &str) -> Result<PathBuf, anyhow::Error> {
let wasmer_dir = std::env::var("WASMER_DIR").expect("no WASMER_DIR set");
let directory = match target {
Expand Down Expand Up @@ -583,7 +647,10 @@ fn run_test_caching_works_for_packages() -> anyhow::Result<()> {
.output()?;

if output.stdout != b"hello\n".to_vec() {
panic!("failed to run https://wapm.io/python/python for the first time");
panic!("failed to run https://wapm.io/python/python for the first time: stdout = {}, stderr = {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
);
}

let time = std::time::Instant::now();
Expand Down