Re: [RFC PATCH 3/6] contrib/cgit-rs: introduce Rust wrapper for libgit.a

From: Mike Hommey <mh@glandium.org>
Date: Aug 7, 2024, 22:47 UTC
Message-ID: <20240807224756.2zq5hkfq5j43b7jk@glandium.org>
In-Reply-To: <9a846c17c891e17566a9907b3627210a6a08ea76.1723054623.git.steadmon@google.com>

On Wed, Aug 07, 2024 at 11:21:28AM -0700, Josh Steadmon wrote:

> +contrib/cgit-rs/hidden_symbol_export.o: contrib/cgit-rs/partial_symbol_export.o
> +	$(OBJCOPY) --localize-hidden $^ $@

This is ELF-specific and won't work on Mac or Windows.

Show 5 quoted lines

> +    let make_output = std::process::Command::new("make")
> +        .env_remove("PROFILE")
> +        .current_dir(git_root.clone())
> +        .args(&[
> +            "CC=clang",

You should probably not set CC at all here.

> +            "CFLAGS=-fvisibility=hidden",

This won't work for Windows targets.

Show 13 quoted lines

> +            "contrib/cgit-rs/libcgit.a"
> +        ])
> +        .output()
> +        .expect("Make failed to run");
> +    if !make_output.status.success() {
> +        panic!(
> +                "Make failed:\n  stdout = {}\n  stderr = {}\n",
> +                String::from_utf8(make_output.stdout).unwrap(),
> +                String::from_utf8(make_output.stderr).unwrap()
> +        );
> +    }
> +    std::fs::copy(crate_root.join("libcgit.a"), dst.join("libcgit.a"))?;
> +    println!("cargo::rustc-link-search=native={}", dst.into_os_string().into_string().unwrap());

You might as well use `dst.display()`.

> +    println!("cargo::rustc-link-lib=cgit");
> +    println!("cargo::rustc-link-lib=z");
> +    println!("cargo::rerun-if-changed={}", git_root.into_os_string().into_string().unwrap());

Likewise.

Mike