diff mbox series

[v2,10/11] cmake: parse the makefile for the sources.

Message ID b0a321a714a766962456e2a9e8dd7d2be349e34d.1589302255.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series CMake build system for git | expand

Commit Message

Johannes Schindelin via GitGitGadget May 12, 2020, 4:50 p.m. UTC 
From: Sibi Siddharthan <sibisiddharthan.github@gmail.com>

The CMake script parses the Makefile for:
SCRIPT_SH
SCRIPT_PERL
TEST_BUILTINS_OBJS
LIB_OBJS
BUILTIN_OBJS
XDIFF_OBJS
VCSSVN_OBJS

By doing this we avoid duplication of text between the Makefile and
the CMake script.

Signed-off-by: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 137 ++++++----------------------
 1 file changed, 30 insertions(+), 107 deletions(-)

Comments

Junio C Hamano May 12, 2020, 9:03 p.m. UTC | #1 
"Sibi Siddharthan via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
>
> The CMake script parses the Makefile for:
> SCRIPT_SH
> SCRIPT_PERL
> TEST_BUILTINS_OBJS
> LIB_OBJS
> BUILTIN_OBJS
> XDIFF_OBJS
> VCSSVN_OBJS
>
> By doing this we avoid duplication of text between the Makefile and
> the CMake script.

Thanks, this makes perfect sense, but it probably should have been
done from the very beginning, without copying the huge list manually
in the step [01/11].

How robust is the "parser"?  Do we need to leave an instruction to
Makefile writers to conform to certain style to help CMake users as
an additional comment in the Makefile, or is any reasonable textual
drift still accepted?
Sibi Siddharthan May 13, 2020, 7:57 p.m. UTC | #2 
On Wed, May 13, 2020 at 2:33 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Sibi Siddharthan via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
> >
> > The CMake script parses the Makefile for:
> > SCRIPT_SH
> > SCRIPT_PERL
> > TEST_BUILTINS_OBJS
> > LIB_OBJS
> > BUILTIN_OBJS
> > XDIFF_OBJS
> > VCSSVN_OBJS
> >
> > By doing this we avoid duplication of text between the Makefile and
> > the CMake script.
>
> Thanks, this makes perfect sense, but it probably should have been
> done from the very beginning, without copying the huge list manually
> in the step [01/11].
>
> How robust is the "parser"?  Do we need to leave an instruction to
> Makefile writers to conform to certain style to help CMake users as
> an additional comment in the Makefile, or is any reasonable textual
> drift still accepted?
>

The parser just uses a regex capture like this "SCRIPT_SH += (.*)".
If the current source/script definition in the Makefile is continued
things will be fine.

On a side note, why are we using SCRIPT_SH and SCRIPT_LIB separately?
Can't we combine them into one? If we can then I can also add it to
the list above.

Thank You,
Sibi Siddharthan
Junio C Hamano May 13, 2020, 8:23 p.m. UTC | #3 
Sibi Siddharthan <sibisiddharthan.github@gmail.com> writes:

> On a side note, why are we using SCRIPT_SH and SCRIPT_LIB separately?

I think _SH are commands to be installed as executable, while _LIB
are to be dot-included and installed as non-executable.  

There may be other differences; they (i.e. what the Makefile uses
these lists for, how the elements on these lists are treated, etc.)
are something you need to know before you can reimplement the logic
of building and installing used by the current system in a different
build system.

Thanks.
diff mbox series

Patch

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index e0ce069dc2a..4592ce614cf 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -21,6 +21,25 @@  project(git
 #TODO Enable NLS on windows natively
 #TODO Add pcre support
 
+#macros for parsing the Makefile for sources and scripts
+macro(parse_makefile_for_sources list_var regex)
+	file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+=(.*)")
+	string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}})
+	string(REPLACE "$(COMPAT_OBJS)" "" ${list_var} ${${list_var}}) #remove "$(COMPAT_OBJS)" This is only for libgit.
+	string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces
+	string(REPLACE ".o" ".c;" ${list_var} ${${list_var}}) #change .o to .c, ; 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()
+
+macro(parse_makefile_for_scripts list_var regex lang)
+	file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+=(.*)")
+	string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}})
+	string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces
+	string(REPLACE " " ";" ${list_var} ${${list_var}}) #convert string to a list
+	list(TRANSFORM ${list_var} REPLACE "${lang}" "") #do the replacement
+endmacro()
+
 
 include(CheckTypeSize)
 include(CheckCSourceRuns)
