polkavm-test-data

[jarkkojs]

Closes: #209

[jarkkojs]

I don't know how to fix build-and-test-windows.

[athei]

The windows runner doesn't seem to use a rustup managed cargo. So they are not able to download and use the toolchain specified in the rustup-toolchain.toml. So you might have to make sure that rustup is used on windows.

[koute]

Why the env_clear?

[jarkkojs]

Not sure. I need refresh on Monday. It's a strong possibility that it is for no good reason ;-)

[jarkkojs]

The history for this is that I derived the code from substrate/frame/revive/fixtures/build.rs.

I guess the argument here (and fixtures) is that by doing this we remove any side-effects of environment variables set by parent i.e. always starting from the same state.

[jarkkojs]

I recall @athei did the code I used as basis so maybe he has a comment for this...

[athei]

Yeah it makes sense since we don't want other variables meant for other targets to influence this build.

[jarkkojs]

See my remarks below.

[koute]

I don't know how to fix build-and-test-windows.

This is weird; it's clearly using rustup because cargo test --all triggers a toolchain install:

>> cargo test (debug)
info: syncing channel updates for '1.75.0-x86_64-pc-windows-msvc'
info: latest update on 2023-12-28, rust version 1.75.0 (82e1608df 2023-12-21)

but when it's ran from inside of the build.rs it ignores the guest-programs/rust-toolchain.toml for some reason?

I'm not sure how to fix it; you'll have to fiddle with it. Maybe clearing the environment variables screws it up?

[athei]

This is weird; it's clearly using rustup because cargo test --all triggers a toolchain install:

Oops didn't see that and jumped to conclusions. Rustup seems to installed and is picking up the toolchain file in the repo root.

but when it's ran from inside of the build.rs it ignores the guest-programs/rust-toolchain.toml for some reason?

Yes it is weird. Maybe some platform differences regarding pathes?

I'm not sure how to fix it; you'll have to fiddle with it. Maybe clearing the environment variables screws it up?

We do the clearing in the other build scripts in polkadot-sdk, too. But I don't think it is ever tested on Windows. Yeah maybe removing the clearing and work from there. Maybe additional or different variables need to be forwarded on Windows.

[athei]

At least now it is failing everywhere? :D

[jarkkojs]

At least now it is failing everywhere? :D

Working on it... temporary glitch ;---)

[jarkkojs]

@koute, @athei, I'll go a few diff's now that I've fixed the stuff I know how to fix!

Diff 1:

diff --git a/crates/polkavm-test-data/build.rs b/crates/polkavm-test-data/build.rs
index 2def01d..4971115 100644
--- a/crates/polkavm-test-data/build.rs
+++ b/crates/polkavm-test-data/build.rs
@@ -21,7 +21,6 @@ fn build(project: &str, profile: &str, target: &str) -> Result<(), String> {
     let mut cmd = Command::new("cargo");
 
     cmd.current_dir(project_path)
-        .env_clear()
         .env("PATH", std::env!("PATH"))
         .env("RUSTFLAGS", rust_flags)
         .env("RUSTUP_HOME", std::env!("RUSTUP_HOME"))

Result:

~/work/github.com/paritytech/polkavm/crates/polkavm-test-data polkavm-test-data*
$ cargo b    
warning: /home/jarkko/work/github.com/paritytech/polkavm/Cargo.toml: unused manifest key: workspace.lints.rust.unexpected_cfgs.check-cfg
   Compiling polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)
error: failed to run custom build command for `polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)`

Caused by:
  process didn't exit successfully: `/home/jarkko/work/github.com/paritytech/polkavm/target/debug/build/polkavm-test-data-0ea5ae9020752356/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=OUT_DIR

  --- stderr
  Error: "error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel\nSee https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.\n"

Diff 2:

diff --git a/crates/polkavm-test-data/build.rs b/crates/polkavm-test-data/build.rs
index 2def01d..b637903 100644
--- a/crates/polkavm-test-data/build.rs
+++ b/crates/polkavm-test-data/build.rs
@@ -18,10 +18,9 @@ fn build(project: &str, profile: &str, target: &str) -> Result<(), String> {
 
     // Note that using `std::env!("CARGO")` fails the compilation since the
     // compilation requires nightly.
-    let mut cmd = Command::new("cargo");
+    let mut cmd = Command::new(std::env!("CARGO"));
 
     cmd.current_dir(project_path)
-        .env_clear()
         .env("PATH", std::env!("PATH"))
         .env("RUSTFLAGS", rust_flags)
         .env("RUSTUP_HOME", std::env!("RUSTUP_HOME"))

Result:

~/work/github.com/paritytech/polkavm/crates/polkavm-test-data polkavm-test-data* 24s
$ cargo b    
warning: /home/jarkko/work/github.com/paritytech/polkavm/Cargo.toml: unused manifest key: workspace.lints.rust.unexpected_cfgs.check-cfg
   Compiling polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)
