CMake and Visual Studio

The intention of this patch series is to make it as easy as possible for a Visual Studio user to start building Git from scratch. The idea is to

* clone Git's source code,
* open it in Visual Studio via File>Open>Folder...
* let CMake configure things (and vcpkg build the dependencies, which will take a while), and
* start the build.

It does require a Git for Windows to be installed (so that the source can be cloned, but also to run the shell script parts of the CMake configuration), and also to run the tests.

This patch series replaces the patch "Ignore files generated by CMake" I sent via https://github.com/gitgitgadget/git/pull/735.

Johannes Schindelin (10):
cmake: ignore files generated by CMake as run in Visual Studio
cmake: do find Git for Windows' shell interpreter
cmake: ensure that the vcpkg packages are found on Windows
cmake: fall back to using vcpkg's msgfmt.exe on Windows
cmake: quote the path accurately when editing test-lib.sh cmake (Windows): let the .dll files are found when running the tests cmake (Windows): complain when encountering an unknown compiler cmake (Windows): initialize vcpkg/build dependencies automatically cmake (Windows): recommend using Visual Studio's built-in CMake
support
hashmap_for_each_entry(): work around MSVC's run-time check failure #3

 .gitignore                          |  1 +
 attr.c                              |  2 +-
 blame.c                             |  4 +--
 bloom.c                             |  2 +-
 builtin/describe.c                  |  2 +-
 builtin/difftool.c                  |  2 +-
 builtin/fast-import.c               |  2 +-
 builtin/sparse-checkout.c           |  6 ++--
 config.c                            |  2 +-
 contrib/buildsystems/CMakeLists.txt | 43 +++++++++++++++++++++++++----
 merge-recursive.c                   | 10 +++----
 name-hash.c                         |  2 +-
 revision.c                          |  4 +--
 submodule-config.c                  |  2 +-
 t/helper/test-hashmap.c             |  2 +-
 t/helper/test-lazy-init-name-hash.c |  4 +--
 16 files changed, 62 insertions(+), 28 deletions(-)

base-commit: e1cfff676549cdcd702cbac105468723ef2722f4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-738%2Fdscho%2Fcmake-and-visual-studio-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-738/dscho/cmake-and-visual-studio-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/738

gitgitgadget

From: Johannes Schindelin <[email protected]>

As of recent Visual Studio versions, CMake support is built-in:
https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

All that needs to be done is to open the worktree as a folder, and Visual Studio will find the CMakeLists.txt file and automatically generate the project files.

Let's ignore the entirety of those generated files.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

index d0f692a355..6521f3cf22 100644
@@ -241,3 +241,4 @@ Release/
 /git.VC.VC.opendb
 /git.VC.db
 *.dSYM
+/contrib/buildsystems/out

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, Git for Windows does not install its sh.exe into the PATH. However, our current CMakeLists.txt expects to find a shell interpreter in the PATH.

So let's fall back to looking in the default location where Git for Windows _does_ install a relatively convenient sh.exe:
C:\Program Files\Git\bin\sh.exe

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

index 5007f173f1..d14fa4f3dc 100644
@@ -43,8 +43,11 @@ set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
 find_program(SH_EXE sh)
 if(NOT SH_EXE)
-	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
-			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+	set(SH_EXE "C:/Program Files/Git/bin/sh.exe")
+	if(NOT EXISTS ${SH_EXE})
+		message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
+				"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+	endif()
 endif()
 
 #Create GIT-VERSION-FILE using GIT-VERSION-GEN

gitgitgadget

Hi Sibi,

It is not doing that, except as a fall-back in case the sh program was not found in the PATH.

Right.

No, it does not. Quite purposefully so.

The /bin/ directory is kind of a hack to reinstate _some_ level of support for use cases that relied on Git for Windows v1.x installing its binaries into that directory (v2.x distributes them between /usr/bin/ and /mingw64/bin/).

What _does_ get appended, at least by default, is the /cmd/ directory (which does _not_ contain sh.exe).

Now, there _is_ an option in the Git for Windows installer to append all of its Unix tools to the PATH, but it is highly discouraged to do so.

Sure, and that's your prerogative. There's unfortunately no good way to support your use case.

Luckily, the vast majority of Git for Windows' users do not change the default location, and this patch is for them. (And "them" in this case includes me, personally ;-))

Ciao,
Dscho

I agree with this decision.

This doesn't fit into my view of Git for Windows' users For some users that have the Administrator right, it's the default location if they grant the Administrator right for the installer.

For those poor souls that works for enterprise companies, and thoses that not feel comfortable give Administrator right to _another_ installer, the installer will install into (hopeful, I type it right):

%USERPROFILE%/AppData/Local/Programs/Git

I think it's better to offer SH_EXE as an OPTION, let user specify it as will. And we'll search in PATH if it's not specified, fallback to 2 default value if not found.

Danh

Hi Danh,

Those poor souls that work for enterprise companies often have Git for Windows installed by default. And of course, that default would be in C:\Program Files\Git.

That's exactly as it is right now. You can specify SH_EXE (but only if running CMake manually, not via Visual Studio). If you don't, it searches PATH, and with my patch it then falls back to trying to find sh.exe in Git for Windows' default location.

So I think we're in agreement here?

Ciao,
Dscho

Yes, that's correct, but that Git is usually very old, and I'm not sure about its layout. Obviously, you know better in this regard :-p

Yes, seems good.

Danh

Hi Danh,

In Git for Windows v1.x, it would have contained the real Bash (at least in 32-bit Windows; in 64-bit Windows, it would have been C:\Program Files (x86)\Git\bin). But v1.x is long deprecated, it's over 5 years old.

The C:\Program Files\Git\bin\sh.exe stand-in is actually not _all_ that old, I only reintroduced it relatively recently. Nevertheless, it is the best bet for a default fall-back that I can think of.

Excellent!
Dscho

From: Johannes Schindelin <[email protected]>

On Windows, we use the vcpkg project to manage the dependencies, via compat/vcbuild/. Let's make sure that these dependencies are found by default.

This is needed because we are about to recommend loading the Git worktree as a folder into Visual Studio, relying on the automatic CMake support (which would make it relatively cumbersome to adjust the search path used by CMake manually).

---
 contrib/buildsystems/CMakeLists.txt | 7 +++++++
 1 file changed, 7 insertions(+)

index d14fa4f3dc..16014ac4f7 100644
@@ -40,6 +40,13 @@ cmake_minimum_required(VERSION 3.14)
 
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
+if(WIN32)
+	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
+
+	# In the vcpkg edition, we need this to be able to link to libcurl
+	set(CURL_NO_CURL_CMAKE ON)
+endif()
 
 find_program(SH_EXE sh)
 if(NOT SH_EXE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We are already relying on vcpkg to manage our dependencies, including libiconv. Let's also use the msgfmt.exe from there.

---
 contrib/buildsystems/CMakeLists.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 16014ac4f7..acb197fba5 100644
@@ -155,7 +155,11 @@ endif()
 
 find_program(MSGFMT_EXE msgfmt)
 if(NOT MSGFMT_EXE)
-	message(WARNING "Text Translations won't be build")
+	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
+	if(NOT EXISTS ${MSGFMT_EXE})
+		message(WARNING "Text Translations won't be build")
+		unset(MSGFMT_EXE)
+	endif()
 endif()
 
 #Force all visual studio outputs to CMAKE_BINARY_DIR

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, the build directory will be called something like contrib/buildsystems/out/build/x64-Debug (default) (note the space and the parentheses). We need to make sure that such a path is quoted properly when editing the assignment of the GIT_BUILD_DIR variable.

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index acb197fba5..e66ae137c2 100644
@@ -952,7 +952,7 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
-		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY\\\"/../${BUILD_DIR_RELATIVE}\" content \"\${content}\")\n"
+		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
 		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
 	#misc copies
 	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)

gitgitgadget

From: Johannes Schindelin <[email protected]>

Contrary to Unix-ish platforms, the dependencies' shared libraries are not usually found in one central place. In our case, since we use vcpkg, they are to be found inside the compat/vcbuild/vcpkg/ tree.

Let's make sure that they are in the search path when running the tests.

---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

index e66ae137c2..5a4e0ccb01 100644
@@ -942,6 +942,9 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n")
+if(WIN32)
+	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n")
+endif()
 
 #Make the tests work when building out of the source tree
 get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)

gitgitgadget

I'm having trouble parsing this. Maybe you want s/let/ensure/ ?

Hi Eric,

Right, that does not parse at all. I did s/are/be/ instead.

Thanks,
Dscho

From: Johannes Schindelin <[email protected]>

We have some custom handling regarding the link options, which are specific to each compiler.

Therefore: let's not just continue without setting the link options when configuring for a currently unhandled compiler, but error out.

Note: for some reason, the invalidcontinue.obj trick seems not to work in this developer's hands when building Git in Debug mode: the assertion dialog that is supposed to be suppressed still pops up, complaining about the entry variable being uninitialized when it is used in config.c's line 1961.

---
 contrib/buildsystems/CMakeLists.txt | 2 ++
 1 file changed, 2 insertions(+)

index 5a4e0ccb01..d21849b730 100644
@@ -620,6 +620,8 @@ if(WIN32)
 		target_link_options(common-main PUBLIC -municode -Wl,-nxcompat -Wl,-dynamicbase -Wl,-entry:wmainCRTStartup -Wl,invalidcontinue.obj)
 	elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 		target_link_options(common-main PUBLIC /IGNORE:4217 /IGNORE:4049 /NOLOGO /ENTRY:wmainCRTStartup /SUBSYSTEM:CONSOLE invalidcontinue.obj)
