On 2024.08.08 07:47, Mike Hommey wrote:
> 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.
>
> > + 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.
> > + "CFLAGS=-fvisibility=hidden",
>
> This won't work for Windows targets.
Understood. I'll have to do some studying on the symbol visibility
options.
> > + "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.
> > + 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