error: failed to run custom build command for `polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)`

Caused by:
  process didn't exit successfully: `/home/jarkko/work/github.com/paritytech/polkavm/target/debug/build/polkavm-test-data-0ea5ae9020752356/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=OUT_DIR

  --- stderr
  Error: "error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel\nSee https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.\n"

Diff 3:

diff --git a/crates/polkavm-test-data/build.rs b/crates/polkavm-test-data/build.rs
index 2def01d..a901351 100644
--- a/crates/polkavm-test-data/build.rs
+++ b/crates/polkavm-test-data/build.rs
@@ -18,7 +18,7 @@ fn build(project: &str, profile: &str, target: &str) -> Result<(), String> {
 
     // Note that using `std::env!("CARGO")` fails the compilation since the
     // compilation requires nightly.
-    let mut cmd = Command::new("cargo");
+    let mut cmd = Command::new(std::env!("CARGO"));
 
     cmd.current_dir(project_path)
         .env_clear()

Result:

~/work/github.com/paritytech/polkavm/crates/polkavm-test-data polkavm-test-data* 19s
$ cargo b    
warning: /home/jarkko/work/github.com/paritytech/polkavm/Cargo.toml: unused manifest key: workspace.lints.rust.unexpected_cfgs.check-cfg
   Compiling polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)
error: failed to run custom build command for `polkavm-test-data v0.18.0 (/home/jarkko/work/github.com/paritytech/polkavm/crates/polkavm-test-data)`

Caused by:
  process didn't exit successfully: `/home/jarkko/work/github.com/paritytech/polkavm/target/debug/build/polkavm-test-data-0ea5ae9020752356/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=OUT_DIR

  --- stderr
  Error: "error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel\nSee https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.\n"

How should I move forward?

[athei]

How should I move forward?

Debug this further. Probably reproducing in a Windows VM. Looking at the code nothing obvious comes to mind for me.

[jarkkojs]

How should I move forward?

Debug this further. Probably reproducing in a Windows VM. Looking at the code nothing obvious comes to mind for me.

I'm working on setting up a Windows VM. It is worth of investment as Windows could break also in future because POSIX does not always align that well in that environment.

So yeah, we're on the same page.

[jarkkojs]

I spent a lot of time on setting up a Windows environment that is usable. Now I have an image that logins automatically and boots to the terminal, and I can reach the host via ssh. Also environment has been setup.

This what happens:

@athei, @koute: So I guess this maps to replacing std::env!("HOME") with std::env::home_dir() doesn't it?

[jarkkojs]

I think it was useful spend couple of days to figure out effective way to use for Windows. After this I can reproduce bugs in no time. It was an investment ;-)

[jarkkojs]

I spent a lot of time on setting up a Windows environment that is usable. Now I have an image that logins automatically and boots to the terminal, and I can reach the host via ssh. Also environment has been setup.

This what happens:

In the previous, I failed to test correctly: I should have run ci/jobs/build-and-test.sh in a bash prompt, not cargo t in a PowerShell prompt.

This what I get now:

>> cargo test (debug)
warning: C:\Users\jarkko\work\github.com\paritytech\polkavm\Cargo.toml: unused manifest key: workspace.lints.rust.unexpected_cfgs.check-cfg
warning: unused imports: `ISA64_V1_NoSbrk`, `build_static_dispatch_table`
 --> crates\polkavm\src\api.rs:9:5
  |
9 |     build_static_dispatch_table, FrameKind, ISA32_V1_NoSbrk, ISA64_V1_NoSbrk, Imports, InstructionSet, Instructions,...
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^                              ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused variable: `exports`
   --> crates\polkavm\src\api.rs:410:13
    |
