Merge pull request #681 from aya-rs/fix-cargo-test

test: document cargo build scripts

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",

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);
 

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);