[PATCH v6 0/5] Introduce libgit-rs, a Rust wrapper around libgit.a

Apologies for the long delay on V6; I am finally back after several months of $DAYJOB firefighting, holidays, and sick leave. I should have time to devote to this series again, but given the lack of feedback on V5 I am hopeful that this will be the final iteration of this series.

This series provides two small Rust wrapper libraries around parts of
Git: "libgit-sys", which exposes a few functions from libgit.a, and
"libgit", which provides a more Rust-friendly interface to some of those
functions. In addition to included unit tests, at $DAYJOB we have tested
building JJ[1] with our library and used it to replace some of the
libgit2-rs uses.

[1] https://github.com/martinvonz/jj

There is known NEEDSWORK, but I feel that they can be addressed in follow-up changes, rather than in this series. If you feel otherwise, please let me know:

* Investigate alternative methods of managing symbol visibility &
  renaming.

* Figure out symbol versioning

Changes in V6:
* Rebased onto current master, since V5 was several months old.

* Move libgit-sys out of libgit-rs; while this sort of nesting is common
  in Rust crates with standalone repositories, it doesn't make as much
  sense when they're contained in the larger Git project's repo.

* Standardize the naming of some of the Makefile targets to always
  include a dash in the "-rs" or "-sys" suffixes.

* Clean up READMEs and crate descriptions in preparation for
  uploading to crates.io.

Changes in V5:
* When building with INCLUDE_LIBGIT_RS defined, add
  "-fvisibility=hidden" to CFLAGS. This allows us to manage symbol
  visibility in libgitpub.a without causing `make all` to rebuild from
  scratch due to changing CFLAGS.

* Avoid using c_int in the higher-level Rust API.

* Remove libgitpub.a and intermediate files with `make clean`.

Changes in V4:
* Drop V3 patch #3, which added wrappers around repository
  initialization and config access. These are not well-libified, and
  they are not necessary for JJ's proof-of-concept use case, so let's
  avoid exporting them for now.

* Set a minimum supported Rust version of 1.63. Autodetect whether our
  Rust version has c_int and c_char types; if not, define them
  ourselves.

* When building libgitpub.a via build.rs, set DEVELOPER=1 to catch
  additional errors at build time.

* In build.rs, use the make_cmd crate to portable select the correct
  invocation of GNU Make.

* Follow naming standards for _alloc() and _free() functions.

* Use String instead of CString in higher-level API.

* Move libgit_configset_alloc() and libgit_configset_free() out of
  upstream Git, to the libgitpub shim library.

* In libgitpub, initialize libgit_config_set structs in the _alloc()
  function rather than with a separate _init() function.

* Remove unnecessary comments in libgit-sys showing where the wrapped
  functions were originally defined.

* Fix clippy lint: don't reborrow configfile path references.

* Various typo fixes and `cargo fmt` fixes.

Changes in V3:
* Renamed cgit-rs to libgit-rs and cgit-sys to libgit-sys

* Makefile cleanup, particularly adding config.mak options that
  developers can set to run Rust builds and tests by default (Patch 6)

* Provide testdata configs for unit tests

* ConfigSet API now uses &Path instead of &str -- more ergonomic for
  Rust users to pass in and errors out if the path string isn't UTF-8

* Fixed unresolved dependency on libz in Cargo.toml

Calvin Wan (2):
  libgit: add higher-level libgit crate
  Makefile: add option to build and test libgit-rs and libgit-rs-sys