@@ -489,57 +508,18 @@  include_directories(${CMAKE_BINARY_DIR})
 
 
 #build
-set(libgit_SOURCES
-	abspath.c add-interactive.c add-patch.c advice.c alias.c
-	alloc.c apply.c archive.c archive-tar.c archive-zip.c argv-array.c
-	attr.c base85.c bisect.c blame.c blob.c bloom.c branch.c bulk-checkin.c
-	bundle.c cache-tree.c chdir-notify.c checkout.c color.c column.c
-	combine-diff.c commit.c commit-graph.c commit-reach.c compat/obstack.c
-	compat/terminal.c config.c connect.c connected.c convert.c copy.c credential.c
-	csum-file.c ctype.c date.c decorate.c delta-islands.c diffcore-break.c
-	diffcore-delta.c diffcore-order.c diffcore-pickaxe.c diffcore-rename.c
-	diff-delta.c diff-lib.c diff-no-index.c diff.c dir.c dir-iterator.c editor.c
-	entry.c environment.c ewah/bitmap.c ewah/ewah_bitmap.c ewah/ewah_io.c
-	ewah/ewah_rlw.c exec-cmd.c fetch-negotiator.c fetch-pack.c fmt-merge-msg.c fsck.c fsmonitor.c
-	gettext.c gpg-interface.c graph.c grep.c hashmap.c linear-assignment.c help.c hex.c
-	ident.c interdiff.c json-writer.c kwset.c levenshtein.c line-log.c line-range.c list-objects.c
-	list-objects-filter.c list-objects-filter-options.c ll-merge.c lockfile.c
-	log-tree.c ls-refs.c mailinfo.c mailmap.c match-trees.c mem-pool.c merge.c merge-blobs.c
-	merge-recursive.c mergesort.c midx.c name-hash.c negotiator/default.c
-	negotiator/skipping.c notes.c notes-cache.c notes-merge.c notes-utils.c object.c oidmap.c
-	oidset.c oid-array.c packfile.c pack-bitmap.c pack-bitmap-write.c pack-check.c pack-objects.c
-	pack-revindex.c pack-write.c pager.c parse-options.c parse-options-cb.c patch-delta.c
-	patch-ids.c path.c pathspec.c pkt-line.c preload-index.c pretty.c prio-queue.c progress.c
-	promisor-remote.c prompt.c protocol.c prune-packed.c quote.c range-diff.c reachable.c read-cache.c rebase.c
-	rebase-interactive.c reflog-walk.c refs.c refs/files-backend.c refs/iterator.c
-	refs/packed-backend.c refs/ref-cache.c refspec.c ref-filter.c remote.c replace-object.c
-	repo-settings.c repository.c rerere.c reset.c resolve-undo.c revision.c run-command.c
-	send-pack.c sequencer.c serve.c server-info.c setup.c sha1-lookup.c
-	sha1-file.c sha1-name.c shallow.c sideband.c sigchain.c split-index.c
-	stable-qsort.c strbuf.c streaming.c string-list.c submodule.c submodule-config.c
-	sub-process.c symlinks.c tag.c tempfile.c thread-utils.c tmp-objdir.c
-	trace.c trace2.c trace2/tr2_cfg.c trace2/tr2_cmd_name.c trace2/tr2_dst.c
-	trace2/tr2_sid.c trace2/tr2_sysenv.c trace2/tr2_tbuf.c trace2/tr2_tgt_event.c
-	trace2/tr2_tgt_normal.c trace2/tr2_tgt_perf.c trace2/tr2_tls.c trailer.c transport.c
-	transport-helper.c tree-diff.c tree.c tree-walk.c unpack-trees.c upload-pack.c url.c
-	urlmatch.c usage.c userdiff.c utf8.c varint.c version.c versioncmp.c walker.c wildmatch.c
-	worktree.c wrapper.c write-or-die.c ws.c wt-status.c xdiff-interface.c
-	zlib.c)
+parse_makefile_for_sources(libgit_SOURCES "LIB_OBJS")
 
 list(TRANSFORM libgit_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 list(TRANSFORM compat_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 add_library(libgit ${libgit_SOURCES} ${compat_SOURCES})
 
-set(libxdiff_SOURCES
-	xdiff/xdiffi.c xdiff/xprepare.c xdiff/xutils.c xdiff/xemit.c
-	xdiff/xmerge.c xdiff/xpatience.c xdiff/xhistogram.c)
+parse_makefile_for_sources(libxdiff_SOURCES "XDIFF_OBJS")
 
 list(TRANSFORM libxdiff_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 add_library(xdiff STATIC ${libxdiff_SOURCES})
 
-set(libvcs-svn_SOURCES
-	vcs-svn/line_buffer.c vcs-svn/sliding_window.c vcs-svn/fast_export.c
-	vcs-svn/svndiff.c vcs-svn/svndump.c)
+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})
@@ -588,40 +568,7 @@  elseif(UNIX)
 endif()
 
 
-set(git_SOURCES
-	builtin/add.c builtin/am.c builtin/annotate.c builtin/apply.c
-	builtin/archive.c builtin/bisect--helper.c builtin/blame.c
-	builtin/branch.c builtin/bundle.c builtin/cat-file.c builtin/check-attr.c
-	builtin/check-ignore.c builtin/check-mailmap.c builtin/check-ref-format.c
-	builtin/checkout-index.c builtin/checkout.c builtin/clean.c
-	builtin/clone.c builtin/column.c builtin/commit-tree.c
-	builtin/commit.c builtin/commit-graph.c builtin/config.c
-	builtin/count-objects.c builtin/credential.c builtin/describe.c
-	builtin/diff-files.c builtin/diff-index.c builtin/diff-tree.c
-	builtin/diff.c builtin/difftool.c builtin/env--helper.c
-	builtin/fast-export.c builtin/fetch-pack.c builtin/fetch.c builtin/fmt-merge-msg.c
-	builtin/for-each-ref.c builtin/fsck.c builtin/gc.c
-	builtin/get-tar-commit-id.c builtin/grep.c builtin/hash-object.c
-	builtin/help.c builtin/index-pack.c builtin/init-db.c
-	builtin/interpret-trailers.c builtin/log.c builtin/ls-files.c
-	builtin/ls-remote.c builtin/ls-tree.c builtin/mailinfo.c builtin/mailsplit.c
-	builtin/merge.c builtin/merge-base.c builtin/merge-file.c builtin/merge-index.c
-	builtin/merge-ours.c builtin/merge-recursive.c builtin/merge-tree.c
-	builtin/mktag.c builtin/mktree.c builtin/multi-pack-index.c builtin/mv.c
-	builtin/name-rev.c builtin/notes.c builtin/pack-objects.c builtin/pack-redundant.c
-	builtin/pack-refs.c builtin/patch-id.c builtin/prune-packed.c builtin/prune.c
-	builtin/pull.c builtin/push.c builtin/range-diff.c builtin/read-tree.c
-	builtin/rebase.c builtin/receive-pack.c builtin/reflog.c builtin/remote.c
-	builtin/remote-ext.c builtin/remote-fd.c builtin/repack.c builtin/replace.c
-	builtin/rerere.c builtin/reset.c builtin/rev-list.c builtin/rev-parse.c
-	builtin/revert.c builtin/rm.c builtin/send-pack.c builtin/shortlog.c
-	builtin/show-branch.c builtin/show-index.c builtin/show-ref.c
-	builtin/sparse-checkout.c builtin/stash.c builtin/stripspace.c
-	builtin/submodule--helper.c builtin/symbolic-ref.c builtin/tag.c
-	builtin/unpack-file.c builtin/unpack-objects.c builtin/update-index.c
-	builtin/update-ref.c builtin/update-server-info.c builtin/upload-archive.c
-	builtin/upload-pack.c builtin/var.c builtin/verify-commit.c builtin/verify-pack.c
-	builtin/verify-tag.c builtin/worktree.c builtin/write-tree.c)
+parse_makefile_for_sources(git_SOURCES "BUILTIN_OBJS")
 
 list(TRANSFORM git_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
 add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})
