| Message ID | d592a3c2e3f56aa4f6915a07307a2ed349296272.1742339107.git.josh@steadmon.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix `cargo package` for libgit-sys | expand |
On 2025.03.18 16:24, Josh Steadmon wrote: > Unlike `cargo build`, `cargo package` does not get access to the entire Git repo > containing a Rust crate. Instead, it prepares a directory starting from the > crate root (potentially excluding files, such as those not under version > control, or explicity excluded in the Cargo.toml file). > > This means that the current method of building the libgit-sys crate does not > work with `cargo package`, as it tries to execute the Makefile from "../.." > relative to the crate root. > > Fix this by adding a `git-src` symlink in the crate that points to the Git > repository root. `cargo package` will flatten this to a copy of the Git repo, > excluding non-version-controlled files, any explicitly-excluded files, and trees > that contain a Cargo.toml file (this prevents infinite recursion on the > symlink). > > We can then execute the Makefile under the flattened git-src directory from our > build.rs script. However, this exposes a second problem; Cargo will check that > the build script does not add, delete, or modify any source files. This means > that after we copy our libgitpub.a dependency to the output directory, we must > run `make clean` to remove the object files we created during the build process. > > Unfortunately, there is not a way to determine from the build.rs script whether > we're running `cargo build` vs. `cargo package`, so now any build of the > libgit-sys crate will result in cleaning the Git worktree. > > A potential alternative is to make an additional temporary copy of the worktree > and run the Makefile there. This would avoid removing build artifacts in the > worktree at the cost of copying MBs worth of source files to a temporary > directory. Perhaps hardlinking instead of making a full copy would help here, > but that might be less portable. I'm currently working on an alternate solution where we build the object files in Cargo's working directory, rather than in the source tree. This will (mostly) avoid the need to clean after the build. I hope to send a v2 with this change in the next few days.
Hi Josh On 19/03/2025 22:23, Josh Steadmon wrote: > On 2025.03.18 16:24, Josh Steadmon wrote: > > I'm currently working on an alternate solution where we build the object > files in Cargo's working directory, rather than in the source tree. This > will (mostly) avoid the need to clean after the build. I hope to send a > v2 with this change in the next few days. Meson has builtin support for out-of-tree builds which would make building in Cargo's OUT_DIR trivial. Our meson build is still experimental though so you may not want to rely on it. Best Wishes Phillip
On 2025.03.20 11:10, Phillip Wood wrote: > Hi Josh > > On 19/03/2025 22:23, Josh Steadmon wrote: > > On 2025.03.18 16:24, Josh Steadmon wrote: > > > > I'm currently working on an alternate solution where we build the object > > files in Cargo's working directory, rather than in the source tree. This > > will (mostly) avoid the need to clean after the build. I hope to send a > > v2 with this change in the next few days. > > Meson has builtin support for out-of-tree builds which would make building > in Cargo's OUT_DIR trivial. Our meson build is still experimental though so > you may not want to rely on it. > > Best Wishes > > Phillip Thanks for the pointer! I have a working Makefile solution which I'll be sending out soon, but I'll keep meson in mind as a backup if the list doesn't like my V2.
diff --git a/Makefile b/Makefile index 52eed88dde..e7d8786e4e 100644 --- a/Makefile +++ b/Makefile @@ -420,6 +420,10 @@ include shared.mak # Define INCLUDE_LIBGIT_RS if you want `make all` and `make test` to build and # test the Rust crates in contrib/libgit-sys and contrib/libgit-rs. # +# Define PRESERVE_LIBGIT_TARGET to prevent `make clean` from removing the Cargo +# output directories for libgit-sys and libgit-rs. This is mainly for use in the +# Cargo build scripts. +# # === Optional library: libintl === # # Define NO_GETTEXT if you don't want Git output to be translated. @@ -3761,7 +3765,9 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(htmldocs).tar.gz $(manpages).tar.gz $(MAKE) -C Documentation/ clean $(RM) Documentation/GIT-EXCLUDED-PROGRAMS +ifndef PRESERVE_LIBGIT_TARGET $(RM) -r contrib/libgit-sys/target contrib/libgit-rs/target +endif $(RM) contrib/libgitpub/partial_symbol_export.o $(RM) contrib/libgitpub/hidden_symbol_export.o $(RM) contrib/libgitpub/libgitpub.a diff --git a/contrib/libgit-sys/build.rs b/contrib/libgit-sys/build.rs index e0d979c196..9d586d272d 100644 --- a/contrib/libgit-sys/build.rs +++ b/contrib/libgit-sys/build.rs @@ -6,7 +6,7 @@ pub fn main() -> std::io::Result<()> { 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("git-src"); let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap()); let make_output = make_cmd::gnu_make() @@ -31,5 +31,24 @@ pub fn main() -> std::io::Result<()> { println!("cargo:rustc-link-lib=gitpub"); println!("cargo:rerun-if-changed={}", git_root.display()); + let make_output = make_cmd::gnu_make() + .env("DEVELOPER", "1") + .env_remove("PROFILE") + .current_dir(git_root.clone()) + .args([ + "INCLUDE_LIBGIT_RS=YesPlease", + "PRESERVE_LIBGIT_TARGET=YesPlease", + "clean", + ]) + .output() + .expect("`make clean` failed to run"); + if !make_output.status.success() { + panic!( + "`make clean` failed:\n stdout = {}\n stderr = {}\n", + String::from_utf8(make_output.stdout).unwrap(), + String::from_utf8(make_output.stderr).unwrap() + ); + } + Ok(()) } diff --git a/contrib/libgit-sys/git-src b/contrib/libgit-sys/git-src new file mode 120000 index 0000000000..c25bddb6dd --- /dev/null +++ b/contrib/libgit-sys/git-src @@ -0,0 +1 @@ +../.. \ No newline at end of file
Unlike `cargo build`, `cargo package` does not get access to the entire Git repo containing a Rust crate. Instead, it prepares a directory starting from the crate root (potentially excluding files, such as those not under version control, or explicity excluded in the Cargo.toml file). This means that the current method of building the libgit-sys crate does not work with `cargo package`, as it tries to execute the Makefile from "../.." relative to the crate root. Fix this by adding a `git-src` symlink in the crate that points to the Git repository root. `cargo package` will flatten this to a copy of the Git repo, excluding non-version-controlled files, any explicitly-excluded files, and trees that contain a Cargo.toml file (this prevents infinite recursion on the symlink). We can then execute the Makefile under the flattened git-src directory from our build.rs script. However, this exposes a second problem; Cargo will check that the build script does not add, delete, or modify any source files. This means that after we copy our libgitpub.a dependency to the output directory, we must run `make clean` to remove the object files we created during the build process. Unfortunately, there is not a way to determine from the build.rs script whether we're running `cargo build` vs. `cargo package`, so now any build of the libgit-sys crate will result in cleaning the Git worktree. A potential alternative is to make an additional temporary copy of the worktree and run the Makefile there. This would avoid removing build artifacts in the worktree at the cost of copying MBs worth of source files to a temporary directory. Perhaps hardlinking instead of making a full copy would help here, but that might be less portable. Signed-off-by: Josh Steadmon <steadmon@google.com> --- Makefile | 6 ++++++ contrib/libgit-sys/build.rs | 21 ++++++++++++++++++++- contrib/libgit-sys/git-src | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) create mode 120000 contrib/libgit-sys/git-src