libbpf-cargo: Remove clang version check logic

Remove the logic for checking the clang version in use. This logic is
worrisome and insufficient for a couple of reasons.
It is insufficient, because we don't actually know what clang version we
require -- that is a moving target. Nobody is actively testing against
the lowest supported clang version to make sure that our stuff actually
works with it. At this point, the stated minimum of 10.0.0 is also
increasingly unlikely to be found in the wild, rendering the check less
and less useful with every passing year. Ultimately, if a necessary
feature really is not available, one would hope that the emitted error
isn't too cryptic for anybody to make sense of it.
On the "worrisome" front, the over reliance on anything and everything
clang (not just with this check, but with naming more generally) boxes
us into a single-compiler world. It is entirely conceivable that a GCC
BPF backend arrives eventually (has it already?), which, if past is any
guidance, shoulder understand roughly the same arguments as clang
itself and, hence, potentially being a drop-in replacement. We should be
ready to support this future without shenanigans such as "gcc" being
passed to an API asking for "clang". Furthermore, parsing the compiler's
output using regular expressions is at best a fragile endeavor.
What's more, the amount of dependencies (direct + transitive) pulled in
just in service of this one feature is penalizing everybody, for no
obvious value-add.
Lastly, the plumping necessary for this feature is part of the reason we
have nonsensical call sites such as the following to deal with:
     make(
       true,
       Some(&cargo_toml),
       None,
       Vec::new(),
       true,
       true,
       Vec::new(),
       None,
     )

In short, removal of this checking logic is our best option. Take it.

Signed-off-by: Daniel Müller <deso@posteo.net>

libbpf-cargo/CHANGELOG.md

@@ -1,3 +1,10 @@
+Unreleased
+----------
+- Removed `SkeletonBuilder::skip_clang_version_check`
+- Removed `--skip-clang-version-checks` option of `libbpf build`
+  sub-command
+
+
 0.25.0-beta.1
 -------------
 - Fixed skeleton generation when `enum64` types are present

libbpf-cargo/Cargo.toml

@@ -33,8 +33,6 @@ anyhow = "1.0.1"
 cargo_metadata = "0.15.0"
 libbpf-rs = { version = "=0.25.0-beta.1", default-features = false, path = "../libbpf-rs" }
 memmap2 = "0.5"
-regex = { version = "1.6.0", default-features = false, features = ["std", "unicode-perl"] }
-semver = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 tempfile = "3.3"

libbpf-cargo/src/build.rs

@@ -12,8 +12,6 @@ use anyhow::anyhow;
 use anyhow::bail;
 use anyhow::Context;
 use anyhow::Result;
-use regex::Regex;
-use semver::Version;
 use tempfile::tempdir;
 
 use crate::metadata;
@@ -51,18 +49,6 @@ fn check_progs(objs: &[UnprocessedObj]) -> Result<()> {
     Ok(())
 }
 
-fn extract_version(output: &str) -> Result<&str> {
-    let re = Regex::new(r"clang\s+version\s+(?P<version_str>\d+\.\d+\.\d+)")?;
-    let captures = re
-        .captures(output)
-        .ok_or_else(|| anyhow!("Failed to run regex on version string"))?;
-
-    captures.name("version_str").map_or_else(
-        || Err(anyhow!("Failed to find version capture group")),
-        |v| Ok(v.as_str()),
-    )
-}
-
 /// Extract vendored libbpf header files to a temporary directory.
 ///
 /// Directory and enclosed contents will be removed when return object is dropped.
@@ -93,44 +79,6 @@ fn extract_libbpf_headers_to_disk(_target_dir: &Path) -> Result<Option<PathBuf>>
     Ok(None)
 }
 
-fn check_clang(debug: bool, clang: &Path, skip_version_checks: bool) -> Result<()> {
-    let output = Command::new(clang.as_os_str())
-        .arg("--version")
-        .output()
-        .context("Failed to execute clang")?;
-
-    if !output.status.success() {
-        bail!("Failed to execute clang binary");
-    }
-
-    if skip_version_checks {
-        return Ok(());
-    }
-
-    // Example output:
-    //
-    //     clang version 10.0.0
-    //     Target: x86_64-pc-linux-gnu
-    //     Thread model: posix
-    //     InstalledDir: /bin
-    //
-    let output = String::from_utf8_lossy(&output.stdout);
-    let version_str = extract_version(&output)?;
-    let version = Version::parse(version_str)?;
-    if debug {
-        println!("{} is version {}", clang.display(), version);
-    }
-
-    if version < Version::parse("10.0.0").unwrap() {
-        bail!(
-            "version {} is too old. Use --skip-clang-version-checks to skip version check",
-            version
-        );
-    }
-
-    Ok(())
-}
-
 /// Strip DWARF information from the provided BPF object file.
 ///
 /// We rely on the `libbpf` linker here, which removes debug information as a
