cmake: determine list of extra built-ins dynamically
PATCH 1/1 cmake: determine list of extra built-ins dynamically
From: Johannes Schindelin <[email protected]>
In 0a21d0e08902 (Makefile: mark git-maintenance as a builtin, 2020-12-01), we marked git-maintenance as a builtin in the Makefile, but forgot to do the same in CMakeLists.txt.
Rather than always play catch-up and adjust git_builtin_extra manually, use the BUILT_INS definitions in the Makefile as authoritative source and generate git_builtin_extra dynamically.
---
ci: fix the vs-build job after adding git maintenance
Together with https://github.com/gitgitgadget/git/pull/805, this should fix the vs-build job in the CI build of seen.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-807%2Fdscho%2Fcmake-and-extra-builtins-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-807/dscho/cmake-and-extra-builtins-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/807
contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
contrib/buildsystems/CMakeLists.txt +11 -4
@@ -114,6 +114,16 @@ macro(parse_makefile_for_scripts list_var regex lang) endif() endmacro() +macro(parse_makefile_for_executables list_var regex) + file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+= git-(.*)") + string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}}) + string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces + string(REPLACE "git-" "" ${list_var} ${${list_var}}) #strip `git-` prefix + string(REPLACE "\$X" ";" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list + list(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list + list(REMOVE_ITEM ${list_var} "") #remove empty list elements +endmacro() + include(CheckTypeSize) include(CheckCSourceRuns) include(CheckCSourceCompiles)
@@ -673,10 +683,7 @@ if(CURL_FOUND) endif() endif() -set(git_builtin_extra - cherry cherry-pick format-patch fsck-objects - init merge-subtree restore show - stage status switch whatchanged) +parse_makefile_for_executables(git_builtin_extra "BUILT_INS") #Creating hardlinks foreach(s ${git_SOURCES} ${git_builtin_extra})
base-commit: 898f80736c75878acc02dc55672317fcc0e0a5a6
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 10 lines · writes:
From: Johannes Schindelin <[email protected]>
In
0a21d0e08902(Makefile: mark git-maintenance as a builtin, 2020-12-01), we marked git-maintenance as a builtin in theMakefile, but forgot to do the same inCMakeLists.txt.Rather than always play catch-up and adjust
git_builtin_extramanually, use theBUILT_INSdefinitions in theMakefileas authoritative source and generategit_builtin_extradynamically.
Quoted 6 lines
Signed-off-by Johannes Schindelin <[email protected]>---
ci: fix the vs-build job after adding git maintenanceTogether with https://github.com/gitgitgadget/git/pull/805, this should fix the vs-build job in the CI build of seen.
I suspect that the breakage 805 addresses affects even 'master'; is the breakage this patch fixes limited to 'seen', or affect everybody?
Quoted 42 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-807%2Fdscho%2Fcmake-and-extra-builtins-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-807/dscho/cmake-and-extra-builtins-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/807contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)contrib/buildsystems/CMakeLists.txt +11
-4@@ -114,6 +114,16 @@ macro(parse_makefile_for_scripts list_var regex lang) endif() endmacro() +macro(parse_makefile_for_executables list_var regex) + file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+= git-(.*)") + string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}}) + string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces + string(REPLACE "git-" "" ${list_var} ${${list_var}}) #strip `git-` prefix + string(REPLACE "\$X" ";" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list + list(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list + list(REMOVE_ITEM ${list_var} "") #remove empty list elements +endmacro() + include(CheckTypeSize) include(CheckCSourceRuns) include(CheckCSourceCompiles)@@ -673,10 +683,7 @@ if(CURL_FOUND) endif() endif() -set(git_builtin_extra - cherry cherry-pick format-patch fsck-objects - init merge-subtree restore show - stage status switch whatchanged) +parse_makefile_for_executables(git_builtin_extra "BUILT_INS") #Creating hardlinks foreach(s ${git_SOURCES} ${git_builtin_extra})base-commit:
898f80736c75878acc02dc55672317fcc0e0a5a6
Quoted 3 lines · Johannes Schindelin via GitGitGadget
In
0a21d0e08902(Makefile: mark git-maintenance as a builtin, 2020-12-01), we marked git-maintenance as a builtin in theMakefile, but forgot to do the same inCMakeLists.txt.
Oof, right.
Quoted 3 lines
Rather than always play catch-up and adjust
git_builtin_extramanually, use theBUILT_INSdefinitions in theMakefileas authoritative source and generategit_builtin_extradynamically.
Yay. This is exactly how I'd hoped things would work or the cmake file in general. I don't mind following micro-formats within our Makefile to keep things easier for the cmake parsing side.
Quoted 2 lines
contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
The implementation looks plausibly correct to me (bearing in mind that I've never written cmake).
-Peff
Hi Junio,
Quoted 17 lines · Junio C Hamano
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 14 lines · writes:
From: Johannes Schindelin <[email protected]>
In
0a21d0e08902(Makefile: mark git-maintenance as a builtin, 2020-12-01), we marked git-maintenance as a builtin in theMakefile, but forgot to do the same inCMakeLists.txt.Rather than always play catch-up and adjust
git_builtin_extramanually, use theBUILT_INSdefinitions in theMakefileas authoritative source and generategit_builtin_extradynamically.Signed-off-by Johannes Schindelin <[email protected]>---
ci: fix the vs-build job after adding git maintenance
Quoted 5 lines
Quoted 2 lines
Together with https://github.com/gitgitgadget/git/pull/805, this should fix the vs-build job in the CI build of seen.
I suspect that the breakage 805 addresses affects even 'master'; is the breakage this patch fixes limited to 'seen', or affect everybody?
I haven't checked master yet, but next definitely was affected. I _think_ the problem was introduced when Peff's patch added the git-maintenance tool to the BUILT_INS list in the Makefile (and hence we try to tar it up with all the other build artifacts in the vs-build job) but wasn't added to CMakeLists.txt (and therefore vs-build would not generate that hard-link).
Ciao,
Dscho
Quoted 44 lines
Quoted 43 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-807%2Fdscho%2Fcmake-and-extra-builtins-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-807/dscho/cmake-and-extra-builtins-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/807contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)contrib/buildsystems/CMakeLists.txt +11
-4@@ -114,6 +114,16 @@ macro(parse_makefile_for_scripts list_var regex lang) endif() endmacro() +macro(parse_makefile_for_executables list_var regex) + file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+= git-(.*)") + string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}}) + string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces + string(REPLACE "git-" "" ${list_var} ${${list_var}}) #strip `git-` prefix + string(REPLACE "\$X" ";" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list + list(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list + list(REMOVE_ITEM ${list_var} "") #remove empty list elements +endmacro() + include(CheckTypeSize) include(CheckCSourceRuns) include(CheckCSourceCompiles)@@ -673,10 +683,7 @@ if(CURL_FOUND) endif() endif() -set(git_builtin_extra - cherry cherry-pick format-patch fsck-objects - init merge-subtree restore show - stage status switch whatchanged) +parse_makefile_for_executables(git_builtin_extra "BUILT_INS") #Creating hardlinks foreach(s ${git_SOURCES} ${git_builtin_extra})base-commit:
898f80736c75878acc02dc55672317fcc0e0a5a6
Hi Peff,
Quoted 9 lines · Jeff King
Quoted 3 lines · Johannes Schindelin via GitGitGadget
Rather than always play catch-up and adjust
git_builtin_extramanually, use theBUILT_INSdefinitions in theMakefileas authoritative source and generategit_builtin_extradynamically.Yay. This is exactly how I'd hoped things would work or the cmake file in general. I don't mind following micro-formats within our
Makefileto keep things easier for the cmake parsing side.
Me, too. I am somewhat embarrassed that I missed the git_builtin_extra list in my review of the CMake patch series...
Quoted 5 lines
Quoted 2 lines
contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)The implementation looks plausibly correct to me (bearing in mind that I've never written cmake).
It is a close copy of the two macros we already use to parse the Makefile for lists of .o files and for scripts.
Together with the fact that this patch fixes the CI build of Git for Windows' shears/seen branch (which is a continuously-rebased version of Git for Windows' main branch onto seen, plus fixups), I am fairly confident that it is correct.
Ciao,
Dscho