Re: [RFC PATCH 3/6] contrib/cgit-rs: introduce Rust wrapper for libgit.a
- From
Josh Steadmon <steadmon@google.com>
- Date
- Aug 7, 2024, 23:29 UTC
- Message-ID
- <njekhcypd7x7vdxmkzbdbeblwm4bwiiz74saxdmzqeod6zz547@qkkwkmpjvwkd>
- In-Reply-To
- <20240807224756.2zq5hkfq5j43b7jk@glandium.org>
On 2024.08.08 07:47, Mike Hommey wrote:
Show 13 quoted lines
> 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.
Show 15 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()`.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.
Show 7 quoted lines
> > + 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