Josh Steadmon (3):
  common-main: split init and exit code into new files
  libgit-sys: introduce Rust wrapper for libgit.a
  libgit-sys: also export some config_set functions

 .gitignore                                |  2 +
 Makefile                                  | 44 +++++++++++
 common-exit.c                             | 26 +++++++
 common-init.c                             | 63 +++++++++++++++
 common-init.h                             |  6 ++
 common-main.c                             | 83 +-------------------
 contrib/libgit-rs/Cargo.lock              | 77 ++++++++++++++++++
 contrib/libgit-rs/Cargo.toml              | 15 ++++
 contrib/libgit-rs/README.md               | 13 ++++
 contrib/libgit-rs/build.rs                |  4 +
 contrib/libgit-rs/src/lib.rs              | 95 +++++++++++++++++++++++
 contrib/libgit-rs/testdata/config1        |  2 +
 contrib/libgit-rs/testdata/config2        |  2 +
 contrib/libgit-rs/testdata/config3        |  2 +
 contrib/libgit-sys/Cargo.lock             | 69 ++++++++++++++++
 contrib/libgit-sys/Cargo.toml             | 18 +++++
 contrib/libgit-sys/README.md              |  4 +
 contrib/libgit-sys/build.rs               | 35 +++++++++
 contrib/libgit-sys/public_symbol_export.c | 50 ++++++++++++
 contrib/libgit-sys/public_symbol_export.h | 18 +++++
 contrib/libgit-sys/src/lib.rs             | 79 +++++++++++++++++++
 t/Makefile                                | 16 ++++
 22 files changed, 642 insertions(+), 81 deletions(-)
 create mode 100644 common-exit.c
 create mode 100644 common-init.c
 create mode 100644 common-init.h
 create mode 100644 contrib/libgit-rs/Cargo.lock
 create mode 100644 contrib/libgit-rs/Cargo.toml
 create mode 100644 contrib/libgit-rs/README.md
 create mode 100644 contrib/libgit-rs/build.rs
 create mode 100644 contrib/libgit-rs/src/lib.rs
 create mode 100644 contrib/libgit-rs/testdata/config1
 create mode 100644 contrib/libgit-rs/testdata/config2
 create mode 100644 contrib/libgit-rs/testdata/config3
 create mode 100644 contrib/libgit-sys/Cargo.lock
 create mode 100644 contrib/libgit-sys/Cargo.toml
 create mode 100644 contrib/libgit-sys/README.md
 create mode 100644 contrib/libgit-sys/build.rs
 create mode 100644 contrib/libgit-sys/public_symbol_export.c
 create mode 100644 contrib/libgit-sys/public_symbol_export.h
 create mode 100644 contrib/libgit-sys/src/lib.rs

