tikv / grpc-rs

The gRPC library for Rust built on C Core library and futures
Apache License 2.0
1.81k stars 253 forks source link

Panic when building on FreeBSD: "grpcwrap_batch_context_(unnamed_struct_at_grpc_wrap_cc_82_3)" is not a valid Ident #649

Closed nkosi23 closed 6 months ago

nkosi23 commented 6 months ago

Describe the bug Building the bindgen crate on FreeBSD produces the following panic (this was run with RUST_BACKTRACE=full but the backtrace is empty):

thread 'bindgen_grpc' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.59.2/src/ir/context.rs:878:9:
    "grpcwrap_batch_context_(unnamed_struct_at_grpc_wrap_cc_82_3)" is not a valid Ident
    stack backtrace:
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
    thread 'main' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/grpcio-sys-0.13.0+1.56.2-patched/build.rs:521:14:
    called `Result::unwrap()` on an `Err` value: Any { .. }
    stack backtrace:
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The section of the file in question is the below:

    /// Returns a mangled name as a rust identifier.
    pub fn rust_ident_raw<T>(&self, name: T) -> Ident
    where
        T: AsRef<str>,
    {
        Ident::new(name.as_ref(), Span::call_site())
    }

More complete logs can be found here.

To Reproduce Steps to reproduce the behavior: Build the grpcio-sys crate under FreeBSD

Expected behavior The build completes without panic and error.

System information

Additional context N/A

BusyJay commented 6 months ago

Does it work if using the master version?

nkosi23 commented 6 months ago

@BusyJay Thanks a lot for your swift reply, unfortunately I am getting the same error when using the master version

nkosi23 commented 6 months ago

I may have located the issue, maybe "freebsd" must be added to the following places:

grpc-rs/grpc-sys/build.rs:

    #[cfg(any(
        feature = "_gen-bindings",
        not(all(
            any(target_os = "linux", target_os = "macos"),
            any(target_arch = "x86_64", target_arch = "aarch64")
        ))
    ))]
    {
        // On some system (like Windows), stack size of main thread may
        // be too small.
        let f = file_path.clone();
        std::thread::Builder::new()
            .stack_size(8 * 1024 * 1024)
            .name("bindgen_grpc".to_string())
            .spawn(move || bindgen_grpc(&f))
            .unwrap()
            .join()
            .unwrap();
    }

grpc-rs/grpc-sys/Cargo.toml:

[target.'cfg(not(all(any(target_os = "linux", target_os = "macos"), any(target_arch = "x86_64", target_arch = "aarch64"))))'.build-dependencies]
bindgen = { version = "0.69.0", default-features = false, features = ["runtime"] }

A problem with the stack size would explain why a panic is triggered during the build process. If freebsd is added to the target_os array of the Cargo file, my understanding is that bindgen would not be included as a dependency on FreeBSD, and the code section in build.rs would not be compiled and executed. But I do not have enough understanding of the domain/project to understand the implications.

BusyJay commented 6 months ago

No, this is an issue with old rust-bindgen and new clang. It's expected to be fixed in bindgen >= 0.61. And master has upgraded bindgen to 0.69, so I asked to check whether master can be compiled.

nkosi23 commented 6 months ago

@BusyJay You were right, I have isolated the repo and can confirm that the build completes without problem when building against master. Sorry for the confusion.

Do you plan to push a release some time or maybe create a tag to ensure that we are building against a relatively stable version?