From 8cec4b25f9091cb369201a4b4423623cf1eb019c Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 20 Jul 2023 10:18:44 -0400 Subject: [PATCH 1/2] Revert "integration-test: add to default-members" This reverts commit 025c76780c0755fa02f9b4b6a9506453b9f56f6b. Fixes #678. --- Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 958be754..e3ed0b7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,8 @@ default-members = [ "aya-log-parser", "aya-obj", "aya-tool", - "test/integration-test", + # test/integration-test is omitted; including it in this list causes `cargo test` to run its + # tests, and that doesn't work unless they've been built with `cargo xtask`. "xtask", "aya-bpf-macros", From a84bd1a95dab2f0092d0ce2d67f508477199c0bc Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 20 Jul 2023 10:51:34 -0400 Subject: [PATCH 2/2] test: document cargo build scripts --- test/integration-ebpf/build.rs | 12 ++++++++++++ test/integration-test/build.rs | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/test/integration-ebpf/build.rs b/test/integration-ebpf/build.rs index 35cb3582..dd4a6501 100644 --- a/test/integration-ebpf/build.rs +++ b/test/integration-ebpf/build.rs @@ -2,6 +2,18 @@ use std::{env, path::PathBuf}; use xtask::{create_symlink_to_binary, AYA_BUILD_INTEGRATION_BPF}; +/// Building this crate has an undeclared dependency on the `bpf-linker` binary. This would be +/// better expressed by [artifact-dependencies][bindeps] but issues such as +/// https://github.com/rust-lang/cargo/issues/12385 make their use impractical for the time being. +/// +/// This file implements an imperfect solution: it causes cargo to rebuild the crate whenever the +/// mtime of `which bpf-linker` changes. Note that possibility that a new bpf-linker is added to +/// $PATH ahead of the one used as the cache key still exists. Solving this in the general case +/// would require rebuild-if-changed-env=PATH *and* rebuild-if-changed={every-directory-in-PATH} +/// which would likely mean far too much cache invalidation. +/// +/// [bindeps]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html?highlight=feature#artifact-dependencies + fn main() { println!("cargo:rerun-if-env-changed={}", AYA_BUILD_INTEGRATION_BPF); diff --git a/test/integration-test/build.rs b/test/integration-test/build.rs index b1d9e7db..2b95f068 100644 --- a/test/integration-test/build.rs +++ b/test/integration-test/build.rs @@ -13,6 +13,29 @@ use cargo_metadata::{ }; use xtask::{exec, AYA_BUILD_INTEGRATION_BPF, LIBBPF_DIR}; +/// This crate has a runtime dependency on artifacts produced by the `integration-ebpf` crate. This +/// would be better expressed as one or more [artifact-dependencies][bindeps] but issues such as: +/// * https://github.com/rust-lang/cargo/issues/12374 +/// * https://github.com/rust-lang/cargo/issues/12375 +/// * https://github.com/rust-lang/cargo/issues/12385 +/// prevent their use for the time being. +/// +/// This file, along with the xtask crate, allows analysis tools such as `cargo check`, `cargo +/// clippy`, and even `cargo build` to work as users expect. Prior to this file's existence, this +/// crate's undeclared dependency on artifacts from `integration-ebpf` would cause build (and `cargo check`, +/// and `cargo clippy`) failures until the user ran certain other commands in the workspace. Conversely, +/// those same tools (e.g. cargo test --no-run) would produce stale results if run naively because +/// they'd make use of artifacts from a previous build of `integration-ebpf`. +/// +/// Note that this solution is imperfect: in particular it has to balance correctness with +/// performance; an environment variable is used to replace true builds of `integration-ebpf` with +/// stubs to preserve the property that code generation and linking (in `integration-ebpf`) do not +/// occur on metadata-only actions such as `cargo check` or `cargo clippy` of this crate. This means +/// that naively attempting to `cargo test --no-run` this crate will produce binaries that fail at +/// runtime because the stubs are inadequate for actually running the tests. +/// +/// [bindeps]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html?highlight=feature#artifact-dependencies + fn main() { println!("cargo:rerun-if-env-changed={}", AYA_BUILD_INTEGRATION_BPF);