Range-diff against v5:
1:  1ae14207f6 = 1:  ff6cd62397 common-main: split init and exit code into new files
2:  1ed010c378 ! 2:  5fc66cdb16 libgit-sys: introduce Rust wrapper for libgit.a
    @@ .gitignore: Release/
      /git.VC.db
      *.dSYM
      /contrib/buildsystems/out
    -+/contrib/libgit-rs/libgit-sys/target
    ++/contrib/libgit-sys/target
     
      ## Makefile ##
     @@ Makefile: CURL_CONFIG = curl-config
    @@ Makefile: CURL_CONFIG = curl-config
      
      export TCL_PATH TCLTK_PATH
      
    -@@ Makefile: OBJECTS += $(XDIFF_OBJS)
    - OBJECTS += $(FUZZ_OBJS)
    - OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
    +@@ Makefile: OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
      OBJECTS += $(UNIT_TEST_OBJS)
    -+OBJECTS += contrib/libgit-rs/libgit-sys/public_symbol_export.o
    + OBJECTS += $(CLAR_TEST_OBJS)
    + OBJECTS += $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
    ++OBJECTS += contrib/libgit-sys/public_symbol_export.o
      
      ifndef NO_CURL
      	OBJECTS += http.o http-walker.o remote-curl.o
    @@ Makefile: 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
    -+	$(RM) -r contrib/libgit-rs/libgit-sys/partial_symbol_export.o
    -+	$(RM) -r contrib/libgit-rs/libgit-sys/hidden_symbol_export.o
    -+	$(RM) -r contrib/libgit-rs/libgit-sys/libgitpub.a
    ++	$(RM) -r contrib/libgit-sys/target
    ++	$(RM) -r contrib/libgit-sys/partial_symbol_export.o
    ++	$(RM) -r contrib/libgit-sys/hidden_symbol_export.o
    ++	$(RM) -r contrib/libgit-sys/libgitpub.a
      ifndef NO_PERL
      	$(RM) -r perl/build/
      endif
    -@@ Makefile: $(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
    +@@ Makefile: $(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-
    + build-unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
    + unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) 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
    ++contrib/libgit-sys/partial_symbol_export.o: contrib/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
    ++contrib/libgit-sys/hidden_symbol_export.o: contrib/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
    ++contrib/libgit-sys/libgitpub.a: contrib/libgit-sys/hidden_symbol_export.o
     +	$(AR) $(ARFLAGS) $@ $^
     
    - ## contrib/libgit-rs/libgit-sys/Cargo.lock (new) ##
    + ## contrib/libgit-sys/Cargo.lock (new) ##
     @@
     +# This file is automatically @generated by Cargo.
     +# It is not intended for manual editing.
    @@ contrib/libgit-rs/libgit-sys/Cargo.lock (new)
     +source = "registry+https://github.com/rust-lang/crates.io-index"
     +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426"
     
    - ## contrib/libgit-rs/libgit-sys/Cargo.toml (new) ##
    + ## contrib/libgit-sys/Cargo.toml (new) ##
     @@
     +[package]
     +name = "libgit-sys"
    @@ contrib/libgit-rs/libgit-sys/Cargo.toml (new)
     +build = "build.rs"
     +links = "gitpub"
     +rust-version = "1.63"
    ++description = "Native bindings to a portion of libgit"
     +
     +[lib]
     +path = "src/lib.rs"
    @@ contrib/libgit-rs/libgit-sys/Cargo.toml (new)
     +autocfg = "1.4.0"
     +make-cmd = "0.1.0"
     
    - ## contrib/libgit-rs/libgit-sys/README.md (new) ##
    + ## contrib/libgit-sys/README.md (new) ##
     @@
    -+# cgit-info
    ++# libgit-sys
     +
    -+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.
    ++A small proof-of-concept crate showing how to provide a Rust FFI to Git
    ++internals.
     
    - ## contrib/libgit-rs/libgit-sys/build.rs (new) ##
    + ## contrib/libgit-sys/build.rs (new) ##
     @@
     +use std::env;
     +use std::path::PathBuf;
    @@ contrib/libgit-rs/libgit-sys/build.rs (new)
     +    ac.emit_has_path("std::ffi::c_char");
     +
     +    let crate_root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
    -+    let git_root = crate_root.join("../../..");
    ++    let git_root = crate_root.join("../..");
     +    let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap());
     +
     +    let make_output = make_cmd::gnu_make()
    @@ contrib/libgit-rs/libgit-sys/build.rs (new)
     +        .current_dir(git_root.clone())
     +        .args([
     +            "CFLAGS=-fvisibility=hidden",
    -+            "contrib/libgit-rs/libgit-sys/libgitpub.a",
    ++            "contrib/libgit-sys/libgitpub.a",
     +        ])
     +        .output()
     +        .expect("Make failed to run");
    @@ contrib/libgit-rs/libgit-sys/build.rs (new)
     +    Ok(())
     +}
     
    - ## contrib/libgit-rs/libgit-sys/public_symbol_export.c (new) ##
    + ## contrib/libgit-sys/public_symbol_export.c (new) ##
     @@
     +// 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
     +// avoids conflicts with other libraries such as libgit2.
     +
     +#include "git-compat-util.h"
    -+#include "contrib/libgit-rs/libgit-sys/public_symbol_export.h"
    ++#include "contrib/libgit-sys/public_symbol_export.h"
     +#include "version.h"
     +
     +#pragma GCC visibility push(default)
    @@ contrib/libgit-rs/libgit-sys/public_symbol_export.c (new)
     +
     +#pragma GCC visibility pop
     
    - ## contrib/libgit-rs/libgit-sys/public_symbol_export.h (new) ##
    + ## contrib/libgit-sys/public_symbol_export.h (new) ##
     @@
     +#ifndef PUBLIC_SYMBOL_EXPORT_H
     +#define PUBLIC_SYMBOL_EXPORT_H
    @@ contrib/libgit-rs/libgit-sys/public_symbol_export.h (new)
     +
     +#endif /* PUBLIC_SYMBOL_EXPORT_H */
     
    - ## contrib/libgit-rs/libgit-sys/src/lib.rs (new) ##
    + ## contrib/libgit-sys/src/lib.rs (new) ##
     @@
     +#[cfg(has_std__ffi__c_char)]
     +use std::ffi::c_char;