+	else()
+		message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}")
 	endif()
 elseif(UNIX)
 	target_link_libraries(common-main pthread rt)

gitgitgadget

Does this happen when running Git under the Visual Studio debugger? If so this might help
_CrtSetReportMode(_CRT_ASSERT,0);

After digging a bit more I found out that _CRTDBG_MODE_DEBUG is environment sensitive.
It depends on how the debugger handles DbgPrintEx().

Thank You,
Sibi Siddharthan

Hi Sibi,

Whoops. I meant to remove this paragraph after addressing that run-time check failure #3 problem.

Will remove it before sending the next iteration.

Thanks,
Dscho

From: Johannes Schindelin <[email protected]>

The idea of having CMake support in Git's source tree is to enable contributors on Windows to start contributing with little effort. To that end, we just added some sensible defaults that will let users open the worktree in Visual Studio and start building.

This expects the dependencies (such as zlib) to be available already, though. If they are not available, we expect the user to run compat/vcbuild/vcpkg_install.bat.

Rather than requiring this step to be manual, detect the situation and run it as part of the CMake configuration step.

This concludes our journey to make it as effortless as possible to start developing Git in Visual Studio: all the developer needs to do is to clone Git's repository, open the worktree via File>Open>Folder... and wait for CMake to finish configuring.

---
 contrib/buildsystems/CMakeLists.txt | 4 ++++
 1 file changed, 4 insertions(+)

index d21849b730..1eaeb8b8e0 100644
@@ -42,6 +42,10 @@ cmake_minimum_required(VERSION 3.14)
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 if(WIN32)
 	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	if(NOT EXISTS ${VCPKG_DIR})
+		message("Initializinge vcpkg and building the Git's dependencies (this will take a while...)")
+		execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat)
+	endif()
 	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
 
 	# In the vcpkg edition, we need this to be able to link to libcurl

gitgitgadget

After reading the patch series I seem to get to feeling that people on Windows are being nudged to use Visual Studio and vcpkg. Although they are great tools, when I want to specify my own libraries I don't see an option here.
I think we need to define a variable, which this conditional block uses, which is set to true if we are using a Visual Studio Generator. We also need a way to override this option if needed.
If this variable(explained above) is not set or false or OFF, we defer building vcpkg libraries. This can also save time in the case when we already have the dependencies(libraries) and just want to point to them instead of building with vcpkg.
I also see that people who use gcc(MinGW) are left out. I think we also need to set the supported compiler(s) here before executing vcpkg_install.bat. People who use gcc would need to point to their own libraries with CMAKE_PREFIX_PATH.

Thank You,
Sibi Siddharthan

Hi Sibi,

Good points, even if our current CMake push really tries to cater only to Visual Studio (or at least, MS Visual C) users.

I added a further guard to the conditional: MSVC needs to be true. This makes sense, as our current support for MSVC/Visual Studio hinges on having vcpkg build the dependencies, we simply do not support anything else. (And it would be easy enough for a proficient Visual Studio user to _still_ override the vcpkg system with their own libraries, if they really want to make that work.)

Thanks,
Dscho

From: Johannes Schindelin <[email protected]>