@@ -302,7 +250,6 @@ pub fn build(
     manifest_path: Option<&PathBuf>,
     clang: Option<&PathBuf>,
     clang_args: Vec<OsString>,
-    skip_clang_version_checks: bool,
 ) -> Result<()> {
     let (target_dir, to_compile) = metadata::get(debug, manifest_path)?;
 
@@ -318,8 +265,6 @@ pub fn build(
     check_progs(&to_compile)?;
 
     let clang = extract_clang_or_default(clang);
-    check_clang(debug, &clang, skip_clang_version_checks)
-        .with_context(|| anyhow!("{} is invalid", clang.display()))?;
     compile(debug, &to_compile, &clang, clang_args, &target_dir)
         .context("Failed to compile progs")?;
 
@@ -332,11 +277,9 @@ pub(crate) fn build_single(
     source: &Path,
     out: &Path,
     clang: Option<&PathBuf>,
-    skip_clang_version_checks: bool,
     mut clang_args: Vec<OsString>,
 ) -> Result<CompilationOutput> {
     let clang = extract_clang_or_default(clang);
-    check_clang(debug, &clang, skip_clang_version_checks)?;
     let header_parent_dir = tempdir()?;
     let header_dir = extract_libbpf_headers_to_disk(header_parent_dir.path())?;
 
@@ -351,23 +294,3 @@ pub(crate) fn build_single(
 
     compile_one(debug, source, out, &clang, &clang_args)
 }
-
-#[test]
-fn test_extract_version() {
-    let upstream_format = r"clang version 10.0.0
-Target: x86_64-pc-linux-gnu
-Thread model: posix
-InstalledDir: /bin
-";
-    assert_eq!(extract_version(upstream_format).unwrap(), "10.0.0");
-
-    let ubuntu_format = r"Ubuntu clang version 11.0.1-++20201121072624+973b95e0a84-1~exp1~20201121063303.19
-Target: x86_64-pc-linux-gnu
-Thread model: posix
-InstalledDir: /bin
-";
-    assert_eq!(extract_version(ubuntu_format).unwrap(), "11.0.1");
-
-    assert!(extract_version("askldfjwe").is_err());
-    assert!(extract_version("my clang version 1.5").is_err());
-}

libbpf-cargo/src/lib.rs

@@ -110,7 +110,6 @@ pub struct SkeletonBuilder {
     obj: Option<PathBuf>,
     clang: Option<PathBuf>,
     clang_args: Vec<OsString>,
-    skip_clang_version_check: bool,
     rustfmt: PathBuf,
     dir: Option<TempDir>,
 }
@@ -129,7 +128,6 @@ impl SkeletonBuilder {
             obj: None,
             clang: None,
             clang_args: Vec::new(),
-            skip_clang_version_check: false,
             rustfmt: "rustfmt".into(),
             dir: None,
         }
@@ -195,14 +193,6 @@ impl SkeletonBuilder {
         self
     }
 
-    /// Specify whether or not to skip clang version check
-    ///
-    /// Default is `false`
-    pub fn skip_clang_version_check(&mut self, skip: bool) -> &mut SkeletonBuilder {
-        self.skip_clang_version_check = skip;
-        self
-    }
-
     /// Specify which `rustfmt` binary to use
     ///
     /// Default searches `$PATH` for `rustfmt`
@@ -256,7 +246,6 @@ impl SkeletonBuilder {
             // Unwrap is safe here since we guarantee that obj.is_some() above
             self.obj.as_ref().unwrap(),
             self.clang.as_ref(),
-            self.skip_clang_version_check,
             self.clang_args.clone(),
         )
         .with_context(|| format!("failed to build `{}`", source.display()))

libbpf-cargo/src/main.rs

@@ -52,9 +52,6 @@ pub struct ClangOpts {
     /// Additional arguments to pass to `clang`.
     #[arg(long, value_parser)]
     clang_args: Vec<OsString>,
-    /// Skip clang version checks
-    #[arg(long)]
-    skip_clang_version_checks: bool,
 }
 
 /// cargo-libbpf is a cargo subcommand that helps develop and build eBPF (BPF) programs.
@@ -116,14 +113,12 @@ fn main() -> Result<()> {
                     ClangOpts {
                         clang_path,
                         clang_args,
-                        skip_clang_version_checks,
                     },
             } => build::build(
                 debug,
                 manifest_path.as_ref(),
                 clang_path.as_ref(),
                 clang_args,
-                skip_clang_version_checks,
             ),
             Command::Gen {
                 manifest_path,
@@ -141,7 +136,6 @@ fn main() -> Result<()> {
                     ClangOpts {
                         clang_path,
                         clang_args,
-                        skip_clang_version_checks,
                     },
                 quiet,
                 cargo_build_args,
@@ -151,7 +145,6 @@ fn main() -> Result<()> {
                 manifest_path.as_ref(),
                 clang_path.as_ref(),
                 clang_args,
-                skip_clang_version_checks,
                 quiet,
                 cargo_build_args,
                 rustfmt_path.as_ref(),

libbpf-cargo/src/make.rs

@@ -15,22 +15,15 @@ pub fn make(
     manifest_path: Option<&PathBuf>,
     clang: Option<&PathBuf>,
     clang_args: Vec<OsString>,
-    skip_clang_version_checks: bool,
     quiet: bool,
     cargo_build_args: Vec<String>,
     rustfmt_path: Option<&PathBuf>,
 ) -> Result<()> {
     if !quiet {
         println!("Compiling BPF objects");
     }
-    build::build(
-        debug,
-        manifest_path,
-        clang,
-        clang_args,
-        skip_clang_version_checks,
-    )
-    .context("Failed to compile BPF objects")?;
+    build::build(debug, manifest_path, clang, clang_args)
+        .context("Failed to compile BPF objects")?;
 
     if !quiet {
         println!("Generating skeletons");