| Message ID | 20240906222116.270196-2-calvinwan@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce libgit-rs, a Rust wrapper around libgit.a | expand |
On Fri, Sep 6, 2024 at 6:21 PM Calvin Wan <calvinwan@google.com> wrote: > From: Josh Steadmon <steadmon@google.com> This is curious... > Co-authored-by: Kyle Lippincott <spectral@google.com> > Co-authored-by: Calvin Wan <calvinwan@google.com> > Signed-off-by: Calvin Wan <calvinwan@google.com> > Signed-off-by: Kyle Lippincott <spectral@google.com> ... since there is no mention of Josh here. > diff --git a/contrib/libgit-rs/libgit-sys/Cargo.toml b/contrib/libgit-rs/libgit-sys/Cargo.toml > @@ -0,0 +1,12 @@ > +[package] > +name = "libgit-sys" > [...] > +[dependencies] > +libz-sys = "1.1.19" > \ No newline at end of file Let's give this file a proper line terminator. > diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs > @@ -0,0 +1,31 @@ > +pub fn main() -> std::io::Result<()> { > + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); > + let git_root = crate_root.join("../../.."); > + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); > + > + let make_output = std::process::Command::new("make") Providing a mechanism for people to override this hardcoded spelling of "make" could be another item for your NEEDSWORK list; in particular, I'm thinking about platforms on which GNU "make" is installed as "gmake". > diff --git a/contrib/libgit-rs/libgit-sys/public_symbol_export.c b/contrib/libgit-rs/libgit-sys/public_symbol_export.c > @@ -0,0 +1,20 @@ > +// Shim to publicly export Git symbols. These must be renamed so that the > +// original symbols can be hidden. Renaming these with a "libgit_" prefix also > +// avoid conflicts with other libraries such as libgit2. s/avoid/avoids/
On Fri, Sep 06, 2024 at 06:39:17PM -0400, Eric Sunshine wrote: > > diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs > > @@ -0,0 +1,31 @@ > > +pub fn main() -> std::io::Result<()> { > > + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); > > + let git_root = crate_root.join("../../.."); > > + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); > > + > > + let make_output = std::process::Command::new("make") > > Providing a mechanism for people to override this hardcoded spelling > of "make" could be another item for your NEEDSWORK list; in > particular, I'm thinking about platforms on which GNU "make" is > installed as "gmake". And/or, use the `make_cmd` crate. Mike
Mike Hommey <mh@glandium.org> writes: > On Fri, Sep 06, 2024 at 06:39:17PM -0400, Eric Sunshine wrote: >> > diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs >> > @@ -0,0 +1,31 @@ >> > +pub fn main() -> std::io::Result<()> { >> > + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); >> > + let git_root = crate_root.join("../../.."); >> > + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); >> > + >> > + let make_output = std::process::Command::new("make") >> >> Providing a mechanism for people to override this hardcoded spelling >> of "make" could be another item for your NEEDSWORK list; in >> particular, I'm thinking about platforms on which GNU "make" is >> installed as "gmake". > > And/or, use the `make_cmd` crate. Thanks for helping.
Thanks for the catches -- will fix in the next reroll
On 2024.09.06 18:39, Eric Sunshine wrote: > On Fri, Sep 6, 2024 at 6:21 PM Calvin Wan <calvinwan@google.com> wrote: > > From: Josh Steadmon <steadmon@google.com> > > This is curious... > > > Co-authored-by: Kyle Lippincott <spectral@google.com> > > Co-authored-by: Calvin Wan <calvinwan@google.com> > > Signed-off-by: Calvin Wan <calvinwan@google.com> > > Signed-off-by: Kyle Lippincott <spectral@google.com> > > ... since there is no mention of Josh here. > > > diff --git a/contrib/libgit-rs/libgit-sys/Cargo.toml b/contrib/libgit-rs/libgit-sys/Cargo.toml > > @@ -0,0 +1,12 @@ > > +[package] > > +name = "libgit-sys" > > [...] > > +[dependencies] > > +libz-sys = "1.1.19" > > \ No newline at end of file > > Let's give this file a proper line terminator. Fixed in V4. > > diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs > > @@ -0,0 +1,31 @@ > > +pub fn main() -> std::io::Result<()> { > > + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); > > + let git_root = crate_root.join("../../.."); > > + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); > > + > > + let make_output = std::process::Command::new("make") > > Providing a mechanism for people to override this hardcoded spelling > of "make" could be another item for your NEEDSWORK list; in > particular, I'm thinking about platforms on which GNU "make" is > installed as "gmake". Ack, will use make_cmd as Mike suggests. > > diff --git a/contrib/libgit-rs/libgit-sys/public_symbol_export.c b/contrib/libgit-rs/libgit-sys/public_symbol_export.c > > @@ -0,0 +1,20 @@ > > +// Shim to publicly export Git symbols. These must be renamed so that the > > +// original symbols can be hidden. Renaming these with a "libgit_" prefix also > > +// avoid conflicts with other libraries such as libgit2. > > s/avoid/avoids/ Fixed in V4. Thanks for the review!
On 2024.09.07 08:04, Mike Hommey wrote: > On Fri, Sep 06, 2024 at 06:39:17PM -0400, Eric Sunshine wrote: > > > diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs > > > @@ -0,0 +1,31 @@ > > > +pub fn main() -> std::io::Result<()> { > > > + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); > > > + let git_root = crate_root.join("../../.."); > > > + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); > > > + > > > + let make_output = std::process::Command::new("make") > > > > Providing a mechanism for people to override this hardcoded spelling > > of "make" could be another item for your NEEDSWORK list; in > > particular, I'm thinking about platforms on which GNU "make" is > > installed as "gmake". > > And/or, use the `make_cmd` crate. Will do, thanks!
diff --git a/.gitignore b/.gitignore index 8caf3700c2..dfd72820fb 100644 --- a/.gitignore +++ b/.gitignore @@ -248,3 +248,4 @@ Release/ /git.VC.db *.dSYM /contrib/buildsystems/out +/contrib/libgit-rs/libgit-sys/target diff --git a/Makefile b/Makefile index 7caeb3c872..0090514e55 100644 --- a/Makefile +++ b/Makefile @@ -653,6 +653,8 @@ CURL_CONFIG = curl-config GCOV = gcov STRIP = strip SPATCH = spatch +LD = ld +OBJCOPY = objcopy export TCL_PATH TCLTK_PATH @@ -2713,6 +2715,7 @@ OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS) OBJECTS += $(UNIT_TEST_OBJS) +OBJECTS += contrib/libgit-rs/libgit-sys/public_symbol_export.o ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o @@ -3720,6 +3723,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(htmldocs).tar.gz $(manpages).tar.gz $(MAKE) -C Documentation/ clean $(RM) Documentation/GIT-EXCLUDED-PROGRAMS + $(RM) -r contrib/libgit-rs/libgit-sys/target ifndef NO_PERL $(RM) -r perl/build/ endif @@ -3865,3 +3869,12 @@ $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o \ build-unit-tests: $(UNIT_TEST_PROGS) unit-tests: $(UNIT_TEST_PROGS) t/helper/test-tool$X $(MAKE) -C t/ unit-tests + +contrib/libgit-rs/libgit-sys/partial_symbol_export.o: contrib/libgit-rs/libgit-sys/public_symbol_export.o libgit.a reftable/libreftable.a xdiff/lib.a + $(LD) -r $^ -o $@ + +contrib/libgit-rs/libgit-sys/hidden_symbol_export.o: contrib/libgit-rs/libgit-sys/partial_symbol_export.o + $(OBJCOPY) --localize-hidden $^ $@ + +contrib/libgit-rs/libgit-sys/libgitpub.a: contrib/libgit-rs/libgit-sys/hidden_symbol_export.o + $(AR) $(ARFLAGS) $@ $^ diff --git a/contrib/libgit-rs/libgit-sys/Cargo.lock b/contrib/libgit-rs/libgit-sys/Cargo.lock new file mode 100644 index 0000000000..af8676ab42 --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/Cargo.lock @@ -0,0 +1,55 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "cc" +version = "1.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57b6a275aa2903740dc87da01c62040406b8812552e97129a63ea8850a17c6e6" +dependencies = [ + "shlex", +] + +[[package]] +name = "libc" +version = "0.2.158" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" + +[[package]] +name = "libgit-sys" +version = "0.1.0" +dependencies = [ + "libz-sys", +] + +[[package]] +name = "libz-sys" +version = "1.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2d16453e800a8cf6dd2fc3eb4bc99b786a9b90c663b8559a5b1a041bf89e472" +dependencies = [ + "cc", + "libc", + "pkg-config", + "vcpkg", +] + +[[package]] +name = "pkg-config" +version = "0.3.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d231b230927b5e4ad203db57bbcbee2802f6bce620b1e4a9024a07d94e2907ec" + +[[package]] +name = "shlex" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" + +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" diff --git a/contrib/libgit-rs/libgit-sys/Cargo.toml b/contrib/libgit-rs/libgit-sys/Cargo.toml new file mode 100644 index 0000000000..905fd6774f --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "libgit-sys" +version = "0.1.0" +edition = "2021" +build = "build.rs" +links = "gitpub" + +[lib] +path = "src/lib.rs" + +[dependencies] +libz-sys = "1.1.19" \ No newline at end of file diff --git a/contrib/libgit-rs/libgit-sys/README.md b/contrib/libgit-rs/libgit-sys/README.md new file mode 100644 index 0000000000..7a59602c30 --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/README.md @@ -0,0 +1,15 @@ +# cgit-info + +A small hacky proof-of-concept showing how to provide a Rust FFI for the Git +library. + +## Building + +`cargo build` automatically builds and picks up on changes made to both +the Rust wrapper and git.git code so there is no need to run `make` +beforehand. + +## Running + +Assuming you don't make any changes to the Git source, you can just work from +`contrib/cgit-rs` and use `cargo build` or `cargo run` as usual. diff --git a/contrib/libgit-rs/libgit-sys/build.rs b/contrib/libgit-rs/libgit-sys/build.rs new file mode 100644 index 0000000000..c187ab6cd0 --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/build.rs @@ -0,0 +1,31 @@ +use std::env; +use std::path::PathBuf; + +pub fn main() -> std::io::Result<()> { + let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + let git_root = crate_root.join("../../.."); + let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + + let make_output = std::process::Command::new("make") + .env_remove("PROFILE") + .current_dir(git_root.clone()) + .args([ + "CFLAGS=-fvisibility=hidden", + "contrib/libgit-rs/libgit-sys/libgitpub.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("libgitpub.a"), dst.join("libgitpub.a"))?; + println!("cargo::rustc-link-search=native={}", dst.display()); + println!("cargo::rustc-link-lib=gitpub"); + println!("cargo::rerun-if-changed={}", git_root.display()); + + Ok(()) +} diff --git a/contrib/libgit-rs/libgit-sys/public_symbol_export.c b/contrib/libgit-rs/libgit-sys/public_symbol_export.c new file mode 100644 index 0000000000..39c27d9c1a --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/public_symbol_export.c @@ -0,0 +1,20 @@ +// Shim to publicly export Git symbols. These must be renamed so that the +// original symbols can be hidden. Renaming these with a "libgit_" prefix also +// avoid conflicts with other libraries such as libgit2. + +#include "contrib/libgit-rs/libgit-sys/public_symbol_export.h" +#include "version.h" + +#pragma GCC visibility push(default) + +const char *libgit_user_agent(void) +{ + return git_user_agent(); +} + +const char *libgit_user_agent_sanitized(void) +{ + return git_user_agent_sanitized(); +} + +#pragma GCC visibility pop diff --git a/contrib/libgit-rs/libgit-sys/public_symbol_export.h b/contrib/libgit-rs/libgit-sys/public_symbol_export.h new file mode 100644 index 0000000000..a3372f93fa --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/public_symbol_export.h @@ -0,0 +1,8 @@ +#ifndef PUBLIC_SYMBOL_EXPORT_H +#define PUBLIC_SYMBOL_EXPORT_H + +const char *libgit_user_agent(void); + +const char *libgit_user_agent_sanitized(void); + +#endif /* PUBLIC_SYMBOL_EXPORT_H */ diff --git a/contrib/libgit-rs/libgit-sys/src/lib.rs b/contrib/libgit-rs/libgit-sys/src/lib.rs new file mode 100644 index 0000000000..346b26083d --- /dev/null +++ b/contrib/libgit-rs/libgit-sys/src/lib.rs @@ -0,0 +1,42 @@ +use std::ffi::c_char; + +extern crate libz_sys; + +extern "C" { + // From version.c + pub fn libgit_user_agent() -> *const c_char; + pub fn libgit_user_agent_sanitized() -> *const c_char; +} + +#[cfg(test)] +mod tests { + use std::ffi::CStr; + + use super::*; + + #[test] + fn user_agent_starts_with_git() { + let c_str = unsafe { CStr::from_ptr(libgit_user_agent()) }; + let agent = c_str + .to_str() + .expect("User agent contains invalid UTF-8 data"); + assert!( + agent.starts_with("git/"), + r#"Expected user agent to start with "git/", got: {}"#, + agent + ); + } + + #[test] + fn sanitized_user_agent_starts_with_git() { + let c_str = unsafe { CStr::from_ptr(libgit_user_agent_sanitized()) }; + let agent = c_str + .to_str() + .expect("Sanitized user agent contains invalid UTF-8 data"); + assert!( + agent.starts_with("git/"), + r#"Expected user agent to start with "git/", got: {}"#, + agent + ); + } +}