410 |         let exports = {
    |             ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_exports`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: method `context` is never used
  --> crates\polkavm\src\error.rs:74:19
   |
62 | impl Error {
   | ---------- method in this implementation
...
74 |     pub(crate) fn context(self, message: impl core::fmt::Display) -> Self {
   |                   ^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: field `engine_state` is never read
   --> crates\polkavm\src\api.rs:229:5
    |
228 | pub(crate) struct ModulePrivate {
    |                   ------------- field in this struct
229 |     engine_state: Option<Arc<EngineState>>,
    |     ^^^^^^^^^^^^

warning: methods `compiled_module` and `address_to_page` are never used
   --> crates\polkavm\src\api.rs:285:19
    |
263 | impl Module {
    | ----------- methods in this implementation
...
285 |     pub(crate) fn compiled_module(&self) -> &CompiledModuleKind {
    |                   ^^^^^^^^^^^^^^^
...
343 |     pub(crate) fn address_to_page(&self, address: u32) -> u32 {
    |                   ^^^^^^^^^^^^^^^

warning: variants `Disjoint`, `None`, `One`, and `Two` are never constructed
  --> crates\polkavm\src\page_set.rs:21:5
   |
20 | enum SubResult {
   |      --------- variants in this enum
21 |     Disjoint,
   |     ^^^^^^^^
22 |     None,
   |     ^^^^
23 |     One(Interval),
   |     ^^^
24 |     Two(Interval, Interval),
   |     ^^^
   |
   = note: `SubResult` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis

warning: associated items `merge` and `subtract` are never used
  --> crates\polkavm\src\page_set.rs:28:8
   |
27 | impl Interval {
   | ------------- associated items in this implementation
28 |     fn merge(self, rhs: Interval) -> Option<Interval> {
   |        ^^^^^
...
41 |     fn subtract(existing: Interval, to_remove: Interval) -> SubResult {
   |        ^^^^^^^^

warning: associated items `new`, `insert`, `contains`, `is_whole_region_empty`, `remove`, and `clear` are never used
   --> crates\polkavm\src\page_set.rs:147:12
    |
146 | impl PageSet {
    | ------------ associated items in this implementation
147 |     pub fn new() -> Self {
    |            ^^^
...
153 |     pub fn insert(&mut self, (insert_min, insert_max): (u32, u32)) {
    |            ^^^^^^
...
188 |     pub fn contains(&self, (min, max): (u32, u32)) -> bool {
    |            ^^^^^^^^
...
203 |     pub fn is_whole_region_empty(&self, (min, max): (u32, u32)) -> bool {
    |            ^^^^^^^^^^^^^^^^^^^^^
...
218 |     pub fn remove(&mut self, (removed_min, removed_max): (u32, u32)) {
    |            ^^^^^^
...
272 |     pub fn clear(&mut self) {
    |            ^^^^^

warning: associated items `new_reusing_memory`, `len`, and `clear` are never used
  --> crates\polkavm\src\utils.rs:63:12
   |
50 | / impl<T> FlatMap<T>
51 | | where
52 | |     T: Copy,
   | |____________- associated items in this implementation
...
63 |       pub fn new_reusing_memory(mut memory: Self, capacity: u32) -> Self {
   |              ^^^^^^^^^^^^^^^^^^
...
75 |       pub fn len(&self) -> u32 {
   |              ^^^
...
85 |       pub fn clear(&mut self) {
   |              ^^^^^

warning: method `lock` is never used
  --> crates\polkavm\src\mutex_std.rs:12:12
   |
5  | impl<T> Mutex<T> {
   | ---------------- method in this implementation
...
12 |     pub fn lock(&self) -> std::sync::MutexGuard<T> {
   |            ^^^^

warning: `polkavm` (lib) generated 10 warnings (run `cargo fix --lib -p polkavm` to apply 2 suggestions)
   Compiling polkavm-test-data v0.18.0 (C:\Users\jarkko\work\github.com\paritytech\polkavm\crates\polkavm-test-data)
   Compiling fnv v1.0.7
error: failed to run custom build command for `polkavm-test-data v0.18.0 (C:\Users\jarkko\work\github.com\paritytech\polkavm\crates\polkavm-test-data)`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_TEST_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.

Caused by:
  process didn't exit successfully: `C:\Users\jarkko\work\github.com\paritytech\polkavm\target\debug\build\polkavm-test-data-6cd164aff147f2c5\build-script-build` (exit code: 1)
  --- stdout
  cargo:rerun-if-env-changed=OUT_DIR

  --- stderr
  Error: "error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel\nSee https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.\n"
warning: build failed, waiting for other jobs to finish...

This looks like the same issue as the one that pops up CI, doesn't it?

@athei, @koute: I give up now for this week but check if you spot anything interesting (and put a note).