From 76bf99149a14443c005323951ad8ce803bd1e5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20M=C3=BCller?= Date: Wed, 13 Nov 2024 11:09:21 -0800 Subject: [PATCH] libbpf-cargo: Clean up crate-structure mess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It goes against every Cargo project preconception to have certain modules be compiled from both lib.rs and main.rs and it leads to shenanigans such as #[allow(dead_code)] tags scattered all over the place, which is its own can of worms when done at such a high level. Do it properly and export necessary functionality from the library, for consumption by the binary. To prevent any misconceptions of there being a stable API contract for functionality *only* being used by the binary, put it into an hidden module. Signed-off-by: Daniel Müller --- libbpf-cargo/src/build.rs | 3 +-- libbpf-cargo/src/gen/btf.rs | 2 +- libbpf-cargo/src/gen/mod.rs | 10 ++++------ libbpf-cargo/src/lib.rs | 23 +++++++++++++++++------ libbpf-cargo/src/main.rs | 9 ++++----- libbpf-cargo/src/metadata.rs | 7 +++++-- libbpf-cargo/src/test.rs | 4 ++-- 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/libbpf-cargo/src/build.rs b/libbpf-cargo/src/build.rs index 9e2442ed..1503336e 100644 --- a/libbpf-cargo/src/build.rs +++ b/libbpf-cargo/src/build.rs @@ -279,7 +279,7 @@ fn extract_clang_or_default(clang: Option<&PathBuf>) -> PathBuf { } #[allow(clippy::too_many_arguments)] -pub(crate) fn build( +pub fn build( debug: bool, manifest_path: Option<&PathBuf>, clang: Option<&PathBuf>, @@ -309,7 +309,6 @@ pub(crate) fn build( } // Only used in libbpf-cargo library -#[allow(dead_code)] pub(crate) fn build_single( debug: bool, source: &Path, diff --git a/libbpf-cargo/src/gen/btf.rs b/libbpf-cargo/src/gen/btf.rs index 94fe2620..9cd131eb 100644 --- a/libbpf-cargo/src/gen/btf.rs +++ b/libbpf-cargo/src/gen/btf.rs @@ -449,7 +449,7 @@ impl StructOps {{ } -pub struct GenBtf<'s> { +pub(crate) struct GenBtf<'s> { btf: Btf<'s>, anon_types: AnonTypes, } diff --git a/libbpf-cargo/src/gen/mod.rs b/libbpf-cargo/src/gen/mod.rs index 4a6003be..92f7b213 100644 --- a/libbpf-cargo/src/gen/mod.rs +++ b/libbpf-cargo/src/gen/mod.rs @@ -1,4 +1,4 @@ -pub mod btf; +mod btf; use std::borrow::Cow; use std::collections::BTreeMap; @@ -43,8 +43,8 @@ use memmap2::Mmap; use crate::metadata; use crate::metadata::UnprocessedObj; -use self::btf::GenBtf; -use self::btf::GenStructOps; +pub(crate) use self::btf::GenBtf; +pub(crate) use self::btf::GenStructOps; /// Name of the `.kconfig` map. @@ -195,9 +195,7 @@ pub(crate) enum OutputDest<'a> { Stdout, /// Infer a filename and place file in specified directory Directory(&'a Path), - #[allow(dead_code)] /// File to place output in - // Only constructed in libbpf-cargo library File(&'a Path), } @@ -1352,7 +1350,7 @@ fn gen_project( Ok(()) } -pub(crate) fn gen( +pub fn gen( debug: bool, manifest_path: Option<&PathBuf>, rustfmt_path: Option<&PathBuf>, diff --git a/libbpf-cargo/src/lib.rs b/libbpf-cargo/src/lib.rs index d2cabb9b..35f6c7af 100644 --- a/libbpf-cargo/src/lib.rs +++ b/libbpf-cargo/src/lib.rs @@ -77,15 +77,9 @@ use anyhow::Result; use tempfile::tempdir; use tempfile::TempDir; -// libbpf-cargo binary is the primary consumer of the following modules. As such, -// we do not use all the symbols. Silence any unused code warnings. -#[allow(dead_code)] mod build; -#[allow(dead_code)] mod gen; -#[allow(dead_code)] mod make; -#[allow(dead_code)] mod metadata; #[cfg(test)] @@ -285,3 +279,20 @@ impl SkeletonBuilder { Ok(()) } } + + +/// Implementation details shared with the binary. +/// +/// NOT PART OF PUBLIC API SURFACE! +#[doc(hidden)] +pub mod __private { + pub mod build { + pub use crate::build::build; + } + pub mod gen { + pub use crate::gen::gen; + } + pub mod make { + pub use crate::make::make; + } +} diff --git a/libbpf-cargo/src/main.rs b/libbpf-cargo/src/main.rs index 893dead8..7a2ed2a9 100644 --- a/libbpf-cargo/src/main.rs +++ b/libbpf-cargo/src/main.rs @@ -9,11 +9,10 @@ use clap::Args; use clap::Parser; use clap::Subcommand; -#[doc(hidden)] -mod build; -mod gen; -mod make; -mod metadata; +use libbpf_cargo::__private::build; +use libbpf_cargo::__private::gen; +use libbpf_cargo::__private::make; + #[doc(hidden)] #[derive(Debug, Parser)] diff --git a/libbpf-cargo/src/metadata.rs b/libbpf-cargo/src/metadata.rs index 1c1e4ae2..27e1b52d 100644 --- a/libbpf-cargo/src/metadata.rs +++ b/libbpf-cargo/src/metadata.rs @@ -24,7 +24,7 @@ struct PackageMetadata { } #[derive(Debug, Clone)] -pub struct UnprocessedObj { +pub(crate) struct UnprocessedObj { /// Package the object belongs to pub package: String, /// Path to .c @@ -142,7 +142,10 @@ fn get_package( } /// Returns the `target_directory` and a list of objects to compile. -pub fn get(debug: bool, manifest_path: Option<&PathBuf>) -> Result<(PathBuf, Vec)> { +pub(crate) fn get( + debug: bool, + manifest_path: Option<&PathBuf>, +) -> Result<(PathBuf, Vec)> { let mut cmd = MetadataCommand::new(); if let Some(path) = manifest_path { diff --git a/libbpf-cargo/src/test.rs b/libbpf-cargo/src/test.rs index 640ecea3..1ae14183 100644 --- a/libbpf-cargo/src/test.rs +++ b/libbpf-cargo/src/test.rs @@ -20,8 +20,8 @@ use tempfile::NamedTempFile; use tempfile::TempDir; use crate::build::build; -use crate::gen::btf::GenBtf; -use crate::gen::btf::GenStructOps; +use crate::gen::GenBtf; +use crate::gen::GenStructOps; use crate::make::make; use crate::SkeletonBuilder;