threads / patch / 61772

patchcmake: fix build of `t-oidtree`

Subject: [PATCH] cmake: fix build of `t-oidtree`

## tl;dr

6 messages between Jul 12, 2024 and Jul 24, 2024. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Johannes Schindelin via GitGitGadget· Jul 12, 2024, 20:34 UTC · lore
From: Johannes Schindelin <johannes.schindelin@gmx.de>

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).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    cmake: fix build of t-oidtree
    
    This is based on the gt/unit-test-oidtree branch.
    
    /cc "Ghanshyam Thakkar" shyamthakkar001@gmail.com
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(-)
Show changes to contrib/buildsystems/CMakeLists.txt +2 −1
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 2f9c33585c6..832f46b316b 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -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
Johannes Schindelin· Jul 12, 2024, 20:46 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH] cmake: fix build of `t-oidtree`

Hi,
now Cc:ing Ghanshyam correctly. Sorry for that.
On Fri, 12 Jul 2024, Johannes Schindelin via GitGitGadget wrote:
Show 47 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     cmake: fix build of t-oidtree
>
>     This is based on the gt/unit-test-oidtree branch.
>
>     /cc "Ghanshyam Thakkar" shyamthakkar001@gmail.com
>
> 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(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 2f9c33585c6..832f46b316b 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -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
>
Junio C Hamano· Jul 12, 2024, 21:38 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH] cmake: fix build of `t-oidtree`

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 16 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     cmake: fix build of t-oidtree
>     
>     This is based on the gt/unit-test-oidtree branch.
>     
>     /cc "Ghanshyam Thakkar" shyamthakkar001@gmail.com

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.
Ghanshyam Thakkar· Jul 12, 2024, 23:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] cmake: fix build of `t-oidtree`

Junio C Hamano <gitster@pobox.com> wrote:
Show 23 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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).
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >     cmake: fix build of t-oidtree
> >     
> >     This is based on the gt/unit-test-oidtree branch.
> >     
> >     /cc "Ghanshyam Thakkar" shyamthakkar001@gmail.com
>
> 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.

Junio C Hamano· Jul 13, 2024, 01:14 UTC · re: Ghanshyam Thakkar · lore

Re: [PATCH] cmake: fix build of `t-oidtree`

"Ghanshyam Thakkar" <shyamthakkar001@gmail.com> writes:
Show 5 quoted lines
> 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).

Johannes Schindelin· Jul 24, 2024, 10:19 UTC · re: Ghanshyam Thakkar · lore

Re: [PATCH] cmake: fix build of `t-oidtree`

Hi Ghanshyam,
On Sat, 13 Jul 2024, Ghanshyam Thakkar wrote:
Show 26 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > > 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).
> > >
> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > ---
> > >     cmake: fix build of t-oidtree
> > >
> > >     This is based on the gt/unit-test-oidtree branch.
> > >
> > >     /cc "Ghanshyam Thakkar" shyamthakkar001@gmail.com
> >
> > 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

← back to recent threads