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.

Sign up for GitHub

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

[pallet-revive] fix fixture build path #6174

Merged
merged 9 commits into from
Oct 22, 2024
Merged

[pallet-revive] fix fixture build path #6174

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b730914
fix fixture build path
pgherveou Oct 22, 2024
3c2740c
restore compile_module
pgherveou Oct 22, 2024
696338e
fix
pgherveou Oct 22, 2024
786d958
Make symlink optional
pgherveou Oct 22, 2024
9cc69d9
check uapi dir exists
pgherveou Oct 22, 2024
9723574
Update from pgherveou running command 'prdoc --audience runtime_dev -…
actions-user Oct 22, 2024
b063c61
update
pgherveou Oct 22, 2024
d114818
Apply suggestions from code review
pgherveou Oct 22, 2024
b82cec8
Update prdoc/pr_6174.prdoc
pgherveou Oct 22, 2024
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
18 changes: 12 additions & 6 deletions substrate/frame/revive/fixtures/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,8 @@ mod build {
let fixtures_dir: PathBuf = env::var("CARGO_MANIFEST_DIR")?.into();
let contracts_dir = fixtures_dir.join("contracts");
let uapi_dir = fixtures_dir.parent().expect("uapi dir exits; qed").join("uapi");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also wrong because it expects the uapi crate being co-located with the fixtures dir? Which is not the case when building outside the mono repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right added a test to check if the dir exists

let ws_dir: PathBuf = env::var("CARGO_WORKSPACE_ROOT_DIR")?.into();
let out_dir: PathBuf = ws_dir.join("target").join("pallet-revive-fixtures");

// create out_dir if it does not exist
if !out_dir.exists() {
fs::create_dir_all(&out_dir)?;
}
let out_dir: PathBuf = env::var("OUT_DIR")?.into();

// the fixtures have a dependency on the uapi crate
println!("cargo::rerun-if-changed={}", fixtures_dir.display());
Expand All @@ -207,6 +202,17 @@ mod build {
invoke_build(tmp_dir_path)?;

write_output(tmp_dir_path, &out_dir, entries)?;

#[cfg(unix)]
{
let symlink_dir: PathBuf = env::var("CARGO_WORKSPACE_ROOT_DIR")?.into();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinkk this does only work on our CI? Shouldn't this be something like if let Ok(symlink_dir) = .. instead of the #[cfg(unix)]? Could probably also use a comment why this is needed, it's not obvious to me :)

let symlink_dir: PathBuf = symlink_dir.join("target").join("pallet-revive-fixtures");
if symlink_dir.is_symlink() {
fs::remove_file(&symlink_dir)?
}
std::os::unix::fs::symlink(&out_dir, &symlink_dir)?;
}

Ok(())
}
}
Expand Down
16 changes: 10 additions & 6 deletions substrate/frame/revive/fixtures/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,7 @@ pub mod bench {
#[cfg(feature = "riscv")]
macro_rules! fixture {
($name: literal) => {
include_bytes!(concat!(
env!("CARGO_WORKSPACE_ROOT_DIR"),
"/target/pallet-revive-fixtures/",
$name,
".polkavm"
))
include_bytes!(concat!(env!("OUT_DIR"), "/", $name, ".polkavm"))
};
}
#[cfg(not(feature = "riscv"))]
Expand All @@ -71,3 +66,12 @@ pub mod bench {
dummy
}
}

#[cfg(test)]
mod test {
#[test]
fn out_dir_should_have_compiled_mocks() {
let out_dir: std::path::PathBuf = env!("OUT_DIR").into();
assert!(out_dir.join("dummy.polkavm").exists());
}
}
Loading