Merge pull request #677 from aya-rs/ebpf-dep-better

rebuild ebpf on bpf-linker

.github/workflows/lint.yml

@@ -38,4 +38,11 @@ jobs:
         run: cargo hack clippy --all-targets --feature-powerset --workspace -- --deny warnings
 
       - name: Run miri
-        run: cargo miri test --all-targets
+        run: |
+          cargo hack miri test --all-targets --feature-powerset \
+            --exclude aya-bpf \
+            --exclude aya-bpf-bindings \
+            --exclude aya-log-ebpf \
+            --exclude integration-ebpf \
+            --exclude integration-test \
+            --workspace

Cargo.toml

@@ -29,7 +29,7 @@ default-members = [
     "aya-log-parser",
     "aya-obj",
     "aya-tool",
-    # test/integration-test is omitted; it must be built with xtask.
+    "test/integration-test",
     "xtask",
 
     "aya-bpf-macros",

bpf/.cargo/config.toml

@@ -6,7 +6,7 @@
 # ignored if you run from the workspace root. See
 # https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure
 [build]
-target = "bpfel-unknown-none"
+target = ["bpfeb-unknown-none", "bpfel-unknown-none"]
 
 [unstable]
 build-std = ["core"]

test/integration-ebpf/.cargo/config.toml

@@ -0,0 +1,12 @@
+# We have this so that one doesn't need to manually pass
+# --target=bpfel-unknown-none -Z build-std=core when running cargo
+# check/build/doc etc.
+#
+# NB: this file gets loaded only if you run cargo from this directory, it's
+# ignored if you run from the workspace root. See
+# https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure
+[build]
+target = ["bpfeb-unknown-none", "bpfel-unknown-none"]
+
+[unstable]
+build-std = ["core"]

test/integration-ebpf/Cargo.toml

@@ -8,6 +8,9 @@ publish = false
 aya-bpf = { path = "../../bpf/aya-bpf" }
 aya-log-ebpf = { path = "../../bpf/aya-log-ebpf" }
 
+[build-dependencies]
+xtask = { path = "../../xtask" }
+
 [[bin]]
 name = "log"
 path = "src/log.rs"

test/integration-ebpf/build.rs

@@ -0,0 +1,23 @@
+use std::{env, path::PathBuf};
+
+use xtask::{create_symlink_to_binary, AYA_BUILD_INTEGRATION_BPF};
+
+fn main() {
+    println!("cargo:rerun-if-env-changed={}", AYA_BUILD_INTEGRATION_BPF);
+
+    let build_integration_bpf = env::var(AYA_BUILD_INTEGRATION_BPF)
+        .as_deref()
+        .map(str::parse)
+        .map(Result::unwrap)
+        .unwrap_or_default();
+
+    if build_integration_bpf {
+        let out_dir = env::var_os("OUT_DIR").unwrap();
+        let out_dir = PathBuf::from(out_dir);
+        let bpf_linker_symlink = create_symlink_to_binary(&out_dir, "bpf-linker").unwrap();
+        println!(
+            "cargo:rerun-if-changed={}",
+            bpf_linker_symlink.to_str().unwrap()
+        );
+    }
+}

test/integration-ebpf/src/lib.rs

@@ -0,0 +1,3 @@
+#![no_std]
+
+// This file exists to enable the library target.

test/integration-test/Cargo.toml

@@ -25,5 +25,17 @@ tokio = { version = "1.24", default-features = false, features = [
 
 [build-dependencies]
 cargo_metadata = { version = "0.15.4", default-features = false }
-which = { version = "4.4.0", default-features = false }
+# TODO(https://github.com/rust-lang/cargo/issues/12375): this should be an artifact dependency, but
+# it's not possible to tell cargo to use `-Z build-std` to build it. We cargo-in-cargo in the build
+# script to build this, but we want to teach cargo about the dependecy so that cache invalidation
+# works properly.
+#
+# Note also that https://github.com/rust-lang/cargo/issues/10593 occurs when `target = ...` is added
+# to an artifact dependency; it seems possible to work around that by setting `resolver = "1"` in
+# Cargo.toml in the workspace root.
+#
+# Finally note that *any* usage of `artifact = ...` in *any* Cargo.toml in the workspace breaks
+# workflows with stable cargo; stable cargo outright refuses to load manifests that use unstable
+# features.
+integration-ebpf = { path = "../integration-ebpf" }
 xtask = { path = "../../xtask" }

test/integration-test/build.rs

@@ -1,5 +1,4 @@
 use std::{
-    collections::{HashMap, HashSet},
     env,
     ffi::OsString,
     fmt::Write as _,
@@ -10,34 +9,24 @@ use std::{
 };
 
 use cargo_metadata::{
-    Artifact, CompilerMessage, Dependency, Message, Metadata, MetadataCommand, Package, Target,
+    Artifact, CompilerMessage, Message, Metadata, MetadataCommand, Package, Target,
 };
-use which::which;
-use xtask::{exec, LIBBPF_DIR};
+use xtask::{exec, AYA_BUILD_INTEGRATION_BPF, LIBBPF_DIR};
 
 fn main() {
-    const AYA_BUILD_INTEGRATION_BPF: &str = "AYA_BUILD_INTEGRATION_BPF";
-
     println!("cargo:rerun-if-env-changed={}", AYA_BUILD_INTEGRATION_BPF);
 
-    let build_integration_bpf = match env::var_os(AYA_BUILD_INTEGRATION_BPF) {
-        None => false,
-        Some(s) => {
-            let s = s.to_str().unwrap();
-            s.parse::<bool>().unwrap()
-        }
-    };
-
-    const INTEGRATION_EBPF_PACKAGE: &str = "integration-ebpf";
+    let build_integration_bpf = env::var(AYA_BUILD_INTEGRATION_BPF)
+        .as_deref()
+        .map(str::parse)
+        .map(Result::unwrap)
+        .unwrap_or_default();
 
     let Metadata { packages, .. } = MetadataCommand::new().no_deps().exec().unwrap();
-    let packages: HashMap<String, _> = packages
+    let integration_ebpf_package = packages
         .into_iter()
-        .map(|package| {
-            let Package { name, .. } = &package;
-            (name.clone(), package)
-        })
-        .collect();
+        .find(|Package { name, .. }| name == "integration-ebpf")
+        .unwrap();
 
     let manifest_dir = env::var_os("CARGO_MANIFEST_DIR").unwrap();
     let manifest_dir = PathBuf::from(manifest_dir);
@@ -121,44 +110,8 @@ fn main() {
 
         let target = format!("{target}-unknown-none");
 
-        // Teach cargo about our dependencies.
-        let mut visited = HashSet::new();
-        let mut frontier = vec![INTEGRATION_EBPF_PACKAGE];
-        while let Some(package) = frontier.pop() {
-            if !visited.insert(package) {
-                continue;
-            }
-            let Package { dependencies, .. } = packages.get(package).unwrap();
-            for Dependency { name, path, .. } in dependencies {
-                if let Some(path) = path {
-                    println!("cargo:rerun-if-changed={}", path.as_str());
-                    frontier.push(name);
-                }
-            }
-        }
-
-        // Create a symlink in the out directory to work around the fact that cargo ignores anything
-        // in `$CARGO_HOME`, which is also where `cargo install` likes to place binaries. Cargo will
-        // stat through the symlink and discover that bpf-linker has changed.
-        //
-        // This was introduced in https://github.com/rust-lang/cargo/commit/99f841c.
-        {
-            let bpf_linker = which("bpf-linker").unwrap();
-            let bpf_linker_symlink = out_dir.join("bpf-linker");
-            match fs::remove_file(&bpf_linker_symlink) {
-                Ok(()) => {}
-                Err(err) => {
-                    if err.kind() != std::io::ErrorKind::NotFound {
-                        panic!("failed to remove symlink: {err}")
-                    }
-                }
-            }
-            std::os::unix::fs::symlink(&bpf_linker, &bpf_linker_symlink).unwrap();
-            println!(
-                "cargo:rerun-if-changed={}",
-                bpf_linker_symlink.to_str().unwrap()
-            );
-        }
+        let Package { manifest_path, .. } = integration_ebpf_package;
+        let integration_ebpf_dir = manifest_path.parent().unwrap();
 
         let mut cmd = Command::new("cargo");
         cmd.args([
@@ -172,8 +125,6 @@ fn main() {
         ]);
 
         // Workaround to make sure that the rust-toolchain.toml is respected.
-        let Package { manifest_path, .. } = packages.get(INTEGRATION_EBPF_PACKAGE).unwrap();
-        let integration_ebpf_dir = manifest_path.parent().unwrap();
         cmd.env_remove("RUSTUP_TOOLCHAIN")
             .current_dir(integration_ebpf_dir);
 
@@ -231,7 +182,7 @@ fn main() {
             fs::write(&dst, []).unwrap_or_else(|err| panic!("failed to create {dst:?}: {err}"));
         }
 
-        let Package { targets, .. } = packages.get(INTEGRATION_EBPF_PACKAGE).unwrap();
+        let Package { targets, .. } = integration_ebpf_package;
         for Target { name, kind, .. } in targets {
             if *kind != ["bin"] {
                 continue;

xtask/Cargo.toml

@@ -14,3 +14,4 @@ proc-macro2 = "1"
 quote = "1"
 syn = "2"
 tempfile = "3"
+which = { version = "4.4.0", default-features = false }

xtask/src/lib.rs

@@ -1,5 +1,13 @@
 use anyhow::{anyhow, Context as _, Result};
-use std::process::Command;
+use std::{
+    fs,
+    path::{Path, PathBuf},
+    process::Command,
+};
+use which::which;
+
+pub const AYA_BUILD_INTEGRATION_BPF: &str = "AYA_BUILD_INTEGRATION_BPF";
+pub const LIBBPF_DIR: &str = "xtask/libbpf";
 
 pub fn exec(cmd: &mut Command) -> Result<()> {
     let status = cmd
@@ -14,4 +22,25 @@ pub fn exec(cmd: &mut Command) -> Result<()> {
     }
 }
 
-pub const LIBBPF_DIR: &str = "xtask/libbpf";
+// Create a symlink in the out directory to work around the fact that cargo ignores anything
+// in `$CARGO_HOME`, which is also where `cargo install` likes to place binaries. Cargo will
+// stat through the symlink and discover that the binary has changed.
+//
+// This was introduced in https://github.com/rust-lang/cargo/commit/99f841c.
+//
+// TODO(https://github.com/rust-lang/cargo/pull/12369): Remove this when the fix is available.
+pub fn create_symlink_to_binary(out_dir: &Path, binary_name: &str) -> Result<PathBuf> {
+    let binary = which(binary_name).unwrap();
+    let symlink = out_dir.join(binary_name);
+    match fs::remove_file(&symlink) {
+        Ok(()) => {}
+        Err(err) => {
+            if err.kind() != std::io::ErrorKind::NotFound {
+                return Err(err).context(format!("failed to remove symlink {}", symlink.display()));
+            }
+        }
+    }
+    std::os::unix::fs::symlink(binary, &symlink)
+        .with_context(|| format!("failed to create symlink {}", symlink.display()))?;
+    Ok(symlink)
+}

xtask/src/run.rs

@@ -8,6 +8,7 @@ use std::{
 use anyhow::{Context as _, Result};
 use cargo_metadata::{Artifact, CompilerMessage, Message, Target};
 use clap::Parser;
+use xtask::AYA_BUILD_INTEGRATION_BPF;
 
 #[derive(Debug, Parser)]
 pub struct BuildOptions {
@@ -35,7 +36,7 @@ pub struct Options {
 pub fn build(opts: BuildOptions) -> Result<Vec<(String, PathBuf)>> {
     let BuildOptions { release, target } = opts;
     let mut cmd = Command::new("cargo");
-    cmd.env("AYA_BUILD_INTEGRATION_BPF", "true").args([
+    cmd.env(AYA_BUILD_INTEGRATION_BPF, "true").args([
         "build",
         "--tests",
         "--message-format=json",