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

On 2024.08.08 07:47, Mike Hommey wrote:

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

quoted

+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.

quoted

+    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.

Ack, fixed in V2.

quoted

+            "CFLAGS=-fvisibility=hidden",

This won't work for Windows targets.

Understood. I'll have to do some studying on the symbol visibility
options.

quoted

+            "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()`.

Wouldn't that fail silently in the event that the path is non-UTF-8? I
think I'd prefer to explicitly fail in that case, even if it seems
unlikely.

quoted

+    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