Makefile 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

gitgitgadget

From: Ezekiel Newren <[email protected]>

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.

---
 Makefile | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

index 92fd8d86d8..e8fad803be 100644
@@ -1390,6 +1390,30 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
 
 UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
 
+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
+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
+
 # 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)
 EXTLIBS =

@@ -2723,30 +2747,6 @@ 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
-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

gitgitgadget

From: Ezekiel Newren <[email protected]>

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.

---
 Makefile | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

index e8fad803be..d89ba03286 100644
@@ -918,7 +918,6 @@ export PYTHON_PATH
 TEST_SHELL_PATH = $(SHELL_PATH)
 
 LIB_FILE = libgit.a
-XDIFF_LIB = xdiff/lib.a
 REFTABLE_LIB = reftable/libreftable.a
 
 GENERATED_H += command-list.h

@@ -1397,8 +1396,7 @@ 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)
 
 REFTABLE_OBJS += reftable/basics.o
 REFTABLE_OBJS += reftable/error.o

@@ -1414,8 +1412,8 @@ REFTABLE_OBJS += reftable/table.o
 REFTABLE_OBJS += reftable/tree.o
 REFTABLE_OBJS += reftable/writer.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)
+# reftable lib may in turn depend on what is in libgit.a
+GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE)
 EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)

@@ -2767,7 +2765,6 @@ OBJECTS += $(GIT_OBJS)
 OBJECTS += $(SCALAR_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
-OBJECTS += $(XDIFF_OBJS)
 OBJECTS += $(FUZZ_OBJS)
 OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
 OBJECTS += $(UNIT_TEST_OBJS)

@@ -2921,9 +2918,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
-$(XDIFF_LIB): $(XDIFF_OBJS)
-	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
-
 $(REFTABLE_LIB): $(REFTABLE_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 

@@ -3765,7 +3759,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) git.rc git.res
 	$(RM) $(OBJECTS)
 	$(RM) headless-git.o
-	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB)
+	$(RM) $(LIB_FILE) $(REFTABLE_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)

@@ -3959,7 +3953,6 @@ endif
 LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o
 LIBGIT_PUB_OBJS += libgit.a
 LIBGIT_PUB_OBJS += reftable/libreftable.a
-LIBGIT_PUB_OBJS += xdiff/lib.a
 
 LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o
 

gitgitgadget

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.

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

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?

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.

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

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 <[email protected]>

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

---
 Makefile | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

index d89ba03286..4c63045443 100644
@@ -918,7 +918,6 @@ export PYTHON_PATH
 TEST_SHELL_PATH = $(SHELL_PATH)
 
 LIB_FILE = libgit.a
-REFTABLE_LIB = reftable/libreftable.a
 
 GENERATED_H += command-list.h
 GENERATED_H += config-list.h

@@ -1411,9 +1410,9 @@ REFTABLE_OBJS += reftable/system.o
 REFTABLE_OBJS += reftable/table.o
 REFTABLE_OBJS += reftable/tree.o
 REFTABLE_OBJS += reftable/writer.o
+LIB_OBJS += $(REFTABLE_OBJS)
 
-# reftable lib may in turn depend on what is in libgit.a
-GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE)
+GITLIBS = common-main.o $(LIB_FILE)
 EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)

@@ -2766,7 +2765,7 @@ OBJECTS += $(SCALAR_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
 OBJECTS += $(FUZZ_OBJS)
-OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
+OBJECTS += $(REFTABLE_TEST_OBJS)
 OBJECTS += $(UNIT_TEST_OBJS)
 OBJECTS += $(CLAR_TEST_OBJS)
 OBJECTS += $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))

@@ -2918,9 +2917,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
-$(REFTABLE_LIB): $(REFTABLE_OBJS)
-	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
-
 export DEFAULT_EDITOR DEFAULT_PAGER
 
 Documentation/GIT-EXCLUDED-PROGRAMS: FORCE

@@ -3759,7 +3755,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) git.rc git.res
 	$(RM) $(OBJECTS)
 	$(RM) headless-git.o
-	$(RM) $(LIB_FILE) $(REFTABLE_LIB)
+	$(RM) $(LIB_FILE)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)

@@ -3952,7 +3948,6 @@ endif
 
 LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o
 LIBGIT_PUB_OBJS += libgit.a
