Skip to contentSkip to conversation

cmake: fix build of `t-oidtree`

PATCH In review Johannes Schindelin via GitGitGadget 4 participants 6 messages Jul 12, 2024 – Jul 24, 2024

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cmake: fix build of `t-oidtree`

From: Johannes Schindelin <[email protected]>

When the oidtree test helper was turned into a unit test, a new lib-oid source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08).

---
cmake: fix build of t-oidtree

This is based on the gt/unit-test-oidtree branch.

/cc "Ghanshyam Thakkar" [email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1761%2Fdscho%2Fcmake-vs-t-oidtree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1761/dscho/cmake-vs-t-oidtree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1761

 contrib/buildsystems/CMakeLists.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
contrib/buildsystems/CMakeLists.txt +2 -1
index 2f9c33585c6..832f46b316b 100644
@@ -976,11 +976,12 @@ list(TRANSFORM test-reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 
 #unit-tests
 add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)
+add_library(unit-test-lib-oid OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/lib-oid.c)
 
 parse_makefile_for_scripts(unit_test_PROGRAMS "UNIT_TEST_PROGRAMS" "")
 foreach(unit_test ${unit_test_PROGRAMS})
 	add_executable("${unit_test}" "${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c")
-	target_link_libraries("${unit_test}" unit-test-lib common-main)
+	target_link_libraries("${unit_test}" unit-test-lib unit-test-lib-oid common-main)
 	set_target_properties("${unit_test}"
 		PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin)
 	if(MSVC)

base-commit: ed548408723d6e969160279398cc47f88f5700bc

Signature · 1 line
gitgitgadget
#1 lore raw

Hi,

now Cc:ing Ghanshyam correctly. Sorry for that.

Quoted 47 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When the oidtree test helper was turned into a unit test, a new lib-oid source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08).

---
cmake: fix build of t-oidtree

This is based on the gt/unit-test-oidtree branch.

/cc "Ghanshyam Thakkar" [email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1761%2Fdscho%2Fcmake-vs-t-oidtree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1761/dscho/cmake-vs-t-oidtree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1761

 contrib/buildsystems/CMakeLists.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
contrib/buildsystems/CMakeLists.txt +2 -1
index 2f9c33585c6..832f46b316b 100644
@@ -976,11 +976,12 @@ list(TRANSFORM test-reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 
 #unit-tests
 add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)
+add_library(unit-test-lib-oid OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/lib-oid.c)
 
 parse_makefile_for_scripts(unit_test_PROGRAMS "UNIT_TEST_PROGRAMS" "")
 foreach(unit_test ${unit_test_PROGRAMS})
 	add_executable("${unit_test}" "${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c")
-	target_link_libraries("${unit_test}" unit-test-lib common-main)
+	target_link_libraries("${unit_test}" unit-test-lib unit-test-lib-oid common-main)
 	set_target_properties("${unit_test}"
 		PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin)
 	if(MSVC)

base-commit: ed548408723d6e969160279398cc47f88f5700bc --
gitgitgadget

#2 lore raw
In reply to Johannes Schindelin via GitGitGadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 9 lines · writes:

From: Johannes Schindelin <[email protected]>

When the oidtree test helper was turned into a unit test, a new lib-oid source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08).

Quoted 7 lines

---
cmake: fix build of t-oidtree

This is based on the gt/unit-test-oidtree branch.

/cc "Ghanshyam Thakkar" [email protected]

The base topic has been merged last month during this cycle, so the fix must be merged down to the 'master' before the release. Will queue.

Thanks.

#3 lore raw
Quoted 16 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 13 lines · writes:

From: Johannes Schindelin <[email protected]>

When the oidtree test helper was turned into a unit test, a new lib-oid source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08).

---
cmake: fix build of t-oidtree

Quoted 7 lines
Quoted 3 lines

This is based on the gt/unit-test-oidtree branch.

/cc "Ghanshyam Thakkar" [email protected]

The base topic has been merged last month during this cycle, so the fix must be merged down to the 'master' before the release. Will queue.

Changes look correct to me. I was not aware of the existance of CMakeLists in the tree cause there was no mention of CMake support in the docs about building git from source, but it was my mistake for grepping for 'test-lib' in just the Makefile instead of project wide. Apologies for the negligance and thanks for the patch.

#4 lore raw
Quoted 5 lines · Ghanshyam Thakkar

Changes look correct to me. I was not aware of the existance of CMakeLists in the tree cause there was no mention of CMake support in the docs about building git from source, but it was my mistake for grepping for 'test-lib' in just the Makefile instead of project wide. Apologies for the negligance and thanks for the patch.

No need to apologize. Nobody caught during the review, so it is not even your fault ;-)

And the CMakeLists is not even meant to work with anything but for Windows, IIRC (even though it might work by accident).

#5 lore raw
In reply to Ghanshyam Thakkar

Hi Ghanshyam,

Quoted 19 lines · Ghanshyam Thakkar
Quoted 18 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 15 lines · writes:

From: Johannes Schindelin <[email protected]>

When the oidtree test helper was turned into a unit test, a new lib-oid source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08).

---
cmake: fix build of t-oidtree

This is based on the gt/unit-test-oidtree branch.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

/cc "Ghanshyam Thakkar" [email protected]

The base topic has been merged last month during this cycle, so the fix must be merged down to the 'master' before the release. Will queue.

Changes look correct to me.

Thank you for confirming.

For what it's worth, it was this confirmation by the domain expert (i.e. you) that I was looking for when I Cc:ed you, not to spread blame.

The CMakeLists.txt file exists primarily to support building Git in Visual Studio, and I removed that part from CI builds specifically so that contributors on the Git mailing list need not worry about that support.

Ciao,
Johannes