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

Always build eBPF; remove xtask #132

Merged
merged 1 commit into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .cargo/config.toml

This file was deleted.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[workspace]
resolver = "2"
members = ["xtask", "{{project-name}}", "{{project-name}}-common", "{{project-name}}-ebpf"]
default-members = ["xtask", "{{project-name}}", "{{project-name}}-common"]
members = ["{{project-name}}", "{{project-name}}-common", "{{project-name}}-ebpf"]
default-members = ["{{project-name}}", "{{project-name}}-common"]

[workspace.dependencies]
aya = { version = "0.13.0", default-features = false }
Expand Down
11 changes: 7 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,21 @@

## Build & Run

Use `cargo build`, `cargo check`, etc. as normal. Run your program with `xtask run`.
Use `cargo build`, `cargo check`, etc. as normal. Run your program with:

```shell
cargo run --release --config 'target."cfg(all())".runner="sudo -E"'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we set this in config.toml? It's what I do for my projects

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that as a default. It should be opt in to invoke sudo.

Copy link
Member

@vadorovsky vadorovsky Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From one hand I find this command quite annoying - the syntax looks really weird.

From the other hand, I do agree that calling sudo should be opt in. Maybe someone wants to use doas or something else.

Only if cargo supported something env var like CARGO_RUNNER, so users could just do export CARGO_RUNNER="sudo -E", which looks less wild.

At the end I think I have a slight preference towards not defaulting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are still concerns with this I'll address them in a follow-up.

```

Cargo build scripts are used to automatically build the eBPF correctly and include it in the
program. When not using `xtask run`, eBPF code generation is skipped for a faster developer
experience; this compromise necessitates the use of `xtask` to actually build the eBPF.
program.

## Cross-compiling on macOS

Cross compilation should work on both Intel and Apple Silicon Macs.

```shell
AYA_BUILD_EBPF=true CC=${ARCH}-linux-musl-gcc cargo build --package {{project-name}} --release \
CC=${ARCH}-linux-musl-gcc cargo build --package {{project-name}} --release \
--target=${ARCH}-unknown-linux-musl \
--config=target.${ARCH}-unknown-linux-musl.linker=\"${ARCH}-linux-musl-gcc\"
```
Expand Down
9 changes: 5 additions & 4 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ case $OS in
if [[ "$ARCH" == "arm64" ]]; then
ARCH="aarch64"
fi
AYA_BUILD_EBPF=true CC=${ARCH}-linux-musl-gcc cargo build --package "${CRATE_NAME}" --release \
--target="${ARCH}"-unknown-linux-musl \
--config=target."${ARCH}"-unknown-linux-musl.linker=\""${ARCH}"-linux-musl-gcc\"
TARGET=${ARCH}-unknown-linux-musl
CC=${ARCH}-linux-musl-gcc cargo build --package "${CRATE_NAME}" --release \
--target="${TARGET}" \
--config=target."${TARGET}".linker=\""${ARCH}"-linux-musl-gcc\"
;;
"Linux")
cargo +nightly fmt --all -- --check
Expand All @@ -85,7 +86,7 @@ case $OS in

expect <<EOF
set timeout 30 ;# Increase timeout if necessary
spawn cargo xtask run
spawn cargo run --release --config "target.\"cfg(all())\".runner=\"sudo -E\""
expect {
-re "Waiting for Ctrl-C.*" {
send -- \003 ;# Send Ctrl-C
Expand Down
8 changes: 0 additions & 8 deletions xtask/Cargo.toml

This file was deleted.

1 change: 0 additions & 1 deletion xtask/src/lib.rs

This file was deleted.

23 changes: 0 additions & 23 deletions xtask/src/main.rs

This file was deleted.

47 changes: 0 additions & 47 deletions xtask/src/run.rs

This file was deleted.

1 change: 0 additions & 1 deletion {{project-name}}-ebpf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ aya-log-ebpf = { workspace = true }

[build-dependencies]
which = { workspace = true }
xtask = { path = "../xtask" }

[[bin]]
name = "{{ project-name }}"
Expand Down
17 changes: 2 additions & 15 deletions {{project-name}}-ebpf/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::env;

use which::which;
use xtask::AYA_BUILD_EBPF;

/// 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
Expand All @@ -15,16 +12,6 @@ use xtask::AYA_BUILD_EBPF;
///
/// [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_EBPF);

let build_ebpf = env::var(AYA_BUILD_EBPF)
.as_deref()
.map(str::parse)
.map(Result::unwrap)
.unwrap_or_default();

if build_ebpf {
let bpf_linker = which("bpf-linker").unwrap();
println!("cargo:rerun-if-changed={}", bpf_linker.to_str().unwrap());
}
let bpf_linker = which("bpf-linker").unwrap();
println!("cargo:rerun-if-changed={}", bpf_linker.to_str().unwrap());
}
1 change: 1 addition & 0 deletions {{project-name}}-ebpf/rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[toolchain]
channel = "nightly"
components = ["rust-src"]
1 change: 0 additions & 1 deletion {{project-name}}/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ cargo_metadata = { workspace = true }
# workflows with stable cargo; stable cargo outright refuses to load manifests that use unstable
# features.
{{project-name}}-ebpf = { path = "../{{project-name}}-ebpf" }
xtask = { path = "../xtask" }

[[bin]]
name = "{{project-name}}"
Expand Down
30 changes: 4 additions & 26 deletions {{project-name}}/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use std::{
use cargo_metadata::{
Artifact, CompilerMessage, Message, Metadata, MetadataCommand, Package, Target,
};
use xtask::AYA_BUILD_EBPF;

/// This crate has a runtime dependency on artifacts produced by the `{{project-name}}-ebpf` crate.
/// This would be better expressed as one or more [artifact-dependencies][bindeps] but issues such
Expand All @@ -20,32 +19,8 @@ use xtask::AYA_BUILD_EBPF;
///
/// 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 `{{project-name}}-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
/// `{{project-name}}-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 `{{project-name}}-ebpf`
/// with stubs to preserve the property that code generation and linking (in
/// `{{project-name}}-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_EBPF);

let build_ebpf = env::var(AYA_BUILD_EBPF)
.as_deref()
.map(str::parse)
.map(Result::unwrap)
.unwrap_or_default();

let Metadata { packages, .. } = MetadataCommand::new().no_deps().exec().unwrap();
let ebpf_package = packages
.into_iter()
Expand All @@ -64,6 +39,9 @@ fn main() {
panic!("unsupported endian={:?}", endian)
};

// TODO(https://github.com/rust-lang/cargo/issues/4001): Make this `false` if we can determine
// we're in a check build.
let build_ebpf = true;
if build_ebpf {
let arch = env::var_os("CARGO_CFG_TARGET_ARCH").unwrap();

Expand Down Expand Up @@ -94,7 +72,7 @@ fn main() {
cmd.env("CARGO_CFG_BPF_TARGET_ARCH", arch);

// Workaround to make sure that the rust-toolchain.toml is respected.
for key in ["RUSTUP_TOOLCHAIN", "RUSTC"] {
for key in ["RUSTUP_TOOLCHAIN", "RUSTC", "RUSTC_WORKSPACE_WRAPPER"] {
cmd.env_remove(key);
}
cmd.current_dir(ebpf_dir);
Expand Down