-LIBGIT_PUB_OBJS += reftable/libreftable.a
 
 LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o
 

gitgitgadget

Same answer as before.

Thanks.

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.

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.

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.

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.

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: Junio C Hamano <[email protected]>

## 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: Junio C Hamano <[email protected]>

## 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: Junio C Hamano <[email protected]>

## 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 <[email protected]>
      
       ## 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)

gitgitgadget

From: Ezekiel Newren <[email protected]>

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.

---
 Makefile | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

index 92fd8d86d8..d37edd3d03 100644
@@ -918,7 +918,6 @@ export PYTHON_PATH
 TEST_SHELL_PATH = $(SHELL_PATH)
 
 LIB_FILE = libgit.a
-XDIFF_LIB = xdiff/lib.a
 REFTABLE_LIB = reftable/libreftable.a
 
 GENERATED_H += command-list.h

@@ -1209,6 +1208,13 @@ 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
 
 BUILTIN_OBJS += builtin/add.o
 BUILTIN_OBJS += builtin/am.o

@@ -1390,8 +1396,8 @@ 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)
+# reftable lib may in turn depend on what is in libgit.a
+GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE)
 EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)

@@ -2723,16 +2729,6 @@ 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

@@ -2767,7 +2763,6 @@ OBJECTS += $(GIT_OBJS)
 OBJECTS += $(SCALAR_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
-OBJECTS += $(XDIFF_OBJS)
 OBJECTS += $(FUZZ_OBJS)
 OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
 OBJECTS += $(UNIT_TEST_OBJS)

@@ -2921,9 +2916,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
-$(XDIFF_LIB): $(XDIFF_OBJS)
-	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
-
 $(REFTABLE_LIB): $(REFTABLE_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 

@@ -3765,7 +3757,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) git.rc git.res
 	$(RM) $(OBJECTS)
 	$(RM) headless-git.o
-	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB)
+	$(RM) $(LIB_FILE) $(REFTABLE_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)

@@ -3959,7 +3951,6 @@ endif
 LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o
 LIBGIT_PUB_OBJS += libgit.a
 LIBGIT_PUB_OBJS += reftable/libreftable.a
-LIBGIT_PUB_OBJS += xdiff/lib.a
 
 LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o
 

gitgitgadget

From: Ezekiel Newren <[email protected]>

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

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

index d37edd3d03..6464ded3a3 100644
@@ -918,7 +918,6 @@ export PYTHON_PATH
 TEST_SHELL_PATH = $(SHELL_PATH)
 
 LIB_FILE = libgit.a
-REFTABLE_LIB = reftable/libreftable.a
 
 GENERATED_H += command-list.h
 GENERATED_H += config-list.h

@@ -1136,6 +1135,19 @@ 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

@@ -1396,8 +1408,7 @@ 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)
+GITLIBS = common-main.o $(LIB_FILE)
 EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)

@@ -2729,20 +2740,6 @@ 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

@@ -2764,7 +2761,7 @@ OBJECTS += $(SCALAR_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
 OBJECTS += $(FUZZ_OBJS)
-OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
+OBJECTS += $(REFTABLE_TEST_OBJS)
 OBJECTS += $(UNIT_TEST_OBJS)
 OBJECTS += $(CLAR_TEST_OBJS)
 OBJECTS += $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))

@@ -2916,9 +2913,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
-$(REFTABLE_LIB): $(REFTABLE_OBJS)
-	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
-
 export DEFAULT_EDITOR DEFAULT_PAGER
 
 Documentation/GIT-EXCLUDED-PROGRAMS: FORCE

@@ -3757,7 +3751,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) git.rc git.res
 	$(RM) $(OBJECTS)
 	$(RM) headless-git.o
-	$(RM) $(LIB_FILE) $(REFTABLE_LIB)
+	$(RM) $(LIB_FILE)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)

@@ -3950,7 +3944,6 @@ endif
 
 LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o
 LIBGIT_PUB_OBJS += libgit.a
-LIBGIT_PUB_OBJS += reftable/libreftable.a
 
 LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o
 

gitgitgadget

Both changes look sensible. Will queue. Thanks.

Agreed, this version looks good to me. Thanks!

Patrick