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

Investigate using cargo as a library for test-kernels #784

Open
n0toose opened this issue Nov 6, 2024 · 3 comments
Open

Investigate using cargo as a library for test-kernels #784

n0toose opened this issue Nov 6, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@n0toose
Copy link
Member

n0toose commented Nov 6, 2024

At the moment, we directly invoke cargo using Command::new to build the test-kernels. A cleaner approach would be using cargo as a library instead.

See: https://docs.rs/cargo/latest/cargo/

uhyve/tests/common.rs

Lines 10 to 37 in 0cbbc3a

/// Uses Cargo to build a kernel in the `tests/test-kernels` directory.
/// Returns a path to the build binary.
pub fn build_hermit_bin(kernel: impl AsRef<Path>) -> PathBuf {
let kernel = kernel.as_ref();
println!("Building Kernel {}", kernel.display());
let kernel_src_path = Path::new("tests/test-kernels");
let cmd = Command::new("cargo")
.arg("build")
.arg("-Zbuild-std=std,panic_abort")
.arg("--target=x86_64-unknown-hermit")
.arg("--bin")
.arg(kernel)
// Remove environment variables related to the current cargo instance (toolchain version, coverage flags)
.env_clear()
// Retain PATH since it is used to find cargo and cc
.env("PATH", env::var_os("PATH").unwrap())
.current_dir(kernel_src_path)
.status()
.expect("failed to execute `cargo build`");
assert!(cmd.success(), "Test binaries could not be build");
[
kernel_src_path,
Path::new("target/x86_64-unknown-hermit/debug"),
Path::new(kernel),
]
.iter()
.collect()
}

@n0toose
Copy link
Member Author

n0toose commented Nov 6, 2024

Some comment on StackOverflow mentions that this is, indeed, still a Command::new under the hood. This may not necessarily have any short-term benefits for resolving #726.

@jounathaen
Copy link
Member

I'm not convinced, that cargo as library has real benefits. Please reopen if you disagree 🙂

@mkroening
Copy link
Member

mkroening commented Nov 12, 2024

I'd prefer using cargo as a library, actually.

The API being unstable for now is a blocker for me, though. We'd just generate unnecessary churn.

@jounathaen jounathaen reopened this Nov 12, 2024
@jounathaen jounathaen added help wanted Extra attention is needed good first issue Good for newcomers labels Nov 12, 2024
@mkroening mkroening removed help wanted Extra attention is needed good first issue Good for newcomers labels Nov 12, 2024
@n0toose n0toose added the enhancement New feature or request label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants