Stream: git-wasmtime

Topic: wasmtime / PR #11418 Fix the bug of QEMU command in the c...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 12:46):

yomaytk edited PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 13:06):

yomaytk commented on PR #11418:

Thank you for the detailed analysis—I understand.
As you suggested, exporting CARGO_TARGET_*_RUNNER resolves the issue, so I’ve updated the code in that way.

but that's a bit of a bummer.

Do you mean that it’s not ideal for .cargo/config.toml to depend on CARGO_TARGET_*_RUNNER?
CARGO_TARGET_*_RUNNER looks like a variable that’s intended to be used only in CI.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 13:07):

yomaytk edited a comment on PR #11418:

Thank you for the detailed analysis—I understand.
As you suggested, exporting CARGO_TARGET_*_RUNNER resolves the issue, so I’ve updated the code in that way.

but that's a bit of a bummer.

Do you mean that it’s not ideal for .cargo/config.toml to depend on CARGO_TARGET_*_RUNNER?
CARGO_TARGET_*_RUNNER looks like a variable that’s intended to be used only in CI.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 16:16):

yomaytk edited a comment on PR #11418:

Thank you for the detailed analysis—I understand.
As you suggested, exporting CARGO_TARGET_*_RUNNER resolves the issue, so I’ve updated the code in that way. Could your review again?

but that's a bit of a bummer.

Do you mean that it’s not ideal for .cargo/config.toml to depend on CARGO_TARGET_*_RUNNER?
CARGO_TARGET_*_RUNNER looks like a variable that’s intended to be used only in CI.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 21:31):

alexcrichton commented on PR #11418:

Would you mind updating this to recommend setting the env vars directly instead of .cargo/config.toml? That way test-util/src/lib.rs wouldn't need modifications or need to encode system-specific paths like it's currently doing.

Ah by "bummer" I mean that requiring env vars is not fun since they're not shared by shells by default (unlike .cargo/config.toml). What you've got here is a bit of a quine-like situation where you want to specify a runner that specifies env vars to specify the runner which would need to specify env vars which ... (so on and so forth). What this really just wants is the env var to be inherited, or some way for the test itself to learn the runner configuration from Cargo's config, but only the former is possible.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 06:27):

yomaytk commented on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, the current cargo_test_runner is implemented to use the first runner in the iterator, so when we want to run tests against multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER, and set it each time we switch targets as follows.
    $ export CARGO_TARGET_RUNNER='qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1'.

  2. Specify each CARGO_*_RUNNER in .cargo/config.toml under the [env] section. Introduce CARGO_TARGET_ARCH, and fix cargo_test_runner to filter by the environment variable (as below). Then run tests like:
    $ CARGO_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

pub fn cargo_test_runner() -> Option<String> {
    // Note that this technically should look for the current target as well
    // instead of picking "any runner", but that's left for a future
    // refactoring.
    let (_, runner) = std::env::vars()
        .filter(|(k, _v)| {
            k.starts_with("CARGO_TARGET")
                && k.ends_with("RUNNER")
                && k.contains(
                    &std::env::var("CARGO_TARGET_ARCH")
                        .map(|target_arch| target_arch.to_uppercase())
                        .unwrap_or_else(|_| "".to_string()),
                )
        })
        .next()?;
    Some(runner)
}

Alternatively, there may be a better approach. I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 06:42):

yomaytk deleted a comment on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, the current cargo_test_runner is implemented to use the first runner in the iterator, so when we want to run tests against multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER, and set it each time we switch targets as follows.
    $ export CARGO_TARGET_RUNNER='qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1'.

  2. Specify each CARGO_*_RUNNER in .cargo/config.toml under the [env] section. Introduce CARGO_TARGET_ARCH, and fix cargo_test_runner to filter by the environment variable (as below). Then run tests like:
    $ CARGO_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

pub fn cargo_test_runner() -> Option<String> {
    // Note that this technically should look for the current target as well
    // instead of picking "any runner", but that's left for a future
    // refactoring.
    let (_, runner) = std::env::vars()
        .filter(|(k, _v)| {
            k.starts_with("CARGO_TARGET")
                && k.ends_with("RUNNER")
                && k.contains(
                    &std::env::var("CARGO_TARGET_ARCH")
                        .map(|target_arch| target_arch.to_uppercase())
                        .unwrap_or_else(|_| "".to_string()),
                )
        })
        .next()?;
    Some(runner)
}

Alternatively, there may be a better approach. I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 07:41):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 07:43):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 08:01):

yomaytk commented on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, since the current cargo_test_runner is implemented to use the first runner in the iterator, when we want to run tests for multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER and set it each time we switch targets.:
    $ export CARGO_TARGET_RUNNER=‘qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1’.

  2. Introduce CARGO_TARGET_ARCH representing the target, and fix cargo_test_runner to detect the appropriate runner based on this environment variable. Run tests like:
    $ CARGO_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

