patch, 3 partsMakefile update libgit.a: Include xdiff and reftable in libgit.a

Add xdiff and reftable to the static library libgit.a that Makefile produces. Meson does not require any changes since it already includes those libraries. The motivation is to simplify Rust's job of linking against the C code by requiring it to only link against a single static library (libgit.a).

The Rust compiler only needs to know how to link against libgit.a in 2 cases that I can think of:

 * Rust unit tests
 * Rust defining the main function

Otherwise Rust can be compiled without linking, and then Makefile and Meson can use Cargo's produced static lib files to build Git.

Note: The flag -fPIE or -fPIC is required for Makefile to build libgit.a in
a way that Cargo can use. It has been deliberately omitted from the
Makefile, for now, since Rust isn't part of Git (yet).

Ezekiel Newren (3):
  make: move xdiff and reftable objects before GITLIBS
  make: delete XDIFF_LIB, add xdiff to LIB_OBJS
  make: delete REFTABLE_LIB, add reftable to LIB_OBJS

 Makefile | 66 +++++++++++++++++++++++---------------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

base-commit: a91ca5db0318b6fda5a6721ee843f56e7e2fadfc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2065%2Fezekielnewren%2Fmerge_xdiff_and_reftable_with_libgit-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2065/ezekielnewren/merge_xdiff_and_reftable_with_libgit-v1
Pull-Request: https://github.com/git/git/pull/2065

From: Ezekiel Newren <ezekielnewren@gmail.com>

XDIFF_OBJS and REFTABLE_OBJS will be added to LIB_OBJS in later commits. Move them here so that GIT_OBJS += $(LIB_OBJS) works correctly. View with --color-moved.

Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
---
 Makefile | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

From: Ezekiel Newren <ezekielnewren@gmail.com>

In a future patch series the 'xdiff' Rust crate will be added. Delete the creation of the static library file for xdiff to avoid a name conflict. This also moves toward the goal of Rust only needing to link against libgit.a.

Changes to Meson are not required as the xdiff library is already included in Meson's libgit.a.

Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
---
 Makefile | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

On Wed, Oct 01, 2025 at 06:02:27PM +0000, Ezekiel Newren via GitGitGadget wrote:

The removal of the `xdiff-objs` target isn't mentioned or justified in the commit message. I personally don't mind that this target goes away, as I don't really have a use case for it anyway. But in theory it could continue to exist. So I'd either retain it, or explain why it goes away.

In case it goes away, is there still a reason to have the separate XDIFF_OBJS variable? Can't we add these objects to `LIB_OBJS` directly?

Patrick

Patrick Steinhardt <ps@pks.im> writes:

Doing it this way lets us still keep the "logical" organization to tell which object is which, even though we may lose physical distinction by throwing all objects in a single library archive.

Incidentally this would slightly reduce the patch noise and make the result more merge friendly when other topics are in flight that touch these (like adding a new file or two to REFTABLE_OBJS), but with the movement of these lines in [1/3], that benefit is diminished.

On Thu, Oct 02, 2025 at 06:31:33AM -0700, Junio C Hamano wrote:

Well, I guess the logical organization still exists due to all the files living in "xdiff/" and "reftable/", respectively. So I'm not sure that's a definitive win.

But in any case, I don't have any strong feelings here. I mostly wondered whether we can simplify the build infra even further.

Patrick

On Thu, Oct 2, 2025 at 9:33 AM Patrick Steinhardt <ps@pks.im> wrote:

My preference is the same as yours Patrick. In my Introduce Rust v2 series (that I dropped) I did it the way that you described. I changed how I did things because of Junio's suggestion. I think doing it Patrick's way would be more consistent because in Meson the `libgit_sources` variable includes all C files that are part of libgit. That variable includes the sources for reftable and xdiff.

snippet from meson.build:
libgit_sources = [
  ...
  'reftable/basics.c',
  'reftable/error.c',
  'reftable/block.c',
  'reftable/blocksource.c',
  'reftable/iter.c',
  'reftable/merged.c',
  'reftable/pq.c',
  'reftable/record.c',
  'reftable/stack.c',
  'reftable/system.c',
  'reftable/table.c',
  'reftable/tree.c',
  'reftable/writer.c',
  ...
  'xdiff/xdiffi.c',
  'xdiff/xemit.c',
  'xdiff/xhistogram.c',
  'xdiff/xmerge.c',
  'xdiff/xpatience.c',
  'xdiff/xprepare.c',
  'xdiff/xutils.c',
]

I will go with your preference Junio. Do you prefer your way or Patrick's way?

Ezekiel Newren <ezekielnewren@gmail.com> writes:

With step [1/3] shuffling lines around, the advantage of keeping XDIFF_OBJS and REFTABLE_OBJS to make the result merge-friendly disappeared, so I am perfectly fine to throw everything into LIB_OBJS.