3:  00762b57d0 ! 3:  03f39b6c3a libgit-sys: also export some config_set functions
    @@ Commit message
         Signed-off-by: Calvin Wan <calvinwan@google.com>
         Signed-off-by: Josh Steadmon <steadmon@google.com>
     
    - ## contrib/libgit-rs/libgit-sys/public_symbol_export.c ##
    + ## contrib/libgit-sys/public_symbol_export.c ##
     @@
      // avoids conflicts with other libraries such as libgit2.
      
      #include "git-compat-util.h"
     +#include "config.h"
    - #include "contrib/libgit-rs/libgit-sys/public_symbol_export.h"
    + #include "contrib/libgit-sys/public_symbol_export.h"
      #include "version.h"
      
      #pragma GCC visibility push(default)
    @@ contrib/libgit-rs/libgit-sys/public_symbol_export.c
      {
      	return git_user_agent();
     
    - ## contrib/libgit-rs/libgit-sys/public_symbol_export.h ##
    + ## contrib/libgit-sys/public_symbol_export.h ##
     @@
      #ifndef PUBLIC_SYMBOL_EXPORT_H
      #define PUBLIC_SYMBOL_EXPORT_H
    @@ contrib/libgit-rs/libgit-sys/public_symbol_export.h
      
      const char *libgit_user_agent_sanitized(void);
     
    - ## contrib/libgit-rs/libgit-sys/src/lib.rs ##
    + ## contrib/libgit-sys/src/lib.rs ##
     @@
      #[cfg(has_std__ffi__c_char)]
     -use std::ffi::c_char;
4:  4e5360931b ! 4:  65166ea0c0 libgit: add higher-level libgit crate
    @@ .gitignore: Release/
      *.dSYM
      /contrib/buildsystems/out
     +/contrib/libgit-rs/target
    - /contrib/libgit-rs/libgit-sys/target
    + /contrib/libgit-sys/target
     
      ## Makefile ##
     @@ Makefile: 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
    -+	$(RM) -r contrib/libgit-rs/target contrib/libgit-rs/libgit-sys/target
    - 	$(RM) -r contrib/libgit-rs/libgit-sys/partial_symbol_export.o
    - 	$(RM) -r contrib/libgit-rs/libgit-sys/hidden_symbol_export.o
    - 	$(RM) -r contrib/libgit-rs/libgit-sys/libgitpub.a
    +-	$(RM) -r contrib/libgit-sys/target
    ++	$(RM) -r contrib/libgit-rs/target contrib/libgit-sys/target
    + 	$(RM) -r contrib/libgit-sys/partial_symbol_export.o
    + 	$(RM) -r contrib/libgit-sys/hidden_symbol_export.o
    + 	$(RM) -r contrib/libgit-sys/libgitpub.a
     
      ## contrib/libgit-rs/Cargo.lock (new) ##
     @@
    @@ contrib/libgit-rs/Cargo.toml (new)
     +path = "src/lib.rs"
     +
     +[dependencies]
    -+libgit-sys = { version = "0.1.0", path = "libgit-sys" }
    ++libgit-sys = { version = "0.1.0", path = "../libgit-sys" }
     +
     +[build-dependencies]
     +autocfg = "1.4.0"
     
    + ## contrib/libgit-rs/README.md (new) ##
    +@@
    ++# libgit-rs
    ++
    ++Proof-of-concept Git bindings for Rust.
    ++
    ++```toml
    ++[dependencies]
    ++libgit = "0.1.0"
    ++```
    ++
    ++## Rust version requirements
    ++
    ++libgit-rs should support Rust versions at least as old as the version included
    ++in Debian stable (currently 1.63).
    +
      ## contrib/libgit-rs/build.rs (new) ##
     @@
     +pub fn main() {
    @@ contrib/libgit-rs/build.rs (new)
     +    ac.emit_has_path("std::ffi::c_char");
     +}
     
    - ## contrib/libgit-rs/libgit-sys/src/lib.rs ##
    -@@
    -+use std::ffi::c_void;
    -+
    - #[cfg(has_std__ffi__c_char)]
    - use std::ffi::{c_char, c_int};
    - 
    -@@ contrib/libgit-rs/libgit-sys/src/lib.rs: pub struct libgit_config_set {
    - }
    - 
    - extern "C" {
    -+    pub fn free(ptr: *mut c_void);
    -+
    -     pub fn libgit_user_agent() -> *const c_char;
    -     pub fn libgit_user_agent_sanitized() -> *const c_char;
    - 
    -
      ## contrib/libgit-rs/src/lib.rs (new) ##
     @@
     +use std::ffi::{c_void, CStr, CString};
    @@ contrib/libgit-rs/testdata/config3 (new)
     @@
     +[trace2]
     +	eventNesting = 3
    +
    + ## contrib/libgit-sys/src/lib.rs ##
    +@@
    ++use std::ffi::c_void;
    ++
    + #[cfg(has_std__ffi__c_char)]
    + use std::ffi::{c_char, c_int};
    + 
    +@@ contrib/libgit-sys/src/lib.rs: pub struct libgit_config_set {
    + }
    + 
    + extern "C" {
    ++    pub fn free(ptr: *mut c_void);
    ++
    +     pub fn libgit_user_agent() -> *const c_char;
    +     pub fn libgit_user_agent_sanitized() -> *const c_char;
    + 
5:  15ce989de8 ! 5:  84706f0db7 Makefile: add option to build and test libgit-rs and libgit-rs-sys
    @@ Makefile: ifdef FSMONITOR_OS_SETTINGS
      ifeq ($(TCLTK_PATH),)
      NO_TCLTK = NoThanks
      endif
    -@@ Makefile: build-unit-tests: $(UNIT_TEST_PROGS)
    - unit-tests: $(UNIT_TEST_PROGS) t/helper/test-tool$X
    +@@ Makefile: build-unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
    + unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) t/helper/test-tool$X
      	$(MAKE) -C t/ unit-tests
      
    -+.PHONY: libgitrs-sys
    -+libgitrs-sys:
    ++.PHONY: libgit-sys
    ++libgit-sys:
     +	$(QUIET)(\
    -+		cd contrib/libgit-rs/libgit-sys && \
    ++		cd contrib/libgit-sys && \
     +		cargo build \
     +	)
    -+.PHONY: libgitrs
    -+libgitrs:
    ++.PHONY: libgit-rs
    ++libgit-rs:
     +	$(QUIET)(\
     +		cd contrib/libgit-rs && \
     +		cargo build \
     +	)
     +ifdef INCLUDE_LIBGIT_RS
    -+all:: libgitrs
    ++all:: libgit-rs
     +endif
     +
    - 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
    + contrib/libgit-sys/partial_symbol_export.o: contrib/libgit-sys/public_symbol_export.o libgit.a reftable/libreftable.a xdiff/lib.a
      	$(LD) -r $^ -o $@
      
     
    - ## contrib/libgit-rs/libgit-sys/build.rs ##
    -@@ contrib/libgit-rs/libgit-sys/build.rs: pub fn main() -> std::io::Result<()> {
    + ## contrib/libgit-sys/build.rs ##
    +@@ contrib/libgit-sys/build.rs: pub fn main() -> std::io::Result<()> {
              .env_remove("PROFILE")
              .current_dir(git_root.clone())
              .args([
     -            "CFLAGS=-fvisibility=hidden",
     +            "INCLUDE_LIBGIT_RS=YesPlease",
    -             "contrib/libgit-rs/libgit-sys/libgitpub.a",
    +             "contrib/libgit-sys/libgitpub.a",
              ])
              .output()
     
    @@ t/Makefile: perf:
      .PHONY: pre-clean $(T) aggregate-results clean valgrind perf \
      	check-chainlint clean-chainlint test-chainlint $(UNIT_TESTS)
     +
    -+.PHONY: libgitrs-sys-test
    -+libgitrs-sys-test:
    ++.PHONY: libgit-sys-test
    ++libgit-sys-test:
     +	$(QUIET)(\
    -+		cd ../contrib/libgit-rs/libgit-sys && \
    ++		cd ../contrib/libgit-sys && \
     +		cargo test \
     +	)
    -+.PHONY: libgitrs-test
    -+libgitrs-test:
    ++.PHONY: libgit-rs-test
    ++libgit-rs-test:
     +	$(QUIET)(\
     +		cd ../contrib/libgit-rs && \
     +		cargo test \
     +	)
     +ifdef INCLUDE_LIBGIT_RS
    -+all:: libgitrs-sys-test libgitrs-test
    ++all:: libgit-sys-test libgit-rs-test
     +endif

base-commit: 757161efcca150a9a96b312d9e780a071e601a03