For reference, the latest changes in this PR implement the second approach.

There may be a better method; I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 08:06):

yomaytk edited a comment on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, since the current cargo_test_runner is implemented to use the first runner in the iterator, when we want to run tests for multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER and set it each time we switch targets.:
    $ export CARGO_TARGET_RUNNER=‘qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1’.

  2. Introduce CARGO_TARGET_ARCH representing the target, and fix cargo_test_runner to detect the appropriate runner based on this environment variable. Run tests like:
    $ CARGO_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

For reference, the latest changes in this PR implement the second approach.

There may be a better method; I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 08:54):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 08:54):

yomaytk edited a comment on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, since the current cargo_test_runner is implemented to use the first runner in the iterator, when we want to run tests for multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER and set it each time we switch targets.:
    $ export CARGO_TARGET_RUNNER=‘qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1’.

  2. Introduce CARGO_LOCAL_TARGET_ARCH representing the target, and fix cargo_test_runner to detect the appropriate runner based on this environment variable. Run tests like:
    $ CARGO_LOCAL_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

For reference, the latest changes in this PR implement the second approach.

There may be a better method; I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 11:07):

yomaytk deleted a comment on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, since the current cargo_test_runner is implemented to use the first runner in the iterator, when we want to run tests for multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering two approaches:

  1. Introduce an architecture-agnostic CARGO_TARGET_RUNNER and set it each time we switch targets.:
    $ export CARGO_TARGET_RUNNER=‘qemu-aarch64 -L /usr/aarch64-linux-gnu -E LD_LIBRARY_PATH=/usr/aarch64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1’.

  2. Introduce CARGO_LOCAL_TARGET_ARCH representing the target, and fix cargo_test_runner to detect the appropriate runner based on this environment variable. Run tests like:
    $ CARGO_LOCAL_TARGET_ARCH=aarch64 cargo test --target aarch64-unknown-linux-gnu.

For reference, the latest changes in this PR implement the second approach.

There may be a better method; I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 13:51):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 13:53):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 14:11):

yomaytk commented on PR #11418:

Thanks! Exporting CARGO_*_RUNNER is simple and requires no code changes. However, since the current cargo_test_runner is implemented to use the first runner in the iterator, when we want to run tests for multiple targets, we have to unset all non-target CARGO_*_RUNNER variables each time. Then, I’m considering the following approaches:

The latest changes in this PR implement this approach.

There may be a better method; I’d appreciate your thoughts.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:29):

alexcrichton commented on PR #11418:

Thanks for your work here, and I'm sorry if this is taking longer than expected. This is unfortunately a bit of a minefield where there's bugs that can't be fixed and I also have personal preferences about how we document and structure this. I also ideally want to have minimal impact on various bits of infrastructure since this is a relatively niche thing that shouldn't haev an outside impact.

This PR as-is adds a new required environment variable that's required to be specified to run tests in QEMU. Additionally it specifies configuration which is redundant in .cargo/config.toml where there's non-obvious overlap and surprising behavior.

If you're interested in seeing this PR all the way through, I'd recommend a possible change of removing the .cargo/config.toml configuration entirely and documenting only the CARGO_*_RUNNER env vars with an example of how to do one. Personally I think that would be sufficient for now and it's not super pressing to get the tests to a working state where you can set all the env vars for all the targets and run tests all at once for many targets.

If you'd like to get the multi-target approach working I'd recommend using cfg(target_arch = "...") instead of a new environment variable to figure out which one to filter for and look for.

Orthogonally to this PR, another option is to skip tests that use Command by default in cross-compiled situations. That way CI could opt-in to running these tests but others running cargo test could use the preexisting .cargo/config.toml configuration and it would work as expected.

Basically I want to avoid redundant configuration, hardcoding system-specifics in the repo, or adding undue burden on infrastructure for running these tests. It's unfortunately not easy to balance all these concerns though.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2025 at 05:56):

yomaytk commented on PR #11418:

Ah, I see—thank you for the detailed explanation. I understand that the proposed feature is niche and that it wouldn’t be good to place a burden on the infrastructure. Then, I’ll update this PR to document how to set CARGO_*_RUNNER.

documenting only the CARGO_*_RUNNER env vars

Just to confirm, do we also need to document CARGO_*_LINKER?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 15:26):

alexcrichton commented on PR #11418:

The linker bits don't need to be in env vars, but they do need to be specified somewhere. If the runner is in an env var I think it'd make sense to follow suit and use that for the linker too (no need to document two similar-and-related configuration systems here)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 17:20):

yomaytk updated PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 17:30):

yomaytk commented on PR #11418:

Got it. I’ve updated the PR—could you review it again?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 17:32):

alexcrichton submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 17:32):

alexcrichton has enabled auto merge for PR #11418.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 17:54):

alexcrichton merged PR #11418.

Last updated: Dec 06 2025 at 07:03 UTC