It is a lot more convenient to use than having to specify the configuration in CMake manually (does not matter whether using the command-line or CMake's GUI).

---
 contrib/buildsystems/CMakeLists.txt | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

index 1eaeb8b8e0..442b4e69ad 100644
@@ -4,7 +4,17 @@
 
 #[[
 
-Instructions to run CMake:
+Instructions how to use this in Visual Studio:
+
+Open the worktree as a folder. Visual Studio 2019 and later will detect
+the CMake configuration automatically and set everything up for you,
+ready to build. You can then run the tests in `t/` via a regular Git Bash.
+
+Note: Visual Studio also has the option of opening the CMake configuration
+directly; Using this option, Visual Studio will not find the source code,
+though, therefore the `File>Open>Folder...` option is preferred.
+
+Instructions to run CMake manually:
 
 cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
 Eg.

gitgitgadget

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

 contrib/buildsystems/CMakeLists.txt | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

index 442b4e69ad..0c748949f9 100644
@@ -16,15 +16,13 @@ though, therefore the `File>Open>Folder...` option is preferred.
 
 Instructions to run CMake manually:
 
-cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
-Eg.
-From the root of git source tree
-	`cmake contrib/buildsystems/ `
-This will build the git binaries at the root
-
-For out of source builds, say build in 'git/git-build/'
-	`mkdir git-build;cd git-build; cmake ../contrib/buildsystems/`
-This will build the git binaries in git-build directory
+    mkdir -p contrib/buildsystems/out
+    cd contrib/buildsystems/out
+    cmake ../ -DCMAKE_BUILD_TYPE=Release
+
+This will build the git binaries in contrib/buildsystems/out
+directory (our top-level .gitignore file knows to ignore contents of
+this directory).
 
 Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding
 compiler flags

Hi Junio,

I squashed your changes into the patch.

The parts that still mention Visual Studio are in the part that talks about running CMake manually, in which case they strike me as helpful, still.

Thank you,
Dscho

From: Johannes Schindelin <[email protected]>

When compiling Git in Visual C, we do not have the luxury of support for typeof(), and therefore OFFSETOF_VAR() unfortunately has to fall back to pointer arithmetic.

When compiling code using the hashmap_for_each_entry() macro in Debug mode, this leads to the "run-time check failure #3" because the variable passed as var are not initialized, yet we calculate the pointer difference &(var->member)-var.

This "run-time check failure" causes a scary dialog to pop up.

Work around this by initializing the respective variables.

Note: according to the C standard, performing pointer arithmetic with NULL is not exactly well-defined, but it seems to work here, and it is at least better than performing pointer arithmetic with an uninitialized pointer.

---
 attr.c                              |  2 +-
 blame.c                             |  4 ++--
 bloom.c                             |  2 +-
 builtin/describe.c                  |  2 +-
 builtin/difftool.c                  |  2 +-
 builtin/fast-import.c               |  2 +-
 builtin/sparse-checkout.c           |  6 +++---
 config.c                            |  2 +-
 merge-recursive.c                   | 10 +++++-----
 name-hash.c                         |  2 +-
 revision.c                          |  4 ++--
 submodule-config.c                  |  2 +-
 t/helper/test-hashmap.c             |  2 +-
 t/helper/test-lazy-init-name-hash.c |  4 ++--
 14 files changed, 23 insertions(+), 23 deletions(-)

index a826b2ef1f..b4fde37877 100644
@@ -160,7 +160,7 @@ static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check)
 	 * field and fill each entry with its corresponding git_attr.
 	 */
 	if (size != check->all_attrs_nr) {
-		struct attr_hash_entry *e;
+		struct attr_hash_entry *e = NULL;
 		struct hashmap_iter iter;
 
 		REALLOC_ARRAY(check->all_attrs, size);

index b475bfa1c0..e9879a772e 100644
@@ -450,7 +450,7 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b)
 {
 	int intersection = 0;
 	struct hashmap_iter iter;
-	const struct fingerprint_entry *entry_a, *entry_b;
+	const struct fingerprint_entry *entry_a, *entry_b = NULL;
 
 	hashmap_for_each_entry(&b->map, &iter, entry_b,
 				entry /* member name */) {

@@ -469,7 +469,7 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b)
 {
 	struct hashmap_iter iter;
 	struct fingerprint_entry *entry_a;
-	const struct fingerprint_entry *entry_b;
+	const struct fingerprint_entry *entry_b = NULL;
 
 	hashmap_iter_init(&b->map, &iter);
 

index 1a573226e7..ee45e9ccce 100644
@@ -221,7 +221,7 @@ struct bloom_filter *get_bloom_filter(struct repository *r,
 
 	if (diffopt.num_changes <= max_changes) {
 		struct hashmap pathmap;
-		struct pathmap_hash_entry *e;
+		struct pathmap_hash_entry *e = NULL;
 		struct hashmap_iter iter;
 		hashmap_init(&pathmap, pathmap_cmp, NULL, 0);
 

index 7668591d57..8b281cf426 100644
@@ -332,7 +332,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
 	if (!have_util) {
 		struct hashmap_iter iter;
 		struct commit *c;
-		struct commit_name *n;
+		struct commit_name *n = NULL;
 
 		init_commit_names(&commit_names);
 		hashmap_for_each_entry(&names, &iter, n,

index 7ac432b881..a1527ea01c 100644
@@ -344,7 +344,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 	FILE *fp;
 	struct hashmap working_tree_dups, submodules, symlinks2;
 	struct hashmap_iter iter;
-	struct pair_entry *entry;
+	struct pair_entry *entry = NULL;
 	struct index_state wtindex;
 	struct checkout lstate, rstate;
 	int rc, flags = RUN_GIT_CMD, err = 0;

index 1bf50a73dc..72154383c3 100644
@@ -498,7 +498,7 @@ static void invalidate_pack_id(unsigned int id)
 	unsigned long lu;
 	struct tag *t;
 	struct hashmap_iter iter;
-	struct object_entry *e;
+	struct object_entry *e = NULL;
 
 	hashmap_for_each_entry(&object_table, &iter, e, ent) {
 		if (e->pack_id == id)

index 4003f4d13a..fcd87da036 100644
@@ -67,7 +67,7 @@ static int sparse_checkout_list(int argc, const char **argv)
 
 	if (pl.use_cone_patterns) {
 		int i;
-		struct pattern_entry *pe;
+		struct pattern_entry *pe = NULL;
 		struct hashmap_iter iter;
 		struct string_list sl = STRING_LIST_INIT_DUP;
 

@@ -153,7 +153,7 @@ static char *escaped_pattern(char *pattern)
 static void write_cone_to_file(FILE *fp, struct pattern_list *pl)
 {
 	int i;
-	struct pattern_entry *pe;
+	struct pattern_entry *pe = NULL;
 	struct hashmap_iter iter;
 	struct string_list sl = STRING_LIST_INIT_DUP;
 	struct strbuf parent_pattern = STRBUF_INIT;

@@ -465,7 +465,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 				   struct pattern_list *pl)
 {
 	struct strbuf buffer = STRBUF_INIT;
-	struct pattern_entry *pe;
+	struct pattern_entry *pe = NULL;
 	struct hashmap_iter iter;
 	struct pattern_list existing;
 	char *sparse_filename = get_sparse_checkout_filename();

index 2bdff4457b..83c72dd6e6 100644
@@ -1953,7 +1953,7 @@ void git_configset_init(struct config_set *cs)
 
 void git_configset_clear(struct config_set *cs)
 {
-	struct config_set_element *entry;
+	struct config_set_element *entry = NULL;
 	struct hashmap_iter iter;
 	if (!cs->hash_initialized)
 		return;

index d0214335a7..11ea550b0d 100644
@@ -2151,8 +2151,8 @@ static void handle_directory_level_conflicts(struct merge_options *opt,
 					     struct tree *merge)
 {
 	struct hashmap_iter iter;
-	struct dir_rename_entry *head_ent;
-	struct dir_rename_entry *merge_ent;
+	struct dir_rename_entry *head_ent = NULL;
+	struct dir_rename_entry *merge_ent = NULL;
 
 	struct string_list remove_from_head = STRING_LIST_INIT_NODUP;
 	struct string_list remove_from_merge = STRING_LIST_INIT_NODUP;

@@ -2221,7 +2221,7 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs)
 {
 	struct hashmap *dir_renames;
 	struct hashmap_iter iter;
-	struct dir_rename_entry *entry;
+	struct dir_rename_entry *entry = NULL;
 	int i;
 
 	/*

@@ -2590,7 +2590,7 @@ static struct string_list *get_renames(struct merge_options *opt,
 	int i;
 	struct hashmap collisions;
 	struct hashmap_iter iter;
-	struct collision_entry *e;
+	struct collision_entry *e = NULL;
 	struct string_list *renames;
 
 	compute_collisions(&collisions, dir_renames, pairs);

@@ -2862,7 +2862,7 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,
 				   struct hashmap *dir_renames)
 {
 	struct hashmap_iter iter;
-	struct dir_rename_entry *e;
+	struct dir_rename_entry *e = NULL;
 
 	hashmap_for_each_entry(dir_renames, &iter, e,
 				ent /* member name */) {

index fb526a3775..a3f710b2f8 100644
@@ -706,7 +706,7 @@ void adjust_dirname_case(struct index_state *istate, char *name)
 
 struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase)
 {
-	struct cache_entry *ce;
+	struct cache_entry *ce = NULL;
 	unsigned int hash = memihash(name, namelen);
 
 	lazy_init_name_hash(istate);

index 067030e64c..232de3f6f5 100644
@@ -132,7 +132,7 @@ static void paths_and_oids_init(struct hashmap *map)
 static void paths_and_oids_clear(struct hashmap *map)
 {
 	struct hashmap_iter iter;
-	struct path_and_oids_entry *entry;
+	struct path_and_oids_entry *entry = NULL;
 
 	hashmap_for_each_entry(map, &iter, entry, ent /* member name */) {
 		oidset_clear(&entry->trees);

@@ -215,7 +215,7 @@ void mark_trees_uninteresting_sparse(struct repository *r,
 	unsigned has_interesting = 0, has_uninteresting = 0;
 	struct hashmap map;
 	struct hashmap_iter map_iter;
-	struct path_and_oids_entry *entry;
+	struct path_and_oids_entry *entry = NULL;
 	struct object_id *oid;
 	struct oidset_iter iter;
 

index c569e22aa3..662b9d9c09 100644
@@ -89,7 +89,7 @@ static void free_one_config(struct submodule_entry *entry)
 static void submodule_cache_clear(struct submodule_cache *cache)
 {
 	struct hashmap_iter iter;
-	struct submodule_entry *entry;
+	struct submodule_entry *entry = NULL;
 
 	if (!cache->initialized)
 		return;

index f38706216f..2bde90309b 100644
@@ -162,7 +162,7 @@ int cmd__hashmap(int argc, const char **argv)
 	while (strbuf_getline(&line, stdin) != EOF) {
 		char *cmd, *p1 = NULL, *p2 = NULL;
 		unsigned int hash = 0;
-		struct test_entry *entry;
+		struct test_entry *entry = NULL;
 
 		/* break line into command and up to two parameters */
 		cmd = strtok(line.buf, DELIM);

index cd1b4c9736..2cb1fa3d8c 100644
@@ -29,8 +29,8 @@ static void dump_run(void)
 		char name[FLEX_ARRAY];
 	};
 
-	struct dir_entry *dir;
-	struct cache_entry *ce;
+	struct dir_entry *dir = NULL;
+	struct cache_entry *ce = NULL;
 
 	read_cache();
 	if (single) {

gitgitgadget

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

Sigh.

Short of changing the signature of hashmap_put_entry() and friends to also take the type of these variables, I do not see any kosher way to reimplement the users of OFFSETOF_VAR() to help compilers without typeof() offhand. As a one-time annotation, the unfortunate noise we see in this patch may be tolerable, but what may make this approach unsustainable is that average programmers would not know, without compiling with that particular compiler, if their new variable that points at a hash_entry needs to have an oterwise unnecessary initialization. Also the variables that are left uninitialized by this patch may later require such an initialization.

Of course, it does not help that this workaround relies on an undefined behaviour, as you pointed out.

Whoa, wait. If it is just that macro, can we perhaps do something like the attached patch?

 hashmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index ef220de4c6..49cd8a8e92 100644
@@ -449,7 +449,7 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map,
  * containing a @member which is a "struct hashmap_entry"
  */
 #define hashmap_for_each_entry(map, iter, var, member) \
-	for (var = hashmap_iter_first_entry_offset(map, iter, \
+	for (var = NULL, var = hashmap_iter_first_entry_offset(map, iter, \
 						OFFSETOF_VAR(var, member)); \
 		var; \
 		var = hashmap_iter_next_entry_offset(iter, \

I looked at all the uses of OFFSETOF_VAR() and I think the one used for hashmap_for_each_entry() is the only instance that 'var' given to it can legitimately be uninitialized, if typeof() were available.

Here are the findings.

#define hashmap_put_entry(map, keyvar, member) \
	container_of_or_null_offset(hashmap_put(map, &(keyvar)->member), \
				OFFSETOF_VAR(keyvar, member))

The keyvar is a pointer to the entry being placed in the map; it must hold a valid one so the pointer-diff implementation of OFFSETOF_VAR() should work fine, or we are putting garbage in to the map.

#define hashmap_remove_entry(map, keyvar, member, keydata) \
	container_of_or_null_offset( \
			hashmap_remove(map, &(keyvar)->member, keydata), \
			OFFSETOF_VAR(keyvar, member))

The keyvar is used to match against an existing entry in the map to be removed---it must have a valid value.

#define hashmap_for_each_entry(map, iter, var, member) \
	for (var = hashmap_iter_first_entry_offset(map, iter, \
						OFFSETOF_VAR(var, member)); \
		var; \
		var = hashmap_iter_next_entry_offset(iter, \
						OFFSETOF_VAR(var, member)))

This, as you discovered, can be fed an uninitialized var and the first thing it does is to use OFFSETOF_VAR() on it in order to call hashmap_iter_first_entry_offset(). After that, i.e. when we called that function to start the loop, var is defined and we would be OK.

The trick I suggested is to initialize var to NULL before making the call to hashmap_iter_first_entry_offset(), i.e.

	for (var = NULL, \
	     var = hashmap_iter_first_entry_offset(map, iter, \
						OFFSETOF_VAR(var, member)); \

#define hashmap_get_entry(map, keyvar, member, keydata) \
	container_of_or_null_offset( \
				hashmap_get(map, &(keyvar)->member, keydata), \
				OFFSETOF_VAR(keyvar, member))

Must be OK for the same reason _put_entry() is OK.

#define hashmap_get_next_entry(map, var, member) \
	container_of_or_null_offset(hashmap_get_next(map, &(var)->member), \
				OFFSETOF_VAR(var, member))

This tries to go to the next-equal-pointer starting from var, so var must be valid already.

So, perhaps the attached may be a viable replacement that would be more futureproof with less maintenance cost, I suspect.

Thanks.

--- >8 ----- cut here ----- >8 ---
Subject: hashmap_for_each_entry(): workaround MSVC's runtime check failure #3

The OFFSETOF_VAR(var, member) macro is implemented in terms of offsetof(typeof(*var), member) with compilers that know typeof(), but its fallback implemenation compares &(var->member) and (var) and count the distance in bytes, i.e.

    ((uintptr_t)&(var)->member - (uintptr_t)(var))

MSVC's runtime check, when fed an uninitialized 'var', flags this as a use of an uninitialized variable (and that is legit---uninitialized contents of 'var' is subtracted) in a debug build.

After auditing all 6 uses of OFFSETOF_VAR(), 1 of them does feed a potentially uninitialized 'var' to the macro in the beginning of the for() loop:

    #define hashmap_for_each_entry(map, iter, var, member) \
            for (var = hashmap_iter_first_entry_offset(map, iter, \
                                                    OFFSETOF_VAR(var, member)); \
                    var; \
                    var = hashmap_iter_next_entry_offset(iter, \
                                                    OFFSETOF_VAR(var, member)))

We can work around this by making sure that var has _some_ value when OFFSETOF_VAR() is called. Strictly speaking, it invites undefined behaviour to use NULL here if we end up with pointer comparison, but MSVC runtime seems to be happy with it, and most other systems have typeof() and don't even need pointer comparison fallback code.

---
 hashmap.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index ef220de4c6..b011b394fe 100644
@@ -449,7 +449,8 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map,
  * containing a @member which is a "struct hashmap_entry"
  */
 #define hashmap_for_each_entry(map, iter, var, member) \
-	for (var = hashmap_iter_first_entry_offset(map, iter, \
+	for (var = NULL, /* for systems without typeof */ \
+	     var = hashmap_iter_first_entry_offset(map, iter, \
 						OFFSETOF_VAR(var, member)); \
 		var; \
 		var = hashmap_iter_next_entry_offset(iter, \

Hi Junio,

Thank you for doing all that leg work. TBH I didn't even think about looking further, after having run a couple tests manually that I thought were exhaustive in exercising this type of code pattern.

Definitely much nicer to maintain, and easier to verify. In my hands, this works better than my manual touch-ups of _all_ the call sites. So I replaced my patch with yours (adding your SOB).

Ciao,
Dscho

The intention of this patch series is to make it as easy as possible for a Visual Studio user to start building Git from scratch. The idea is to

* clone Git's source code,
* open it in Visual Studio via File>Open>Folder...
* let CMake configure things (and vcpkg build the dependencies, which will take a while), and
* start the build.

It does require a Git for Windows to be installed (so that the source can be cloned, but also to run the shell script parts of the CMake configuration), and also to run the tests.

This patch series replaces the patch "Ignore files generated by CMake" I sent via https://github.com/gitgitgadget/git/pull/735.

Changes since v1:

* Touched up a couple commit messages in response to reviewer comments
* Adjusted the instructions at the top of CMakeLists.txt so that the part talking about running CMake manually also talks about using contrib/buildsystems/out/ as build directory (in line with the part talking about Visual Studio's out-of-the-box experience)
* Replaced patch 10/10 with Junio's much more elegant solution of editing the hashmap_for_each_entry() macro instead of all of its callers.

Johannes Schindelin (9):
cmake: ignore files generated by CMake as run in Visual Studio
cmake: do find Git for Windows' shell interpreter
cmake: ensure that the vcpkg packages are found on Windows
cmake: fall back to using vcpkg's msgfmt.exe on Windows
cmake: quote the path accurately when editing test-lib.sh cmake (Windows): let the .dll files be found when running the tests cmake (Windows): complain when encountering an unknown compiler cmake (Windows): initialize vcpkg/build dependencies automatically cmake (Windows): recommend using Visual Studio's built-in CMake
support

Junio C Hamano (1):
hashmap_for_each_entry(): workaround MSVC's runtime check failure #3

 .gitignore                          |  1 +
 contrib/buildsystems/CMakeLists.txt | 57 ++++++++++++++++++++++-------
 hashmap.h                           |  3 +-
 3 files changed, 47 insertions(+), 14 deletions(-)

base-commit: e1cfff676549cdcd702cbac105468723ef2722f4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-738%2Fdscho%2Fcmake-and-visual-studio-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-738/dscho/cmake-and-visual-studio-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/738

Range-diff vs v1:

  1:  ba314373e9 =  1:  ba314373e9 cmake: ignore files generated by CMake as run in Visual Studio
  2:  05b4b69fee =  2:  05b4b69fee cmake: do find Git for Windows' shell interpreter
  3:  5967822a0c =  3:  5967822a0c cmake: ensure that the `vcpkg` packages are found on Windows
  4:  efc93f1dd9 =  4:  efc93f1dd9 cmake: fall back to using `vcpkg`'s `msgfmt.exe` on Windows
  5:  8b3500a07c =  5:  8b3500a07c cmake: quote the path accurately when editing `test-lib.sh`
  6:  bb8f122cde !  6:  88abbd9a78 cmake (Windows): let the `.dll` files are found when running the tests

@@ Metadata
Author: Johannes Schindelin <[email protected]>

## Commit message ##
- cmake (Windows): let the .dll files are found when running the tests
+ cmake (Windows): let the .dll files be found when running the tests

Contrary to Unix-ish platforms, the dependencies' shared libraries are not usually found in one central place. In our case, since we use
7: dddfa3a9b4 ! 7: 26ebeecb14 cmake (Windows): complain when encountering an unknown compiler
@@ Commit message
We have some custom handling regarding the link options, which are specific to each compiler.

- Therefore: let's not just continue without setting the link options when
+ Therefore: let's not just continue without setting the link options if
configuring for a currently unhandled compiler, but error out.

- Note: for some reason, the invalidcontinue.obj trick seems not to work
- in this developer's hands when building Git in Debug mode: the assertion
- dialog that is supposed to be suppressed still pops up, complaining
- about the entry variable being uninitialized when it is used in
- config.c's line 1961.
-

## contrib/buildsystems/CMakeLists.txt ##

  8:  3a07bd4916 =  8:  3587ee5e8d cmake (Windows): initialize vcpkg/build dependencies automatically
  9:  f07e421666 !  9:  68a7d984ec cmake (Windows): recommend using Visual Studio's built-in CMake support

@@ Commit message
configuration in CMake manually (does not matter whether using the command-line or CMake's GUI).

+ While at it, recommend using contrib/buildsystems/out/ as build
+ directory also in the part that talks about running CMake manually.
+
+ Helped-by: Junio C Hamano <[email protected]>

## contrib/buildsystems/CMakeLists.txt ##
@@ contrib/buildsystems/CMakeLists.txt

-Instructions to run CMake:
+Instructions how to use this in Visual Studio:
-+
+
+-cmake relative-path-to-CMakeLists.txt -DCMAKE_BUILD_TYPE=Release +-Eg.
+-From the root of git source tree
+- cmake contrib/buildsystems/
+-This will build the git binaries at the root
+Open the worktree as a folder. Visual Studio 2019 and later will detect +the CMake configuration automatically and set everything up for you, +ready to build. You can then run the tests in t/ via a regular Git Bash.
-+
-+Note: Visual Studio also has the option of opening the CMake configuration +
+-For out of source builds, say build in 'git/git-build/'
+- mkdir git-build;cd git-build; cmake ../contrib/buildsystems/
+-This will build the git binaries in git-build directory ++Note: Visual Studio also has the option of opening CMakeLists.txt
+directly; Using this option, Visual Studio will not find the source code, +though, therefore the File>Open>Folder... option is preferred. +
+Instructions to run CMake manually:
++
++ mkdir -p contrib/buildsystems/out
++ cd contrib/buildsystems/out
++ cmake ../ -DCMAKE_BUILD_TYPE=Release
++
++This will build the git binaries in contrib/buildsystems/out ++directory (our top-level .gitignore file knows to ignore contents of ++this directory).

- cmake relative-path-to-CMakeLists.txt -DCMAKE_BUILD_TYPE=Release
- Eg.
+ Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding
+ compiler flags

 10:  dc46d39611 <  -:  ---------- hashmap_for_each_entry(): work around MSVC's run-time check failure #3
  -:  ---------- > 10:  152705bbc6 hashmap_for_each_entry(): workaround MSVC's runtime check failure #3

gitgitgadget

From: Johannes Schindelin <[email protected]>

As of recent Visual Studio versions, CMake support is built-in:
https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

All that needs to be done is to open the worktree as a folder, and Visual Studio will find the CMakeLists.txt file and automatically generate the project files.

Let's ignore the entirety of those generated files.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

index d0f692a355..6521f3cf22 100644
@@ -241,3 +241,4 @@ Release/
 /git.VC.VC.opendb
 /git.VC.db
 *.dSYM
+/contrib/buildsystems/out

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, Git for Windows does not install its sh.exe into the PATH. However, our current CMakeLists.txt expects to find a shell interpreter in the PATH.

So let's fall back to looking in the default location where Git for Windows _does_ install a relatively convenient sh.exe:
C:\Program Files\Git\bin\sh.exe

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

index 5007f173f1..d14fa4f3dc 100644
@@ -43,8 +43,11 @@ set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
 find_program(SH_EXE sh)
 if(NOT SH_EXE)
-	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
-			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+	set(SH_EXE "C:/Program Files/Git/bin/sh.exe")
+	if(NOT EXISTS ${SH_EXE})
+		message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
+				"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+	endif()
 endif()
 
 #Create GIT-VERSION-FILE using GIT-VERSION-GEN

gitgitgadget

Hi Øystein,

Thank you, I was not aware of this neat feature.

Indeed. This is what I get in that case:

-- snip --
1> [CMake] Generating GIT-VERSION-FILE
1> [CMake] C:/git-sdk-64/usr/src/vs2017-test/contrib/buildsystems/../../GIT-VERSION-GEN: line 24: sed: command not found 1> [CMake] C:/git-sdk-64/usr/src/vs2017-test/contrib/buildsystems/../../GIT-VERSION-GEN: line 29: expr: command not found 1> [CMake] GIT_VERSION =
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:79 (string):
1> [CMake] string sub-command FIND requires 3 or 4 parameters.
1> [CMake]
1> [CMake]
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:83 (string):
1> [CMake] string sub-command REGEX, mode MATCH needs at least 5 arguments total to
1> [CMake] command.
1> [CMake]
1> [CMake]
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:87 (project):
1> [CMake] VERSION ".0" format invalid.
-- snap --

The explanation is pretty simple: you cannot just call into sh.exe via an absolute path and expect it to add its containing directory to the PATH. It does not, and the symptom is that neither sed nor expr are found.

One solution is to add it to the PATH manually, which is the original expectation in our CMakeLists.txt version.

Another solution is to point it to C:\Program Files\Git\bin\sh.exe which is not, in fact, a shell, but a small wrapper executable whose job it is to set up a couple environment variables (PATH being one of them) and then spawning the _actual_ sh.exe. The source code for that wrapper:
https://github.com/git-for-windows/MINGW-packages/blob/main/mingw-w64-git/git-wrapper.c

As you figured out, it is _not_ enough to use ...\usr\bin\sh.exe directly without adjusting the PATH.

Ciao,
Dscho

From: Johannes Schindelin <[email protected]>

On Windows, we use the vcpkg project to manage the dependencies, via compat/vcbuild/. Let's make sure that these dependencies are found by default.

This is needed because we are about to recommend loading the Git worktree as a folder into Visual Studio, relying on the automatic CMake support (which would make it relatively cumbersome to adjust the search path used by CMake manually).

---
 contrib/buildsystems/CMakeLists.txt | 7 +++++++
 1 file changed, 7 insertions(+)

index d14fa4f3dc..16014ac4f7 100644
@@ -40,6 +40,13 @@ cmake_minimum_required(VERSION 3.14)
 
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
+if(WIN32)
+	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
+
+	# In the vcpkg edition, we need this to be able to link to libcurl
+	set(CURL_NO_CURL_CMAKE ON)
+endif()
 
 find_program(SH_EXE sh)
 if(NOT SH_EXE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We are already relying on vcpkg to manage our dependencies, including libiconv. Let's also use the msgfmt.exe from there.

---
 contrib/buildsystems/CMakeLists.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 16014ac4f7..acb197fba5 100644
@@ -155,7 +155,11 @@ endif()
 
 find_program(MSGFMT_EXE msgfmt)
 if(NOT MSGFMT_EXE)
-	message(WARNING "Text Translations won't be build")
+	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
+	if(NOT EXISTS ${MSGFMT_EXE})
+		message(WARNING "Text Translations won't be build")
+		unset(MSGFMT_EXE)
+	endif()
 endif()
 
 #Force all visual studio outputs to CMAKE_BINARY_DIR

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, the build directory will be called something like contrib/buildsystems/out/build/x64-Debug (default) (note the space and the parentheses). We need to make sure that such a path is quoted properly when editing the assignment of the GIT_BUILD_DIR variable.

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index acb197fba5..e66ae137c2 100644
@@ -952,7 +952,7 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
-		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY\\\"/../${BUILD_DIR_RELATIVE}\" content \"\${content}\")\n"
+		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
 		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
 	#misc copies
 	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)

gitgitgadget

From: Johannes Schindelin <[email protected]>

Contrary to Unix-ish platforms, the dependencies' shared libraries are not usually found in one central place. In our case, since we use vcpkg, they are to be found inside the compat/vcbuild/vcpkg/ tree.

Let's make sure that they are in the search path when running the tests.

---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

index e66ae137c2..5a4e0ccb01 100644
@@ -942,6 +942,9 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n")
+if(WIN32)
+	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n")
+endif()
 
 #Make the tests work when building out of the source tree
 get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We have some custom handling regarding the link options, which are specific to each compiler.

Therefore: let's not just continue without setting the link options if configuring for a currently unhandled compiler, but error out.

---
 contrib/buildsystems/CMakeLists.txt | 2 ++
 1 file changed, 2 insertions(+)

index 5a4e0ccb01..d21849b730 100644
@@ -620,6 +620,8 @@ if(WIN32)
 		target_link_options(common-main PUBLIC -municode -Wl,-nxcompat -Wl,-dynamicbase -Wl,-entry:wmainCRTStartup -Wl,invalidcontinue.obj)
 	elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 		target_link_options(common-main PUBLIC /IGNORE:4217 /IGNORE:4049 /NOLOGO /ENTRY:wmainCRTStartup /SUBSYSTEM:CONSOLE invalidcontinue.obj)
+	else()
+		message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}")
 	endif()
 elseif(UNIX)
 	target_link_libraries(common-main pthread rt)

gitgitgadget

From: Johannes Schindelin <[email protected]>

The idea of having CMake support in Git's source tree is to enable contributors on Windows to start contributing with little effort. To that end, we just added some sensible defaults that will let users open the worktree in Visual Studio and start building.

This expects the dependencies (such as zlib) to be available already, though. If they are not available, we expect the user to run compat/vcbuild/vcpkg_install.bat.

Rather than requiring this step to be manual, detect the situation and run it as part of the CMake configuration step.

This concludes our journey to make it as effortless as possible to start developing Git in Visual Studio: all the developer needs to do is to clone Git's repository, open the worktree via File>Open>Folder... and wait for CMake to finish configuring.

---
 contrib/buildsystems/CMakeLists.txt | 4 ++++
 1 file changed, 4 insertions(+)

index d21849b730..1eaeb8b8e0 100644
@@ -42,6 +42,10 @@ cmake_minimum_required(VERSION 3.14)
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 if(WIN32)
 	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	if(NOT EXISTS ${VCPKG_DIR})
+		message("Initializinge vcpkg and building the Git's dependencies (this will take a while...)")
+		execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat)
+	endif()
 	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
 
 	# In the vcpkg edition, we need this to be able to link to libcurl

gitgitgadget

From: Johannes Schindelin <[email protected]>

It is a lot more convenient to use than having to specify the configuration in CMake manually (does not matter whether using the command-line or CMake's GUI).

While at it, recommend using contrib/buildsystems/out/ as build directory also in the part that talks about running CMake manually.

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

index 1eaeb8b8e0..de92e4fe2d 100644
@@ -4,17 +4,25 @@
 
 #[[
 
-Instructions to run CMake:
+Instructions how to use this in Visual Studio:
 
-cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
-Eg.
-From the root of git source tree
-	`cmake contrib/buildsystems/ `
-This will build the git binaries at the root
+Open the worktree as a folder. Visual Studio 2019 and later will detect
+the CMake configuration automatically and set everything up for you,
+ready to build. You can then run the tests in `t/` via a regular Git Bash.
 
-For out of source builds, say build in 'git/git-build/'
-	`mkdir git-build;cd git-build; cmake ../contrib/buildsystems/`
-This will build the git binaries in git-build directory
+Note: Visual Studio also has the option of opening `CMakeLists.txt`
+directly; Using this option, Visual Studio will not find the source code,
+though, therefore the `File>Open>Folder...` option is preferred.
+
+Instructions to run CMake manually:
+
+    mkdir -p contrib/buildsystems/out
+    cd contrib/buildsystems/out
+    cmake ../ -DCMAKE_BUILD_TYPE=Release
+
+This will build the git binaries in contrib/buildsystems/out
+directory (our top-level .gitignore file knows to ignore contents of
+this directory).
 
 Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding
 compiler flags

gitgitgadget

From: Junio C Hamano <[email protected]>

The OFFSETOF_VAR(var, member) macro is implemented in terms of offsetof(typeof(*var), member) with compilers that know typeof(), but its fallback implemenation compares &(var->member) and (var) and count the distance in bytes, i.e.

    ((uintptr_t)&(var)->member - (uintptr_t)(var))

MSVC's runtime check, when fed an uninitialized 'var', flags this as a use of an uninitialized variable (and that is legit---uninitialized contents of 'var' is subtracted) in a debug build.

After auditing all 6 uses of OFFSETOF_VAR(), 1 of them does feed a potentially uninitialized 'var' to the macro in the beginning of the for() loop:

    #define hashmap_for_each_entry(map, iter, var, member) \
            for (var = hashmap_iter_first_entry_offset(map, iter, \
                                                    OFFSETOF_VAR(var, member)); \
                    var; \
                    var = hashmap_iter_next_entry_offset(iter, \
                                                    OFFSETOF_VAR(var, member)))

We can work around this by making sure that var has _some_ value when OFFSETOF_VAR() is called. Strictly speaking, it invites undefined behaviour to use NULL here if we end up with pointer comparison, but MSVC runtime seems to be happy with it, and most other systems have typeof() and don't even need pointer comparison fallback code.

---
 hashmap.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index ef220de4c6..b011b394fe 100644
@@ -449,7 +449,8 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map,
  * containing a @member which is a "struct hashmap_entry"
  */
 #define hashmap_for_each_entry(map, iter, var, member) \
-	for (var = hashmap_iter_first_entry_offset(map, iter, \
+	for (var = NULL, /* for systems without typeof */ \
+	     var = hashmap_iter_first_entry_offset(map, iter, \
 						OFFSETOF_VAR(var, member)); \
 		var; \
 		var = hashmap_iter_next_entry_offset(iter, \

gitgitgadget

The intention of this patch series is to make it as easy as possible for a Visual Studio user to start building Git from scratch. The idea is to

* clone Git's source code,
* open it in Visual Studio via File>Open>Folder...
* let CMake configure things (and vcpkg build the dependencies, which will take a while), and
* start the build.

It does require a Git for Windows to be installed so that the source can be cloned, but also to run the shell script parts of the CMake configuration, and also to run the tests.

This patch series replaces the patch "Ignore files generated by CMake" I sent via https://github.com/gitgitgadget/git/pull/735.

Changes since v2:

* Fixed a typo in the message when msgfmt.exe is not found.
* Simplified the logic to find sh.exe (thanks Øystein).

Changes since v1:

* Touched up a couple commit messages in response to reviewer comments
* Adjusted the instructions at the top of CMakeLists.txt so that the part talking about running CMake manually also talks about using contrib/buildsystems/out/ as build directory (in line with the part talking about Visual Studio's out-of-the-box experience)
* Replaced patch 10/10 with Junio's much more elegant solution of editing the hashmap_for_each_entry() macro instead of all of its callers.

Johannes Schindelin (10):
cmake: ignore files generated by CMake as run in Visual Studio
cmake: do find Git for Windows' shell interpreter
cmake: ensure that the vcpkg packages are found on Windows
cmake: fall back to using vcpkg's msgfmt.exe on Windows
cmake: quote the path accurately when editing test-lib.sh cmake (Windows): let the .dll files be found when running the tests cmake (Windows): complain when encountering an unknown compiler cmake (Windows): initialize vcpkg/build dependencies automatically cmake (Windows): recommend using Visual Studio's built-in CMake
support
cmake: fix typo in message when msgfmt was not found

Junio C Hamano (1):
hashmap_for_each_entry(): workaround MSVC's runtime check failure #3

 .gitignore                          |  1 +
 contrib/buildsystems/CMakeLists.txt | 52 ++++++++++++++++++++++-------
 hashmap.h                           |  3 +-
 3 files changed, 43 insertions(+), 13 deletions(-)

base-commit: 9bc233ae1cf19a49e51842c7959d80a675dbd1c0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-738%2Fdscho%2Fcmake-and-visual-studio-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-738/dscho/cmake-and-visual-studio-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/738

Range-diff vs v2:

  1:  ba314373e9 =  1:  9c7a964519 cmake: ignore files generated by CMake as run in Visual Studio
  2:  05b4b69fee <  -:  ---------- cmake: do find Git for Windows' shell interpreter
  -:  ---------- >  2:  3b467360bb cmake: do find Git for Windows' shell interpreter
  3:  5967822a0c !  3:  a49b28e001 cmake: ensure that the `vcpkg` packages are found on Windows

@@ contrib/buildsystems/CMakeLists.txt: cmake_minimum_required(VERSION 3.14)
+ set(CURL_NO_CURL_CMAKE ON)
+endif()

- find_program(SH_EXE sh)
+ find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin") if(NOT SH_EXE)

  4:  efc93f1dd9 =  4:  f1baf0f974 cmake: fall back to using `vcpkg`'s `msgfmt.exe` on Windows
  5:  8b3500a07c =  5:  8380aa55ab cmake: quote the path accurately when editing `test-lib.sh`
  6:  88abbd9a78 =  6:  f61c3ef442 cmake (Windows): let the `.dll` files be found when running the tests
  7:  26ebeecb14 =  7:  7a4346a437 cmake (Windows): complain when encountering an unknown compiler
  8:  3587ee5e8d =  8:  7c1601c7e3 cmake (Windows): initialize vcpkg/build dependencies automatically
  9:  68a7d984ec =  9:  ec182738f0 cmake (Windows): recommend using Visual Studio's built-in CMake support
 10:  152705bbc6 = 10:  38cef4deb5 hashmap_for_each_entry(): workaround MSVC's runtime check failure #3
  -:  ---------- > 11:  c3e235fe39 cmake: fix typo in message when `msgfmt` was not found

gitgitgadget

From: Johannes Schindelin <[email protected]>

As of recent Visual Studio versions, CMake support is built-in:
https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

All that needs to be done is to open the worktree as a folder, and Visual Studio will find the CMakeLists.txt file and automatically generate the project files.

Let's ignore the entirety of those generated files.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

index 800b812c6a..6232d33924 100644
@@ -242,3 +242,4 @@ Release/
 /git.VC.VC.opendb
 /git.VC.db
 *.dSYM
+/contrib/buildsystems/out

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, Git for Windows does not install its sh.exe into the PATH. However, our current CMakeLists.txt expects to find a shell interpreter in the PATH.

So let's fall back to looking in the default location where Git for Windows _does_ install a relatively convenient sh.exe:
C:\Program Files\Git\bin\sh.exe

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 5007f173f1..1847a6fd58 100644
@@ -41,7 +41,7 @@ cmake_minimum_required(VERSION 3.14)
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
-find_program(SH_EXE sh)
+find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
 if(NOT SH_EXE)
 	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
 			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")

gitgitgadget

From: Johannes Schindelin <[email protected]>

On Windows, we use the vcpkg project to manage the dependencies, via compat/vcbuild/. Let's make sure that these dependencies are found by default.

This is needed because we are about to recommend loading the Git worktree as a folder into Visual Studio, relying on the automatic CMake support (which would make it relatively cumbersome to adjust the search path used by CMake manually).

---
 contrib/buildsystems/CMakeLists.txt | 7 +++++++
 1 file changed, 7 insertions(+)

index 1847a6fd58..12268f61ba 100644
@@ -40,6 +40,13 @@ cmake_minimum_required(VERSION 3.14)
 
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
+if(WIN32)
+	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
+
+	# In the vcpkg edition, we need this to be able to link to libcurl
+	set(CURL_NO_CURL_CMAKE ON)
+endif()
 
 find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
 if(NOT SH_EXE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We are already relying on vcpkg to manage our dependencies, including libiconv. Let's also use the msgfmt.exe from there.

---
 contrib/buildsystems/CMakeLists.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 12268f61ba..683cf38263 100644
@@ -152,7 +152,11 @@ endif()
 
 find_program(MSGFMT_EXE msgfmt)
 if(NOT MSGFMT_EXE)
-	message(WARNING "Text Translations won't be build")
+	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
+	if(NOT EXISTS ${MSGFMT_EXE})
+		message(WARNING "Text Translations won't be build")
+		unset(MSGFMT_EXE)
+	endif()
 endif()
 
 #Force all visual studio outputs to CMAKE_BINARY_DIR

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, the build directory will be called something like contrib/buildsystems/out/build/x64-Debug (default) (note the space and the parentheses). We need to make sure that such a path is quoted properly when editing the assignment of the GIT_BUILD_DIR variable.

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 683cf38263..1999db0c6b 100644
@@ -949,7 +949,7 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
-		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY\\\"/../${BUILD_DIR_RELATIVE}\" content \"\${content}\")\n"
+		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
 		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
 	#misc copies
 	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)

gitgitgadget

From: Johannes Schindelin <[email protected]>

Contrary to Unix-ish platforms, the dependencies' shared libraries are not usually found in one central place. In our case, since we use vcpkg, they are to be found inside the compat/vcbuild/vcpkg/ tree.

Let's make sure that they are in the search path when running the tests.

---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

index 1999db0c6b..361fe3de6d 100644
@@ -939,6 +939,9 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n")
+if(WIN32)
+	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n")
+endif()
 
 #Make the tests work when building out of the source tree
 get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We have some custom handling regarding the link options, which are specific to each compiler.

Therefore: let's not just continue without setting the link options if configuring for a currently unhandled compiler, but error out.

---
 contrib/buildsystems/CMakeLists.txt | 2 ++
 1 file changed, 2 insertions(+)

index 361fe3de6d..13d958f5a6 100644
@@ -617,6 +617,8 @@ if(WIN32)
 		target_link_options(common-main PUBLIC -municode -Wl,-nxcompat -Wl,-dynamicbase -Wl,-entry:wmainCRTStartup -Wl,invalidcontinue.obj)
 	elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 		target_link_options(common-main PUBLIC /IGNORE:4217 /IGNORE:4049 /NOLOGO /ENTRY:wmainCRTStartup /SUBSYSTEM:CONSOLE invalidcontinue.obj)
+	else()
+		message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}")
 	endif()
 elseif(UNIX)
 	target_link_libraries(common-main pthread rt)

gitgitgadget

From: Johannes Schindelin <[email protected]>

The idea of having CMake support in Git's source tree is to enable contributors on Windows to start contributing with little effort. To that end, we just added some sensible defaults that will let users open the worktree in Visual Studio and start building.

This expects the dependencies (such as zlib) to be available already, though. If they are not available, we expect the user to run compat/vcbuild/vcpkg_install.bat.

Rather than requiring this step to be manual, detect the situation and run it as part of the CMake configuration step.

This concludes our journey to make it as effortless as possible to start developing Git in Visual Studio: all the developer needs to do is to clone Git's repository, open the worktree via File>Open>Folder... and wait for CMake to finish configuring.

---
 contrib/buildsystems/CMakeLists.txt | 4 ++++
 1 file changed, 4 insertions(+)

index 13d958f5a6..6a92ec9763 100644
@@ -42,6 +42,10 @@ cmake_minimum_required(VERSION 3.14)
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 if(WIN32)
 	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	if(NOT EXISTS ${VCPKG_DIR})
+		message("Initializinge vcpkg and building the Git's dependencies (this will take a while...)")
+		execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat)
+	endif()
 	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
 
 	# In the vcpkg edition, we need this to be able to link to libcurl

gitgitgadget

Hi Sibi,

Aw shucks! Thank you for being thorough,
Dscho

From: Johannes Schindelin <[email protected]>

It is a lot more convenient to use than having to specify the configuration in CMake manually (does not matter whether using the command-line or CMake's GUI).

While at it, recommend using contrib/buildsystems/out/ as build directory also in the part that talks about running CMake manually.

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

index 6a92ec9763..4b16744847 100644
@@ -4,17 +4,25 @@
 
 #[[
 
-Instructions to run CMake:
+Instructions how to use this in Visual Studio:
 
-cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
-Eg.
-From the root of git source tree
-	`cmake contrib/buildsystems/ `
-This will build the git binaries at the root
+Open the worktree as a folder. Visual Studio 2019 and later will detect
+the CMake configuration automatically and set everything up for you,
+ready to build. You can then run the tests in `t/` via a regular Git Bash.
 
-For out of source builds, say build in 'git/git-build/'
-	`mkdir git-build;cd git-build; cmake ../contrib/buildsystems/`
-This will build the git binaries in git-build directory
+Note: Visual Studio also has the option of opening `CMakeLists.txt`
+directly; Using this option, Visual Studio will not find the source code,
+though, therefore the `File>Open>Folder...` option is preferred.
+
+Instructions to run CMake manually:
+
+    mkdir -p contrib/buildsystems/out
+    cd contrib/buildsystems/out
+    cmake ../ -DCMAKE_BUILD_TYPE=Release
+
+This will build the git binaries in contrib/buildsystems/out
+directory (our top-level .gitignore file knows to ignore contents of
+this directory).
 
 Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding
 compiler flags

gitgitgadget

From: Junio C Hamano <[email protected]>

The OFFSETOF_VAR(var, member) macro is implemented in terms of offsetof(typeof(*var), member) with compilers that know typeof(), but its fallback implemenation compares &(var->member) and (var) and count the distance in bytes, i.e.

    ((uintptr_t)&(var)->member - (uintptr_t)(var))

MSVC's runtime check, when fed an uninitialized 'var', flags this as a use of an uninitialized variable (and that is legit---uninitialized contents of 'var' is subtracted) in a debug build.

After auditing all 6 uses of OFFSETOF_VAR(), 1 of them does feed a potentially uninitialized 'var' to the macro in the beginning of the for() loop:

    #define hashmap_for_each_entry(map, iter, var, member) \
            for (var = hashmap_iter_first_entry_offset(map, iter, \
                                                    OFFSETOF_VAR(var, member)); \
                    var; \
                    var = hashmap_iter_next_entry_offset(iter, \
                                                    OFFSETOF_VAR(var, member)))

We can work around this by making sure that var has _some_ value when OFFSETOF_VAR() is called. Strictly speaking, it invites undefined behaviour to use NULL here if we end up with pointer comparison, but MSVC runtime seems to be happy with it, and most other systems have typeof() and don't even need pointer comparison fallback code.

---
 hashmap.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index ef220de4c6..b011b394fe 100644
@@ -449,7 +449,8 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map,
  * containing a @member which is a "struct hashmap_entry"
  */
 #define hashmap_for_each_entry(map, iter, var, member) \
-	for (var = hashmap_iter_first_entry_offset(map, iter, \
+	for (var = NULL, /* for systems without typeof */ \
+	     var = hashmap_iter_first_entry_offset(map, iter, \
 						OFFSETOF_VAR(var, member)); \
 		var; \
 		var = hashmap_iter_next_entry_offset(iter, \

gitgitgadget

From: Johannes Schindelin <[email protected]>

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 4b16744847..b9fced682e 100644
@@ -166,7 +166,7 @@ find_program(MSGFMT_EXE msgfmt)
 if(NOT MSGFMT_EXE)
 	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
 	if(NOT EXISTS ${MSGFMT_EXE})
-		message(WARNING "Text Translations won't be build")
+		message(WARNING "Text Translations won't be built")
 		unset(MSGFMT_EXE)
 	endif()
 endif()

gitgitgadget

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

Shouldn't this be squashed into the step being fixed, which is part of this series? Would it be the "fall back to using..." step (4/11)?

Hi Junio,

Oy. I did not even realize that it was _my_ typo. Fixed in my branch. I'll wait a little, just in case that anything else needs to be fixed, before sending the next iteration.

Ciao,
Dscho

The intention of this patch series is to make it as easy as possible for a Visual Studio user to start building Git from scratch. The idea is to

* clone Git's source code,
* open it in Visual Studio via File>Open>Folder...
* let CMake configure things (and vcpkg build the dependencies, which will take a while), and
* start the build.

It does require a Git for Windows to be installed so that the source can be cloned, but also to run the shell script parts of the CMake configuration, and also to run the tests.

This patch series replaces the patch "Ignore files generated by CMake" I sent via https://github.com/gitgitgadget/git/pull/735.

Changes since v3:

* Squashed the typo fix into the correct commit (d'oh!)
* Fixed another typo (thanks, Sibi!)
* Guarded the code block that initializes vcpkg and builds the dependencies behind the MSVC conditional (thanks, Sibi!)

Changes since v2:

* Fixed a typo in the message when msgfmt.exe is not found.
* Simplified the logic to find sh.exe (thanks Øystein).

Changes since v1:

* Touched up a couple commit messages in response to reviewer comments
* Adjusted the instructions at the top of CMakeLists.txt so that the part talking about running CMake manually also talks about using contrib/buildsystems/out/ as build directory (in line with the part talking about Visual Studio's out-of-the-box experience)
* Replaced patch 10/10 with Junio's much more elegant solution of editing the hashmap_for_each_entry() macro instead of all of its callers.

Johannes Schindelin (9):
cmake: ignore files generated by CMake as run in Visual Studio
cmake: do find Git for Windows' shell interpreter
cmake: ensure that the vcpkg packages are found on Windows
cmake: fall back to using vcpkg's msgfmt.exe on Windows
cmake: quote the path accurately when editing test-lib.sh cmake (Windows): let the .dll files be found when running the tests cmake (Windows): complain when encountering an unknown compiler cmake (Windows): initialize vcpkg/build dependencies automatically cmake (Windows): recommend using Visual Studio's built-in CMake
support

Junio C Hamano (1):
hashmap_for_each_entry(): workaround MSVC's runtime check failure #3

 .gitignore                          |  1 +
 contrib/buildsystems/CMakeLists.txt | 52 ++++++++++++++++++++++-------
 hashmap.h                           |  3 +-
 3 files changed, 43 insertions(+), 13 deletions(-)

base-commit: 9bc233ae1cf19a49e51842c7959d80a675dbd1c0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-738%2Fdscho%2Fcmake-and-visual-studio-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-738/dscho/cmake-and-visual-studio-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/738

Range-diff vs v3:

  1:  9c7a964519 =  1:  9c7a964519 cmake: ignore files generated by CMake as run in Visual Studio
  2:  3b467360bb =  2:  3b467360bb cmake: do find Git for Windows' shell interpreter
  3:  a49b28e001 =  3:  a49b28e001 cmake: ensure that the `vcpkg` packages are found on Windows
  4:  f1baf0f974 !  4:  56cfdb2ee9 cmake: fall back to using `vcpkg`'s `msgfmt.exe` on Windows
     @@ contrib/buildsystems/CMakeLists.txt: endif()
      -	message(WARNING "Text Translations won't be build")
      +	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
      +	if(NOT EXISTS ${MSGFMT_EXE})
     -+		message(WARNING "Text Translations won't be build")
     ++		message(WARNING "Text Translations won't be built")
      +		unset(MSGFMT_EXE)
      +	endif()
       endif()
  5:  8380aa55ab =  5:  cfedb1673a cmake: quote the path accurately when editing `test-lib.sh`
  6:  f61c3ef442 =  6:  d402f721c2 cmake (Windows): let the `.dll` files be found when running the tests
  7:  7a4346a437 =  7:  0e7fdfddb4 cmake (Windows): complain when encountering an unknown compiler
  8:  7c1601c7e3 !  8:  14ddfc1781 cmake (Windows): initialize vcpkg/build dependencies automatically

@@ Commit message
Rather than requiring this step to be manual, detect the situation and run it as part of the CMake configuration step.

+ Note that this obviously only applies to the scenario when we want to
+ compile in Visual Studio (i.e. with MS Visual C), not with GCC.
+ Therefore, we guard this new code block behind the MSVC conditional.
+
This concludes our journey to make it as effortless as possible to start developing Git in Visual Studio: all the developer needs to do is to clone Git's repository, open the worktree via File>Open>Folder... and
@@ Commit message

+ squash! cmake (Windows): initialize vcpkg/build dependencies automatically
+
+ MSVC
+
## contrib/buildsystems/CMakeLists.txt ##
@@ contrib/buildsystems/CMakeLists.txt: cmake_minimum_required(VERSION 3.14)
set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..) if(WIN32)
set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
-+ if(NOT EXISTS ${VCPKG_DIR})
-+ message("Initializinge vcpkg and building the Git's dependencies (this will take a while...)")
++ if(MSVC AND NOT EXISTS ${VCPKG_DIR})
++ message("Initializing vcpkg and building the Git's dependencies (this will take a while...)")
+ execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat)
+ endif()
list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")

  9:  ec182738f0 =  9:  b239162eb2 cmake (Windows): recommend using Visual Studio's built-in CMake support
 10:  38cef4deb5 = 10:  7643a766b7 hashmap_for_each_entry(): workaround MSVC's runtime check failure #3
 11:  c3e235fe39 <  -:  ---------- cmake: fix typo in message when `msgfmt` was not found

gitgitgadget

From: Johannes Schindelin <[email protected]>

As of recent Visual Studio versions, CMake support is built-in:
https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

All that needs to be done is to open the worktree as a folder, and Visual Studio will find the CMakeLists.txt file and automatically generate the project files.

Let's ignore the entirety of those generated files.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

index 800b812c6a..6232d33924 100644
@@ -242,3 +242,4 @@ Release/
 /git.VC.VC.opendb
 /git.VC.db
 *.dSYM
+/contrib/buildsystems/out

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, Git for Windows does not install its sh.exe into the PATH. However, our current CMakeLists.txt expects to find a shell interpreter in the PATH.

So let's fall back to looking in the default location where Git for Windows _does_ install a relatively convenient sh.exe:
C:\Program Files\Git\bin\sh.exe

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 5007f173f1..1847a6fd58 100644
@@ -41,7 +41,7 @@ cmake_minimum_required(VERSION 3.14)
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
-find_program(SH_EXE sh)
+find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
 if(NOT SH_EXE)
 	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
 			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")

gitgitgadget

From: Johannes Schindelin <[email protected]>

On Windows, we use the vcpkg project to manage the dependencies, via compat/vcbuild/. Let's make sure that these dependencies are found by default.

This is needed because we are about to recommend loading the Git worktree as a folder into Visual Studio, relying on the automatic CMake support (which would make it relatively cumbersome to adjust the search path used by CMake manually).

---
 contrib/buildsystems/CMakeLists.txt | 7 +++++++
 1 file changed, 7 insertions(+)

index 1847a6fd58..12268f61ba 100644
@@ -40,6 +40,13 @@ cmake_minimum_required(VERSION 3.14)
 
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
+if(WIN32)
+	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
+
+	# In the vcpkg edition, we need this to be able to link to libcurl
+	set(CURL_NO_CURL_CMAKE ON)
+endif()
 
 find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
 if(NOT SH_EXE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We are already relying on vcpkg to manage our dependencies, including libiconv. Let's also use the msgfmt.exe from there.

---
 contrib/buildsystems/CMakeLists.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 12268f61ba..02241dcc77 100644
@@ -152,7 +152,11 @@ endif()
 
 find_program(MSGFMT_EXE msgfmt)
 if(NOT MSGFMT_EXE)
-	message(WARNING "Text Translations won't be build")
+	set(MSGFMT_EXE ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg/downloads/tools/msys2/msys64/usr/bin/msgfmt.exe)
+	if(NOT EXISTS ${MSGFMT_EXE})
+		message(WARNING "Text Translations won't be built")
+		unset(MSGFMT_EXE)
+	endif()
 endif()
 
 #Force all visual studio outputs to CMAKE_BINARY_DIR

gitgitgadget

From: Johannes Schindelin <[email protected]>

By default, the build directory will be called something like contrib/buildsystems/out/build/x64-Debug (default) (note the space and the parentheses). We need to make sure that such a path is quoted properly when editing the assignment of the GIT_BUILD_DIR variable.

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 02241dcc77..3bb74337e1 100644
@@ -949,7 +949,7 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
 		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
-		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY\\\"/../${BUILD_DIR_RELATIVE}\" content \"\${content}\")\n"
+		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
 		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
 	#misc copies
 	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)

gitgitgadget

From: Johannes Schindelin <[email protected]>

Contrary to Unix-ish platforms, the dependencies' shared libraries are not usually found in one central place. In our case, since we use vcpkg, they are to be found inside the compat/vcbuild/vcpkg/ tree.

Let's make sure that they are in the search path when running the tests.

---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

index 3bb74337e1..55d0960002 100644
@@ -939,6 +939,9 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n")
+if(WIN32)
+	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n")
+endif()
 
 #Make the tests work when building out of the source tree
 get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)

gitgitgadget

From: Johannes Schindelin <[email protected]>

We have some custom handling regarding the link options, which are specific to each compiler.

Therefore: let's not just continue without setting the link options if configuring for a currently unhandled compiler, but error out.

---
 contrib/buildsystems/CMakeLists.txt | 2 ++
 1 file changed, 2 insertions(+)

index 55d0960002..78b90abe5e 100644
@@ -617,6 +617,8 @@ if(WIN32)
 		target_link_options(common-main PUBLIC -municode -Wl,-nxcompat -Wl,-dynamicbase -Wl,-entry:wmainCRTStartup -Wl,invalidcontinue.obj)
 	elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 		target_link_options(common-main PUBLIC /IGNORE:4217 /IGNORE:4049 /NOLOGO /ENTRY:wmainCRTStartup /SUBSYSTEM:CONSOLE invalidcontinue.obj)
+	else()
+		message(FATAL_ERROR "Unhandled compiler: ${CMAKE_C_COMPILER_ID}")
 	endif()
 elseif(UNIX)
 	target_link_libraries(common-main pthread rt)

gitgitgadget

From: Johannes Schindelin <[email protected]>

The idea of having CMake support in Git's source tree is to enable contributors on Windows to start contributing with little effort. To that end, we just added some sensible defaults that will let users open the worktree in Visual Studio and start building.

This expects the dependencies (such as zlib) to be available already, though. If they are not available, we expect the user to run compat/vcbuild/vcpkg_install.bat.

Rather than requiring this step to be manual, detect the situation and run it as part of the CMake configuration step.

Note that this obviously only applies to the scenario when we want to compile in Visual Studio (i.e. with MS Visual C), not with GCC. Therefore, we guard this new code block behind the MSVC conditional.

This concludes our journey to make it as effortless as possible to start developing Git in Visual Studio: all the developer needs to do is to clone Git's repository, open the worktree via File>Open>Folder... and wait for CMake to finish configuring.

squash! cmake (Windows): initialize vcpkg/build dependencies automatically

MSVC

---
 contrib/buildsystems/CMakeLists.txt | 4 ++++
 1 file changed, 4 insertions(+)

index 78b90abe5e..ebadae2271 100644
@@ -42,6 +42,10 @@ cmake_minimum_required(VERSION 3.14)
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 if(WIN32)
 	set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg")
+	if(MSVC AND NOT EXISTS ${VCPKG_DIR})
+		message("Initializing vcpkg and building the Git's dependencies (this will take a while...)")
+		execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat)
+	endif()
 	list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/x64-windows")
 
 	# In the vcpkg edition, we need this to be able to link to libcurl

gitgitgadget

Hi,

Aaargh! This was of course meant to be deleted after rewording the commit message :-(

Sorry about that,
Dscho

OK. I noticed it and what got pushed out has them removed.

Hopefully we can merge this round down to 'next' after waiting for a day or two to see if anything new gets noticed.

Thanks.

From: Johannes Schindelin <[email protected]>

It is a lot more convenient to use than having to specify the configuration in CMake manually (does not matter whether using the command-line or CMake's GUI).

While at it, recommend using contrib/buildsystems/out/ as build directory also in the part that talks about running CMake manually.

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

index ebadae2271..df539a44fa 100644
@@ -4,17 +4,25 @@
 
 #[[
 
-Instructions to run CMake:
+Instructions how to use this in Visual Studio:
 
-cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
-Eg.
-From the root of git source tree
-	`cmake contrib/buildsystems/ `
-This will build the git binaries at the root
+Open the worktree as a folder. Visual Studio 2019 and later will detect
+the CMake configuration automatically and set everything up for you,
+ready to build. You can then run the tests in `t/` via a regular Git Bash.
 
-For out of source builds, say build in 'git/git-build/'
-	`mkdir git-build;cd git-build; cmake ../contrib/buildsystems/`
-This will build the git binaries in git-build directory
+Note: Visual Studio also has the option of opening `CMakeLists.txt`
+directly; Using this option, Visual Studio will not find the source code,
+though, therefore the `File>Open>Folder...` option is preferred.
+
+Instructions to run CMake manually:
+
+    mkdir -p contrib/buildsystems/out
+    cd contrib/buildsystems/out
+    cmake ../ -DCMAKE_BUILD_TYPE=Release
+
+This will build the git binaries in contrib/buildsystems/out
+directory (our top-level .gitignore file knows to ignore contents of
+this directory).
 
 Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding
 compiler flags

gitgitgadget

From: Junio C Hamano <[email protected]>

The OFFSETOF_VAR(var, member) macro is implemented in terms of offsetof(typeof(*var), member) with compilers that know typeof(), but its fallback implemenation compares &(var->member) and (var) and count the distance in bytes, i.e.

    ((uintptr_t)&(var)->member - (uintptr_t)(var))

MSVC's runtime check, when fed an uninitialized 'var', flags this as a use of an uninitialized variable (and that is legit---uninitialized contents of 'var' is subtracted) in a debug build.

After auditing all 6 uses of OFFSETOF_VAR(), 1 of them does feed a potentially uninitialized 'var' to the macro in the beginning of the for() loop:

    #define hashmap_for_each_entry(map, iter, var, member) \
            for (var = hashmap_iter_first_entry_offset(map, iter, \
                                                    OFFSETOF_VAR(var, member)); \
                    var; \
                    var = hashmap_iter_next_entry_offset(iter, \
                                                    OFFSETOF_VAR(var, member)))

We can work around this by making sure that var has _some_ value when OFFSETOF_VAR() is called. Strictly speaking, it invites undefined behaviour to use NULL here if we end up with pointer comparison, but MSVC runtime seems to be happy with it, and most other systems have typeof() and don't even need pointer comparison fallback code.

---
 hashmap.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index ef220de4c6..b011b394fe 100644
@@ -449,7 +449,8 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map,
  * containing a @member which is a "struct hashmap_entry"
  */
 #define hashmap_for_each_entry(map, iter, var, member) \
-	for (var = hashmap_iter_first_entry_offset(map, iter, \
+	for (var = NULL, /* for systems without typeof */ \
+	     var = hashmap_iter_first_entry_offset(map, iter, \
 						OFFSETOF_VAR(var, member)); \
 		var; \
 		var = hashmap_iter_next_entry_offset(iter, \

gitgitgadget