Re: What's cooking in git.git (Aug 2020, #01; Mon, 3)

[Jeff King <peff@peff.net>]

On Wed, Aug 12, 2020 at 03:35:06PM +0200, Johannes Schindelin wrote:

> > That was my philosophy, too, but it's annoying in the meantime as I get
> > a notification for "your build is broken" every time I run CI. So it
> > becomes a game of chicken over who gets annoyed first. ;)
> 
> I am a bit sad to read all this, as I thought that we had reached
> consensus that the `Makefile` _is_ the source of truth.
> 
> But then, most of the source files that need to be compiled _are_ parsed
> from the Makefile.
> 
> So I wonder what problems you ran into; Maybe we can come up with a
> strategy how to preempt future instances of the same nature?

There are definitely a lot of lists that are copied from the Makefile
into CMakeLists. For some concrete data, here are the patches I needed
for two of my topics.

This first one is for a topic that remotes git-remote-testsvn and
associated code.

---
 contrib/buildsystems/CMakeLists.txt | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 4be61247e5..bdd3121a7c 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -502,7 +502,7 @@ unset(CMAKE_REQUIRED_INCLUDES)
 #programs
 set(PROGRAMS_BUILT
 	git git-bugreport git-daemon git-fast-import git-http-backend git-sh-i18n--envsubst
-	git-shell git-remote-testsvn)
+	git-shell)
 
 if(NOT CURL_FOUND)
 	list(APPEND excluded_progs git-http-fetch git-http-push)
@@ -568,12 +568,6 @@ parse_makefile_for_sources(libxdiff_SOURCES "XDIFF_OBJS")
 list(TRANSFORM libxdiff_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 add_library(xdiff STATIC ${libxdiff_SOURCES})
 
-#libvcs-svn
-parse_makefile_for_sources(libvcs-svn_SOURCES "VCSSVN_OBJS")
-
-list(TRANSFORM libvcs-svn_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
-add_library(vcs-svn STATIC ${libvcs-svn_SOURCES})
-
 if(WIN32)
 	if(NOT MSVC)#use windres when compiling with gcc and clang
 		add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/git.res
@@ -660,9 +654,6 @@ if(CURL_FOUND)
 	endif()
 endif()
 
-add_executable(git-remote-testsvn ${CMAKE_SOURCE_DIR}/remote-testsvn.c)
-target_link_libraries(git-remote-testsvn common-main vcs-svn)
-
 set(git_builtin_extra
 	cherry cherry-pick format-patch fsck-objects
 	init merge-subtree restore show
@@ -838,26 +829,20 @@ if(BUILD_TESTING)
 add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c)
 target_link_libraries(test-fake-ssh common-main)
 
-add_executable(test-line-buffer ${CMAKE_SOURCE_DIR}/t/helper/test-line-buffer.c)
-target_link_libraries(test-line-buffer common-main vcs-svn)
-
-add_executable(test-svn-fe ${CMAKE_SOURCE_DIR}/t/helper/test-svn-fe.c)
-target_link_libraries(test-svn-fe common-main vcs-svn)
-
 #test-tool
 parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS")
 
 list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/t/helper/")
 add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES})
 target_link_libraries(test-tool common-main)
 
-set_target_properties(test-fake-ssh test-line-buffer test-svn-fe test-tool
+set_target_properties(test-fake-ssh test-tool
 			PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/helper)
 
 if(MSVC)
-	set_target_properties(test-fake-ssh test-line-buffer test-svn-fe test-tool
+	set_target_properties(test-fake-ssh test-tool
 				PROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/helper)
-	set_target_properties(test-fake-ssh test-line-buffer test-svn-fe test-tool
+	set_target_properties(test-fake-ssh test-tool
 				PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/helper)
 endif()
 
@@ -866,7 +851,7 @@ set(wrapper_scripts
 	git git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext)
 
 set(wrapper_test_scripts
-	test-fake-ssh test-line-buffer test-svn-fe test-tool)
+	test-fake-ssh test-tool)
 
 
 foreach(script ${wrapper_scripts})
@@ -967,7 +952,6 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(COPY ${CMAKE_SOURCE_DIR}/mergetools/tkdiff DESTINATION ${CMAKE_BINARY_DIR}/mergetools/)
 	file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-prompt.sh DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)
 	file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)
-	file(COPY ${CMAKE_SOURCE_DIR}/contrib/svn-fe/svnrdump_sim.py DESTINATION ${CMAKE_BINARY_DIR}/contrib/svn-fe/)
 endif()
 
 file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")


It's mostly removal, which is nice, but I think it gives a pretty clear
example of how often lists from Makefile are replicated (and often
repeated in several spots within CMakeLists). I suspect most of these
lists could be pulled from the Makefile with a pre-processing step.

This second one is for a topic which moved some credential programs into
builtins (since they link libgit.a and nothing else, there's no reason
for them to take up extra space on disk). Even if we read more lists
from the Makefile, I think these hunks still would have needed to be
modified in CMakeLists because I changed the way they interact with
NO_UNIX_SOCKETS (instead of not building credential-cache in that case,
we get a builtin that says "sorry, this was built with
NO_UNIX_SOCKETS").

---
 contrib/buildsystems/CMakeLists.txt | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 47215df25b..4be61247e5 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -501,15 +501,9 @@ unset(CMAKE_REQUIRED_INCLUDES)
 
 #programs
 set(PROGRAMS_BUILT
-	git git-bugreport git-credential-store git-daemon git-fast-import git-http-backend git-sh-i18n--envsubst
+	git git-bugreport git-daemon git-fast-import git-http-backend git-sh-i18n--envsubst
 	git-shell git-remote-testsvn)
 
-if(NO_UNIX_SOCKETS)
-	list(APPEND excluded_progs git-credential-cache git-credential-cache--daemon)
-else()
-	list(APPEND PROGRAMS_BUILT git-credential-cache git-credential-cache--daemon)
-endif()
-
 if(NOT CURL_FOUND)
 	list(APPEND excluded_progs git-http-fetch git-http-push)
 	add_compile_definitions(NO_CURL)
@@ -633,9 +627,6 @@ target_link_libraries(git common-main)
 add_executable(git-bugreport ${CMAKE_SOURCE_DIR}/bugreport.c)
 target_link_libraries(git-bugreport common-main)
 
-add_executable(git-credential-store ${CMAKE_SOURCE_DIR}/credential-store.c)
-target_link_libraries(git-credential-store common-main)
-
 add_executable(git-daemon ${CMAKE_SOURCE_DIR}/daemon.c)
 target_link_libraries(git-daemon common-main)
 
@@ -672,15 +663,6 @@ endif()
 add_executable(git-remote-testsvn ${CMAKE_SOURCE_DIR}/remote-testsvn.c)
 target_link_libraries(git-remote-testsvn common-main vcs-svn)
 
-if(NOT NO_UNIX_SOCKETS)
-	add_executable(git-credential-cache ${CMAKE_SOURCE_DIR}/credential-cache.c)
-	target_link_libraries(git-credential-cache common-main)
-
-	add_executable(git-credential-cache--daemon ${CMAKE_SOURCE_DIR}/credential-cache--daemon.c)
-	target_link_libraries(git-credential-cache--daemon common-main)
-endif()
-
-
 set(git_builtin_extra
 	cherry cherry-pick format-patch fsck-objects
 	init merge-subtree restore show