@@ -714,11 +661,9 @@  set(GITWEBDIR ${FALLBACK_RUNTIME_PREFIX}/share/locale)
 set(INSTLIBDIR ${FALLBACK_RUNTIME_PREFIX}/share/perl5)
 
 #shell scripts
+parse_makefile_for_scripts(git_sh_scripts "SCRIPT_SH" ".sh")
 set(git_shell_scripts
-	git-bisect git-difftool--helper git-filter-branch
-	git-merge-octopus git-merge-one-file git-merge-resolve
-	git-mergetool git-quiltimport
-	git-request-pull git-submodule git-web--browse
+	${git_sh_scripts}
 	git-mergetool--lib git-parse-remote git-rebase--preserve-merges
 	git-sh-setup git-sh-i18n git-instaweb)
 
@@ -738,9 +683,7 @@  foreach(script ${git_shell_scripts})
 endforeach()
 
 #perl scripts
-set(git_perl_scripts
-	git-add--interactive git-archimport git-cvsexportcommit
-	git-cvsimport git-cvsserver git-send-email git-svn)
+parse_makefile_for_scripts(git_perl_scripts "SCRIPT_PERL" ".perl")
 
 #create perl header
 file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )
@@ -865,30 +808,10 @@  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)
 
-set(test-tool_SOURCES
-	t/helper/test-tool.c t/helper/test-advise.c t/helper/test-bloom.c t/helper/test-chmtime.c
-	t/helper/test-config.c t/helper/test-ctype.c t/helper/test-date.c t/helper/test-delta.c
-	t/helper/test-dir-iterator.c t/helper/test-drop-caches.c t/helper/test-dump-cache-tree.c
-	t/helper/test-dump-fsmonitor.c t/helper/test-dump-split-index.c
-	t/helper/test-dump-untracked-cache.c t/helper/test-example-decorate.c
-	t/helper/test-genrandom.c t/helper/test-genzeros.c t/helper/test-hash.c
-	t/helper/test-hashmap.c t/helper/test-hash-speed.c t/helper/test-index-version.c
-	t/helper/test-json-writer.c t/helper/test-lazy-init-name-hash.c
-	t/helper/test-match-trees.c t/helper/test-mergesort.c t/helper/test-mktemp.c
-	t/helper/test-oidmap.c t/helper/test-online-cpus.c t/helper/test-parse-options.c
-	t/helper/test-parse-pathspec-file.c t/helper/test-path-utils.c t/helper/test-pkt-line.c
-	t/helper/test-prio-queue.c t/helper/test-progress.c t/helper/test-reach.c
-	t/helper/test-read-cache.c t/helper/test-read-graph.c t/helper/test-read-midx.c
-	t/helper/test-ref-store.c t/helper/test-regex.c t/helper/test-repository.c
-	t/helper/test-revision-walking.c t/helper/test-run-command.c t/helper/test-scrap-cache-tree.c
-	t/helper/test-serve-v2.c t/helper/test-sha1.c t/helper/test-oid-array.c t/helper/test-sha256.c
-	t/helper/test-sigchain.c t/helper/test-strcmp-offset.c t/helper/test-string-list.c
-	t/helper/test-submodule-config.c t/helper/test-submodule-nested-repo-config.c t/helper/test-subprocess.c
-	t/helper/test-trace2.c t/helper/test-urlmatch-normalization.c t/helper/test-xml-encode.c
-	t/helper/test-wildmatch.c t/helper/test-windows-named-pipe.c t/helper/test-write-cache.c)
-
-list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
-add_executable(test-tool ${test-tool_SOURCES})
+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