Thanks.

On Thu, Oct 2, 2025 at 1:01 PM Junio C Hamano <gitster@pobox.com> wrote:

Sounds good. I'll do that in my next version.

On Wed, Oct 1, 2025 at 11:47 PM Patrick Steinhardt <ps@pks.im> wrote:

If I understand correctly, the PHONY target xdiff-objs is to make sure that xdiff/lib.a is always built. But if it's folded into libgit.a then its purpose becomes moot. I will update my commit message to make this clear.

From: Ezekiel Newren <ezekielnewren@gmail.com>

Same idea as the previous commit except that I don't know when or if reftable will be turned into a Rust crate.

Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
---
 Makefile | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

On Wed, Oct 01, 2025 at 06:02:28PM +0000, Ezekiel Newren via GitGitGadget wrote:

Same question here as on the preceding commit: do we even need REFTABLE_OBJS anymore?

Other than that these patches look sensible to me, thanks. Even without Rust they simplify our build infra a bit, so I think that landing them independently of Rust is a good thing.

Thanks!

Patrick

Patrick Steinhardt <ps@pks.im> writes:

Same answer as before.

Thanks.

On Wed, Oct 1, 2025 at 11:49 PM Patrick Steinhardt <ps@pks.im> wrote:

I agree. I've been trying to find and fix things that don't depend on Rust, but will need to be changed to make the adoption of Rust smoother.

Thanks for your feedback.

"Ezekiel Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

It is a bit unclear why two (or three) are so much more hassle than one, but OK. Allowing both build systems to agree on the same set of artifacts is very much desirable, and if meson based build rolls everythning into a single library archive, the the other one should do the same.

Of course we could run "ar" ourselves and combine the three into a single library archive, but as an approach, what you have here is a perfectly fine, and more preferrable, way to achieve the goal of ending up with a single archive file.

This topic, however, especially its first step, had caused rather unpleasant textual conflicts when merged to 'seen' (I didn't check which other topic was the most heavily conflicting, though). I may attempt to get a clean merge again tomorrow, but due to time pressure, tonight's 'seen' was done without these patches merged.

Thanks.

On Wed, Oct 1, 2025 at 5:32 PM Junio C Hamano <gitster@pobox.com> wrote:

In order for Cargo to tell the Rust compiler how to link against the C
archives they need to be specified in the build.rs file for each
crate. That would look something like this:
tree gitcore/
gitcore/
├── build.rs
├── Cargo.toml
└── src
    ├── do_that.rs
    ├── do_this.rs
    └── lib.rs

Where gitcore/build.rs would look something like this:
fn main() {
    ...
    println!("cargo:rustc-link-search=native={}", git_build_dir);
    println!("cargo:rustc-link-lib=static=git");
    if let Some(built_with_makefile) = std::env::var("BUILT_WITH_MAKEFILE") {
        println!("cargo:rustc-link-search=native={}",
git_build_dir.join("xdiff"));
        println!("cargo:rustc-link-lib=static=xdiff");
        println!("cargo:rustc-link-search=native={}",
git_build_dir.join("reftable"));
        println!("cargo:rustc-link-lib=static=reftable");
    }
}

But `cargo build` is invoked by a shell script by both Makefile and Meson, and build.rs would need to have an environment variable set. Something like BUILT_WITH_MAKEFILE for only when build_rust.sh is invoked by makefile and then you'd have to remember to update both Makefile and every build.rs if the library files produced by Makefile are ever changed. It seems a lot cleaner to hardcode just libgit.a and leave it up to the build systems to ensure that everything inside that static library contains everything that is needed.

I wonder if Patrick's approach would cause fewer merge conflicts. If I add xdiff and reftable objects to LIB_OBJS directly then I don't need to bother with the move commit.

Junio C Hamano <gitster@pobox.com> writes:

I think I have sorted it out. I'll push out 'seen' with this topic at the tip, so could you please check the resulting Makefile for any funny mismerges?

Thanks.

On Thu, Oct 2, 2025 at 3:02 PM Junio C Hamano <gitster@pobox.com> wrote:

Looks like it merged correctly. I'd like to release a v2 where objects are directly added to LIB_OBJS.

Here is what I ran to compare it to what I created:
  git rev-parse next
  a91ca5db0318b6fda5a6721ee843f56e7e2fadfc

  git range-diff d30e0f3024^1..d30e0f3024^2
next..merge_xdiff_and_reftable_with_libgit_v1
  1:  40d798afad ! 1:  fdcf5a0de8 make: move xdiff and reftable
objects before GITLIBS
      @@ Commit message
           with --color-moved.

           Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>

        ## Makefile ##
       @@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
  2:  22d1910ec8 ! 2:  28e7fd27b6 make: delete XDIFF_LIB, add xdiff to LIB_OBJS
      @@ Commit message
           included in Meson's libgit.a.

           Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>

        ## Makefile ##
       @@ Makefile: export PYTHON_PATH
  3:  9f1670c048 ! 3:  8549f63415 make: delete REFTABLE_LIB, add
reftable to LIB_OBJS
      @@ Commit message
           reftable will be turned into a Rust crate.

           Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>

        ## Makefile ##
       @@ Makefile: export PYTHON_PATH

Changes in v2:

 * Add xdiff and reftable objects directly to LIB_OBJS.
 * Explain why xdiff-objs is removed.

Original cover letter: ======================

Add xdiff and reftable to the static library libgit.a that Makefile produces. Meson does not require any changes since it already includes those libraries. The motivation is to simplify Rust's job of linking against the C code by requiring it to only link against a single static library (libgit.a).

The Rust compiler only needs to know how to link against libgit.a in 2 cases that I can think of:

 * Rust unit tests
 * Rust defining the main function

Otherwise Rust can be compiled without linking, and then Makefile and Meson can use Cargo's produced static lib files to build Git.

Note: The flag -fPIE or -fPIC is required for Makefile to build libgit.a in
a way that Cargo can use. It has been deliberately omitted from the
Makefile, for now, since Rust isn't part of Git (yet).

Ezekiel Newren (2):
  make: delete XDIFF_LIB, add xdiff to LIB_OBJS
  make: delete REFTABLE_LIB, add reftable to LIB_OBJS

 Makefile | 62 +++++++++++++++++++++-----------------------------------
 1 file changed, 23 insertions(+), 39 deletions(-)

base-commit: a91ca5db0318b6fda5a6721ee843f56e7e2fadfc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2065%2Fezekielnewren%2Fmerge_xdiff_and_reftable_with_libgit-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2065/ezekielnewren/merge_xdiff_and_reftable_with_libgit-v2
Pull-Request: https://github.com/git/git/pull/2065

Range-diff vs v1:

 1:  fdcf5a0de8 < -:  ---------- make: move xdiff and reftable objects before GITLIBS
 2:  28e7fd27b6 ! 1:  b535a456ae make: delete XDIFF_LIB, add xdiff to LIB_OBJS
     @@ Commit message
          Changes to Meson are not required as the xdiff library is already
          included in Meson's libgit.a.
      
     +    xdiff-objs was a historical make target to allow building just the
     +    objects in xdiff. Since it was defined in terms of XDIFF_OBJS (which
     +    no longer exists) this convenience make target no longer makes sense.
     +    Remove it.
     +
          Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
      
       ## Makefile ##
     @@ Makefile: export PYTHON_PATH
       REFTABLE_LIB = reftable/libreftable.a
       
       GENERATED_H += command-list.h
     -@@ Makefile: XDIFF_OBJS += xdiff/xmerge.o
     - XDIFF_OBJS += xdiff/xpatience.o
     - XDIFF_OBJS += xdiff/xprepare.o
     - XDIFF_OBJS += xdiff/xutils.o
     --.PHONY: xdiff-objs
     --xdiff-objs: $(XDIFF_OBJS)
     -+LIB_OBJS += $(XDIFF_OBJS)
     +@@ Makefile: LIB_OBJS += write-or-die.o
     + LIB_OBJS += ws.o
     + LIB_OBJS += wt-status.o
     + LIB_OBJS += xdiff-interface.o
     ++LIB_OBJS += xdiff/xdiffi.o
     ++LIB_OBJS += xdiff/xemit.o
     ++LIB_OBJS += xdiff/xhistogram.o
     ++LIB_OBJS += xdiff/xmerge.o
     ++LIB_OBJS += xdiff/xpatience.o
     ++LIB_OBJS += xdiff/xprepare.o
     ++LIB_OBJS += xdiff/xutils.o
       
     - REFTABLE_OBJS += reftable/basics.o
     - REFTABLE_OBJS += reftable/error.o
     -@@ Makefile: REFTABLE_OBJS += reftable/table.o
     - REFTABLE_OBJS += reftable/tree.o
     - REFTABLE_OBJS += reftable/writer.o
     + BUILTIN_OBJS += builtin/add.o
     + BUILTIN_OBJS += builtin/am.o
     +@@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
     + 
     + UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
       
      -# xdiff and reftable libs may in turn depend on what is in libgit.a
      -GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
     @@ Makefile: REFTABLE_OBJS += reftable/table.o
       EXTLIBS =
       
       GIT_USER_AGENT = git/$(GIT_VERSION)
     +@@ Makefile: reconfigure config.mak.autogen: config.status
     + .PHONY: reconfigure # This is a convenience target.
     + endif
     + 
     +-XDIFF_OBJS += xdiff/xdiffi.o
     +-XDIFF_OBJS += xdiff/xemit.o
     +-XDIFF_OBJS += xdiff/xhistogram.o
     +-XDIFF_OBJS += xdiff/xmerge.o
     +-XDIFF_OBJS += xdiff/xpatience.o
     +-XDIFF_OBJS += xdiff/xprepare.o
     +-XDIFF_OBJS += xdiff/xutils.o
     +-.PHONY: xdiff-objs
     +-xdiff-objs: $(XDIFF_OBJS)
     +-
     + REFTABLE_OBJS += reftable/basics.o
     + REFTABLE_OBJS += reftable/error.o
     + REFTABLE_OBJS += reftable/block.o
      @@ Makefile: OBJECTS += $(GIT_OBJS)
       OBJECTS += $(SCALAR_OBJS)
       OBJECTS += $(PROGRAM_OBJS)
 3:  8549f63415 ! 2:  9031610dc8 make: delete REFTABLE_LIB, add reftable to LIB_OBJS
     @@ Makefile: export PYTHON_PATH
       
       GENERATED_H += command-list.h
       GENERATED_H += config-list.h
     -@@ Makefile: REFTABLE_OBJS += reftable/system.o
     - REFTABLE_OBJS += reftable/table.o
     - REFTABLE_OBJS += reftable/tree.o
     - REFTABLE_OBJS += reftable/writer.o
     -+LIB_OBJS += $(REFTABLE_OBJS)
     +@@ Makefile: LIB_OBJS += refs/iterator.o
     + LIB_OBJS += refs/packed-backend.o
     + LIB_OBJS += refs/ref-cache.o
     + LIB_OBJS += refspec.o
     ++LIB_OBJS += reftable/basics.o
     ++LIB_OBJS += reftable/error.o
     ++LIB_OBJS += reftable/block.o
     ++LIB_OBJS += reftable/blocksource.o
     ++LIB_OBJS += reftable/iter.o
     ++LIB_OBJS += reftable/merged.o
     ++LIB_OBJS += reftable/pq.o
     ++LIB_OBJS += reftable/record.o
     ++LIB_OBJS += reftable/stack.o
     ++LIB_OBJS += reftable/system.o
     ++LIB_OBJS += reftable/table.o
     ++LIB_OBJS += reftable/tree.o
     ++LIB_OBJS += reftable/writer.o
     + LIB_OBJS += remote.o
     + LIB_OBJS += replace-object.o
     + LIB_OBJS += repo-settings.o
     +@@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
     + 
     + UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
       
      -# reftable lib may in turn depend on what is in libgit.a
      -GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE)
     @@ Makefile: REFTABLE_OBJS += reftable/system.o
       EXTLIBS =
       
       GIT_USER_AGENT = git/$(GIT_VERSION)
     +@@ Makefile: reconfigure config.mak.autogen: config.status
     + .PHONY: reconfigure # This is a convenience target.
     + endif
     + 
     +-REFTABLE_OBJS += reftable/basics.o
     +-REFTABLE_OBJS += reftable/error.o
     +-REFTABLE_OBJS += reftable/block.o
     +-REFTABLE_OBJS += reftable/blocksource.o
     +-REFTABLE_OBJS += reftable/iter.o
     +-REFTABLE_OBJS += reftable/merged.o
     +-REFTABLE_OBJS += reftable/pq.o
     +-REFTABLE_OBJS += reftable/record.o
     +-REFTABLE_OBJS += reftable/stack.o
     +-REFTABLE_OBJS += reftable/system.o
     +-REFTABLE_OBJS += reftable/table.o
     +-REFTABLE_OBJS += reftable/tree.o
     +-REFTABLE_OBJS += reftable/writer.o
     +-
     + TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
     + 
     + .PHONY: test-objs
      @@ Makefile: OBJECTS += $(SCALAR_OBJS)
       OBJECTS += $(PROGRAM_OBJS)
       OBJECTS += $(TEST_OBJS)

From: Ezekiel Newren <ezekielnewren@gmail.com>

In a future patch series the 'xdiff' Rust crate will be added. Delete the creation of the static library file for xdiff to avoid a name conflict. This also moves toward the goal of Rust only needing to link against libgit.a.

Changes to Meson are not required as the xdiff library is already included in Meson's libgit.a.

xdiff-objs was a historical make target to allow building just the objects in xdiff. Since it was defined in terms of XDIFF_OBJS (which no longer exists) this convenience make target no longer makes sense. Remove it.

Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
---
 Makefile | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

From: Ezekiel Newren <ezekielnewren@gmail.com>

Same idea as the previous commit except that I don't know when or if reftable will be turned into a Rust crate.

Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
---
 Makefile | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

"Ezekiel Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Both changes look sensible.  Will queue.  Thanks.

On Fri, Oct 03, 2025 at 09:43:09AM -0700, Junio C Hamano wrote:

Agreed, this version looks good to me. Thanks!

Patrick