Skip to contentSkip to conversation

Fix MSVC support, at long last

PATCH Superseded Johannes Schindelin via GitGitGadget 6 participants 83 messages Jun 18, 2019 – Jun 25, 2019 Kbuild

lore
Share & tools
lore
#0 lore raw

cover Fix MSVC support, at long last

Philip Oakley and Jeff Hostetler worked quite a bit on getting Git to compile with MS Visual C again, and this patch series is the culmination of those efforts. With these patches, it is as easy as

make MSVC=1

Note: the patches went through quite the number of iterations. For example, for a long time we targeted Visual Studio 2015, and used NuGet packages for the dependencies (such as OpenSSL, cURL, etc), while the current iteration targets Visual Studio 2017 and uses vcpkg
[https://docs.microsoft.com/en-us/cpp/vcpkg?view=vs-2017] for dependencies. Hopefully I did not forget to remove any remnants of those previous versions.

Please also note that this patch series is part 1 of 3 in a bigger story:
the next patch series will add support to build Git in Microsoft Visual Studio, and the third patch series will add Continuous Testing by adding an MSVC build and a corresponding parallelized test job to our Azure Pipeline.

Jeff Hostetler (8):
cache-tree.c: avoid reusing the DEBUG constant
msvc: mark a variable as non-const
msvc: do not re-declare the timespec struct
msvc: define ftello()
msvc: fix detect_msys_tty()
msvc: support building Git using MS Visual C++
msvc: do not pretend to support all signals
msvc: ignore .dll and incremental compile output

Johannes Schindelin (6):
Mark .bat files as requiring CR/LF endings
t0001 (mingw): do not expect a specific order of stdout/stderr
obstack: fix compiler warning
mingw: replace mingw_startup() hack
msvc: fix dependencies of compat/msvc.c
msvc: avoid debug assertion windows in Debug Mode

Philip Oakley (3):
msvc: include sigset_t definition
msvc: define O_ACCMODE
msvc: add pragmas for common warnings

 .gitattributes                     |   1 +
 .gitignore                         |   5 +
 Makefile                           |  42 ++++++-
 cache-tree.c                       |  14 +--
 compat/mingw.c                     | 100 +++++++++++++----
 compat/mingw.h                     |  24 ++--
 compat/msvc.h                      |  10 ++
 compat/obstack.h                   |   2 +-
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  51 +++++++++
 compat/vcbuild/find_vs_env.bat     | 169 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 compat/winansi.c                   |  13 +++
 config.mak.uname                   |  79 ++++++++++++--
 git-compat-util.h                  |   9 ++
 t/t0001-init.sh                    |   3 +-
 18 files changed, 628 insertions(+), 57 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat

base-commit: b697d92f56511e804b8ba20ccbe7bdc85dc66810
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-149%2Fdscho%2Fmsvc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-149/dscho/msvc-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/149

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/17 Mark .bat files as requiring CR/LF endings

From: Johannes Schindelin <[email protected]>

Just like the natural line ending for Unix shell scripts consist of a single Line Feed, the natural line ending for (DOS) Batch scripts consists of a Carriage Return followed by a Line Feed.

It seems that both Unix shell script interpreters and the interpreter for Batch scripts (cmd.exe) are keen on seeing the "right" line endings.

---
 .gitattributes | 1 +
 1 file changed, 1 insertion(+)
.gitattributes +1 -0
index 9fa72ad450..b08a1416d8 100644
@@ -5,6 +5,7 @@
 *.pl eof=lf diff=perl
 *.pm eol=lf diff=perl
 *.py eol=lf diff=python
+*.bat eol=crlf
 /Documentation/**/*.txt eol=lf
 /command-list.txt eol=lf
 /GIT-VERSION-GEN eol=lf
Signature · 1 line
gitgitgadget
#19 lore raw

PATCH 5/17 mingw: replace mingw_startup() hack

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Git for Windows has special code to retrieve the command-line parameters (and even the environment) in UTF-16 encoding, so that they can be converted to UTF-8. This is necessary because Git for Windows wants to use UTF-8 encoded strings throughout its code, and the main() function does not get the parameters in that encoding.

To do that, we used the __wgetmainargs() function, which is not even a Win32 API function, but provided by the MINGW "runtime" instead.

Obviously, this method would not work with any compiler other than GCC, and in preparation for compiling with Visual C++, we would like to avoid precisely that.

Lucky us, there is a much more elegant way: we can simply implement the UTF-16 variant of main(): wmain().

To make that work, we need to link with -municode. The command-line parameters are passed to wmain() encoded in UTF-16, as desired, and this method also works with GCC, and also with Visual C++ after adjusting the MSVC linker flags to force it to use wmain().

---
 compat/mingw.c   | 53 +++++++++++++++++++++++++++++++-----------------
 compat/mingw.h   | 22 ++++++++++----------
 config.mak.uname |  3 ++-
 3 files changed, 47 insertions(+), 31 deletions(-)
compat/mingw.c +34 -19
index 9b6d2400e1..0d8713e515 100644
@@ -2301,18 +2301,13 @@ static void setup_windows_environment(void)
 		setenv("TERM", "cygwin", 1);
 }
 
+#if !defined(_MSC_VER)
 /*
  * Disable MSVCRT command line wildcard expansion (__getmainargs called from
  * mingw startup code, see init.c in mingw runtime).
  */
 int _CRT_glob = 0;
-
-typedef struct {
-	int newmode;
-} _startupinfo;
-
-extern int __wgetmainargs(int *argc, wchar_t ***argv, wchar_t ***env, int glob,
-		_startupinfo *si);
+#endif
 
 static NORETURN void die_startup(void)
 {
@@ -2390,22 +2385,25 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
-void mingw_startup(void)
+/*
+ * We implement wmain() and compile with -municode, which would
+ * normally ignore main(), but we call the latter from the former
+ * so that we can handle non-ASCII command-line parameters
+ * appropriately.
+ *
+ * To be more compatible with the core git code, we convert
+ * argv into UTF8 and pass them directly to main().
+ */
+int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, argc;
-	char *buffer;
-	wchar_t **wenv, **wargv;
-	_startupinfo si;
+	int i, maxlen, exit_status;
+	char *buffer, **save;
+	const char **argv;
 
 	trace2_initialize_clock();
 
 	maybe_redirect_std_handles();
 
-	/* get wide char arguments and environment */
-	si.newmode = 0;
-	if (__wgetmainargs(&argc, &wargv, &wenv, _CRT_glob, &si) < 0)
-		die_startup();
-
 	/* determine size of argv and environ conversion buffer */
 	maxlen = wcslen(wargv[0]);
 	for (i = 1; i < argc; i++)
@@ -2415,9 +2413,16 @@ void mingw_startup(void)
 	maxlen = 3 * maxlen + 1;
 	buffer = malloc_startup(maxlen);
 
-	/* convert command line arguments and environment to UTF-8 */
+	/*
+	 * Create a UTF-8 version of w_argv. Also create a "save" copy
+	 * to remember all the string pointers because parse_options()
+	 * will remove claimed items from the argv that we pass down.
+	 */
+	ALLOC_ARRAY(argv, argc + 1);
+	ALLOC_ARRAY(save, argc + 1);
 	for (i = 0; i < argc; i++)
-		__argv[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+		argv[i] = save[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+	argv[i] = save[i] = NULL;
 	free(buffer);
 
 	/* fix Windows specific environment settings */
@@ -2436,6 +2441,16 @@ void mingw_startup(void)
 
 	/* initialize Unicode console */
 	winansi_init();
+
+	/* invoke the real main() using our utf8 version of argv. */
+	exit_status = main(argc, argv);
+
+	for (i = 0; i < argc; i++)
+		free(save[i]);
+	free(save);
+	free(argv);
+
+	return exit_status;
 }
 
 int uname(struct utsname *buf)
compat/mingw.h +11 -11
index 593bdbffe6..210f1b01a8 100644
@@ -562,18 +562,18 @@ int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen);
 extern CRITICAL_SECTION pinfo_cs;
 
 /*
- * A replacement of main() that adds win32 specific initialization.
+ * Git, like most portable C applications, implements a main() function. On
+ * Windows, this main() function would receive parameters encoded in the
+ * current locale, but Git for Windows would prefer UTF-8 encoded  parameters.
+ *
+ * To make that happen, we still declare main() here, and then declare and
+ * implement wmain() (which is the Unicode variant of main()) and compile with
+ * -municode. This wmain() function reencodes the parameters from UTF-16 to
+ * UTF-8 format, sets up a couple of other things as required on Windows, and
+ * then hands off to the main() function.
  */
-
-void mingw_startup(void);
-#define main(c,v) dummy_decl_mingw_main(void); \
-static int mingw_main(c,v); \
-int main(int argc, const char **argv) \
-{ \
-	mingw_startup(); \
-	return mingw_main(__argc, (void *)__argv); \
-} \
-static int mingw_main(c,v)
+int wmain(int argc, const wchar_t **w_argv);
+int main(int argc, const char **argv);
 
 /*
  * Used by Pthread API implementation for Windows
config.mak.uname +2 -1
index b71688eeb7..bf5d160ef4 100644
@@ -401,7 +401,7 @@ ifeq ($(uname_S),Windows)
 		compat/win32/trace2_win32_process_info.o \
 		compat/win32/dirent.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
-	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE
+	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -ENTRY:wmainCRTStartup -SUBSYSTEM:CONSOLE
 	EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj
 	PTHREAD_LIBS =
 	lib =
@@ -548,6 +548,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	ETAGS_TARGET = ETAGS
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	DEFAULT_HELP_FORMAT = html
+	BASIC_LDFLAGS += -municode
 	COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/17 t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When redirecting stdout/stderr to the same file, we cannot guarantee that stdout will come first.

In fact, in this test case, it seems that an MSVC build always prints stderr first.

In any case, this test case does not want to verify the *order* but the *presence* of both outputs, so let's relax the test a little.

---
 t/t0001-init.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
t/t0001-init.sh +2 -1
index 77a224aafb..c7ab18f40f 100755
@@ -474,7 +474,8 @@ test_expect_success MINGW 'redirect std handles' '
 		GIT_REDIRECT_STDERR="2>&1" \
 		git rev-parse --git-dir --verify refs/invalid &&
 	printf ".git\nfatal: Needed a single revision\n" >expect &&
-	test_cmp expect output.txt
+	sort <output.txt >output.sorted &&
+	test_cmp expect output.sorted
 '
 
 test_done
Signature · 1 line
gitgitgadget
#10 lore raw

PATCH 3/17 cache-tree.c: avoid reusing the DEBUG constant

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

In MSVC, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c, so let's use a less ambiguous constant there.

---
 cache-tree.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
cache-tree.c +7 -7
index b13bfaf71e..706ffcf188 100644
@@ -6,8 +6,8 @@
 #include "object-store.h"
 #include "replace-object.h"
 
-#ifndef DEBUG
-#define DEBUG 0
+#ifndef DEBUG_CACHE_TREE
+#define DEBUG_CACHE_TREE 0
 #endif
 
 struct cache_tree *cache_tree(void)
@@ -111,7 +111,7 @@ static int do_invalidate_path(struct cache_tree *it, const char *path)
 	int namelen;
 	struct cache_tree_sub *down;
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree invalidate <%s>\n", path);
 #endif
 
@@ -398,7 +398,7 @@ static int update_one(struct cache_tree *it,
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 		fprintf(stderr, "cache-tree update-one %o %.*s\n",
 			mode, entlen, path + baselen);
 #endif
@@ -421,7 +421,7 @@ static int update_one(struct cache_tree *it,
 
 	strbuf_release(&buffer);
 	it->entry_count = to_invalidate ? -1 : i - *skip_count;
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
 		it->entry_count, it->subtree_nr,
 		oid_to_hex(&it->oid));
@@ -462,7 +462,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
 	strbuf_add(buffer, path, pathlen);
 	strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%.*s> (%d ent, %d subtree) %s\n",
 			pathlen, path, it->entry_count, it->subtree_nr,
@@ -536,7 +536,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
 		size -= rawsz;
 	}
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%s> (%d ent, %d subtree) %s\n",
 			*buffer, it->entry_count, subtree_nr,
Signature · 1 line
gitgitgadget
#23 lore raw

PATCH 9/17 msvc: mark a variable as non-const

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015 complains when using a const pointer in memcpy()/free().

---
 compat/mingw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
compat/mingw.c +4 -1
index 0d8713e515..d14d33308d 100644
@@ -1553,7 +1553,10 @@ static int try_shell_exec(const char *cmd, char *const *argv)
 	if (prog) {
 		int exec_id;
 		int argc = 0;
-		const char **argv2;
+#ifndef _MSC_VER
+		const
+#endif
+		char **argv2;
 		while (argv[argc]) argc++;
 		ALLOC_ARRAY(argv2, argc + 1);
 		argv2[0] = (char *)cmd;	/* full path to the script file */
Signature · 1 line
gitgitgadget
#20 lore raw

PATCH 6/17 msvc: fix dependencies of compat/msvc.c

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The file compat/msvc.c includes compat/mingw.c, which means that we have to recompile compat/msvc.o if compat/mingw.c changes.

---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)
config.mak.uname +2 -0
index bf5d160ef4..3310f6284c 100644
@@ -414,6 +414,8 @@ else
 	BASIC_CFLAGS += -Zi -MDd
 endif
 	X = .exe
+
+compat/msvc.o: compat/msvc.c compat/mingw.c GIT-CFLAGS
 endif
 ifeq ($(uname_S),Interix)
 	NO_INITGROUPS = YesPlease
Signature · 1 line
gitgitgadget
#24 lore raw

PATCH 10/17 msvc: do not re-declare the timespec struct

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015's headers already declare that struct.

---
 compat/mingw.h | 2 ++
 1 file changed, 2 insertions(+)
compat/mingw.h +2 -0
index 210f1b01a8..a03e40e6e2 100644
@@ -352,11 +352,13 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 #ifndef __MINGW64_VERSION_MAJOR
 #define off_t off64_t
 #define lseek _lseeki64
+#ifndef _MSC_VER
 struct timespec {
 	time_t tv_sec;
 	long tv_nsec;
 };
 #endif
+#endif
 
 struct mingw_stat {
     _dev_t st_dev;
Signature · 1 line
gitgitgadget
#21 lore raw

PATCH 7/17 msvc: include sigset_t definition

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

On MSVC (VS2008) sigset_t is not defined.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 29a8ce8204..04b4750b87 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+typedef int sigset_t;
+
 #include "compat/mingw.h"
 
 #endif
Signature · 1 line
gitgitgadget
#22 lore raw

PATCH 8/17 msvc: define O_ACCMODE

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

This constant is not defined in MSVC's headers.

In UCRT's fcntl.h, _O_RDONLY, _O_WRONLY and _O_RDWR are defined as 0, 1 and 2, respectively. Yes, that means that UCRT breaks with the tradition that O_RDWR == O_RDONLY | O_WRONLY.

It is a perfectly legal way to define those constants, though, therefore we need to take care of defining O_ACCMODE accordingly.

This is particularly important in order to keep our "open() can set errno to EISDIR" emulation working: it tests that (flags & O_ACCMODE) is not identical to O_RDONLY before going on to test specifically whether the file for which open() reported EACCES is, in fact, a directory.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 04b4750b87..d336d80670 100644
@@ -19,6 +19,8 @@
 #undef ERROR
 
 typedef int sigset_t;
+/* open for reading, writing, or both (not in fcntl.h) */
+#define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
 
 #include "compat/mingw.h"
 
Signature · 1 line
gitgitgadget
#18 lore raw

PATCH 4/17 obstack: fix compiler warning

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

MS Visual C suggests that the construct

condition ? (int) i : (ptrdiff_t) d

is incorrect. Let's fix this by casting to ptrdiff_t also for the positive arm of the conditional.

---
 compat/obstack.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
compat/obstack.h +1 -1
index ced94d0118..ae36ed6a66 100644
@@ -496,7 +496,7 @@ __extension__								\
 ( (h)->temp.tempint = (char *) (obj) - (char *) (h)->chunk,		\
   ((((h)->temp.tempint > 0						\
     && (h)->temp.tempint < (h)->chunk_limit - (char *) (h)->chunk))	\
-   ? (int) ((h)->next_free = (h)->object_base				\
+   ? (ptrdiff_t) ((h)->next_free = (h)->object_base				\
 	    = (h)->temp.tempint + (char *) (h)->chunk)			\
    : (((obstack_free) ((h), (h)->temp.tempint + (char *) (h)->chunk), 0), 0)))
 
Signature · 1 line
gitgitgadget
#33 lore raw

PATCH 15/17 msvc: do not pretend to support all signals

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

This special-cases various signals that are not supported on Windows, such as SIGPIPE. These cause the UCRT to throw asserts (at least in debug mode).

---
 compat/mingw.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
compat/mingw.c +26 -0
index 667285887a..8b56aa5773 100644
@@ -2119,8 +2119,34 @@ int mingw_raise(int sig)
 			sigint_fn(SIGINT);
 		return 0;
 
+#if defined(_MSC_VER)
+		/*
+		 * <signal.h> in the CRT defines 8 signals as being
+		 * supported on the platform.  Anything else causes
+		 * an "Invalid signal or error" (which in DEBUG builds
+		 * causes the Abort/Retry/Ignore dialog).  We by-pass
+		 * the CRT for things we already know will fail.
+		 */
+		/*case SIGINT:*/
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGTERM:
+	case SIGBREAK:
+	case SIGABRT:
+	case SIGABRT_COMPAT:
+		return raise(sig);
+	default:
+		errno = EINVAL;
+		return -1;
+
+#else
+
 	default:
 		return raise(sig);
+
+#endif
+
 	}
 }
 
Signature · 1 line
gitgitgadget
#36 lore raw

PATCH 16/17 msvc: avoid debug assertion windows in Debug Mode

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

For regular debugging, it is pretty helpful when a debug assertion in a running application triggers a window that offers to start the debugger.

However, when running the test suite, it is not so helpful, in particular when the debug assertions are then suppressed anyway because we disable the invalid parameter checking (via invalidcontinue.obj, see the comment in config.mak.uname about that object for more information).

So let's simply disable that window in Debug Mode (it is already disabled in Release Mode).

---
 compat/mingw.c | 4 ++++
 1 file changed, 4 insertions(+)
compat/mingw.c +4 -0
index 8b56aa5773..cdb99b940e 100644
@@ -2438,6 +2438,10 @@ int wmain(int argc, const wchar_t **wargv)
 	trace2_initialize_clock();
 
 #ifdef _MSC_VER
+#ifdef _DEBUG
+	_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG);
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
 #endif
Signature · 1 line
gitgitgadget
#27 lore raw

PATCH 13/17 msvc: support building Git using MS Visual C++

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

With this patch, Git can be built using the Microsoft toolchain, via:

	make MSVC=1 [DEBUG=1]

Third party libraries are built from source using the open source "vcpkg" tool set. See https://github.com/Microsoft/vcpkg

On a first build, the vcpkg tools and the third party libraries are automatically downloaded and built. DLLs for the third party libraries are copied to the top-level (and t/helper) directory to facilitate debugging. See compat/vcbuild/README.

A series of .bat files are invoked by the Makefile to find the location of the installed version of Visual Studio and the associated compiler tools (essentially replicating the environment setup performed by a "Developer Command Prompt"). This should find the most recent VS2015 or VS2017 installation. Output from these scripts are used by the Makefile to define compiler and linker pathnames and -I and -L arguments.

The build produces .pdb files for both debug and release builds.

Note: This commit was squashed from an organic series of commits developed between 2016 and 2018 in Git for Windows' master branch. This combined commit eliminates the obsolete commits related to fetching NuGet packages for third party libraries. It is difficult to use NuGet packages for C/C++ sources because they may be built by earlier versions of the MSVC compiler and have CRT version and linking issues.

Additionally, the C/C++ NuGet packages that we were using tended to not be updated concurrently with the sources. And in the case of cURL and OpenSSL, this could expose us to security issues.

---
 Makefile                           |  42 ++++++-
 compat/mingw.c                     |  12 ++
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  51 +++++++++
 compat/vcbuild/find_vs_env.bat     | 169 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 config.mak.uname                   |  74 +++++++++++--
 git-compat-util.h                  |   9 ++
 10 files changed, 504 insertions(+), 16 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat
Makefile +41 -1
index 8a7e235352..7777af7d0e 100644
@@ -1240,7 +1240,7 @@ endif
 
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
 PATH := $(SANE_TOOL_PATH):${PATH}
 else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
@@ -2873,6 +2873,33 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+ifdef MSVC
+	# We DO NOT install the individual foo.o.pdb files because they
+	# have already been rolled up into the exe's pdb file.
+	# We DO NOT have pdb files for the builtin commands (like git-status.exe)
+	# because it is just a copy/hardlink of git.exe, rather than a unique binary.
+	$(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ifndef DEBUG
+	$(INSTALL) $(vcpkg_rel_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_rel_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+else
+	$(INSTALL) $(vcpkg_dbg_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_dbg_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 	$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
@@ -3085,6 +3112,19 @@ endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
 	$(RM) GIT-USER-AGENT GIT-PREFIX
 	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
+ifdef MSVC
+	$(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS))
+	$(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(TEST_PROGRAMS))
+	$(RM) compat/vcbuild/MSVC-DEFS-GEN
+endif
 
 .PHONY: all install profile-clean cocciclean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
compat/mingw.c +12 -0
index d14d33308d..667285887a 100644
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
+#ifdef _MSC_VER
+#ifdef _DEBUG
+#include <crtdbg.h>
+#endif
+#endif
+
 /*
  * We implement wmain() and compile with -municode, which would
  * normally ignore main(), but we call the latter from the former
@@ -2405,6 +2411,12 @@ int wmain(int argc, const wchar_t **wargv)
 
 	trace2_initialize_clock();
 
+#ifdef _MSC_VER
+#ifdef USE_MSVC_CRTDBG
+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+#endif
+#endif
+
 	maybe_redirect_std_handles();
 
 	/* determine size of argv and environ conversion buffer */
compat/vcbuild/.gitignore new file +3 -0
new file mode 100644
index 0000000000..8f8b794ef3
@@ -0,0 +1,3 @@
+/vcpkg/
+/MSVC-DEFS-GEN
+/VCPKG-DEFS
compat/vcbuild/README +51 -0

54 lines not on this page · raw message

compat/vcbuild/find_vs_env.bat new file +169 -0

169 lines not on this page · raw message

compat/vcbuild/scripts/clink.pl +36 -5

78 lines not on this page · raw message

compat/vcbuild/vcpkg_copy_dlls.bat new file +39 -0
new file mode 100644
index 0000000000..13661c14f8
@@ -0,0 +1,39 @@
+@ECHO OFF
+REM ================================================================
+REM This script is an optional step. It copies the *.dll and *.pdb
+REM files (created by vcpkg_install.bat) into the top-level directory
+REM of the repo so that you can type "./git.exe" and find them without
+REM having to fixup your PATH.
+REM
+REM NOTE: Because the names of some DLL files change between DEBUG and
+REM NOTE: RELEASE builds when built using "vcpkg.exe", you will need
+REM NOTE: to copy up the corresponding version.
+REM ================================================================
+
+	SETLOCAL EnableDelayedExpansion
+
+	@FOR /F "delims=" %%D IN ("%~dp0") DO @SET cwd=%%~fD
+	cd %cwd%
+
+	SET arch=x64-windows
+	SET inst=%cwd%vcpkg\installed\%arch%
+
+	IF [%1]==[release] (
+		echo Copying RELEASE mode DLLs to repo root...
+	) ELSE IF [%1]==[debug] (
+		SET inst=%inst%\debug
+		echo Copying DEBUG mode DLLs to repo root...
+	) ELSE (
+		echo ERROR: Invalid argument.
+		echo Usage: %~0 release
+		echo Usage: %~0 debug
+		EXIT /B 1
+	)
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\t\helper\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\t\helper\
+
+	EXIT /B 0
compat/vcbuild/vcpkg_install.bat new file +80 -0

80 lines not on this page · raw message

config.mak.uname +64 -10

123 lines not on this page · raw message

git-compat-util.h +9 -0
index cc0e7e9733..83be89de0a 100644
@@ -1,6 +1,15 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#ifdef USE_MSVC_CRTDBG
+/*
+ * For these to work they must appear very early in each
+ * file -- before most of the standard header files.
+ */
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #define _FILE_OFFSET_BITS 64
 
 
Signature · 1 line
gitgitgadget
#32 lore raw

PATCH 14/17 msvc: add pragmas for common warnings

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

MSVC can be overzealous about some warnings. Disable them.

---
 compat/msvc.h | 4 ++++
 1 file changed, 4 insertions(+)
compat/msvc.h +4 -0
index d7525cf61d..1d7a8c6145 100644
@@ -6,6 +6,10 @@
 #include <malloc.h>
 #include <io.h>
 
+#pragma warning(disable: 4018) /* signed/unsigned comparison */
+#pragma warning(disable: 4244) /* type conversion, possible loss of data */
+#pragma warning(disable: 4090) /* 'function' : different 'const' qualifiers (ALLOC_GROW etc.)*/
+
 /* porting function */
 #define inline __inline
 #define __inline__ __inline
Signature · 1 line
gitgitgadget
#26 lore raw

PATCH 12/17 msvc: fix detect_msys_tty()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

The ntstatus.h header is only available in MINGW.

---
 compat/winansi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
compat/winansi.c +13 -0
index f4f08237f9..11cd9b82cc 100644
@@ -544,7 +544,20 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
 #ifdef DETECT_MSYS_TTY
 
 #include <winternl.h>
+
+#if defined(_MSC_VER)
+
+typedef struct _OBJECT_NAME_INFORMATION
+{
+	UNICODE_STRING Name;
+	WCHAR NameBuffer[0];
+} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
+
+#define ObjectNameInformation 1
+
+#else
 #include <ntstatus.h>
+#endif
 
 static void detect_msys_tty(int fd)
 {
Signature · 1 line
gitgitgadget
#37 lore raw

PATCH 17/17 msvc: ignore .dll and incremental compile output

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

Ignore .dll files copied into the top-level directory. Ignore MSVC incremental compiler output files.

---
 .gitignore | 5 +++++
 1 file changed, 5 insertions(+)
.gitignore +5 -0
index 2374f77a1a..ba0e52c4d1 100644
@@ -226,6 +226,11 @@
 *.user
 *.idb
 *.pdb
+*.ilk
+*.iobj
+*.ipdb
+*.dll
+.vs/
 /Debug/
 /Release/
 *.dSYM
Signature · 1 line
gitgitgadget
#25 lore raw

PATCH 11/17 msvc: define ftello()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

It is just called differently in MSVC's headers.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index d336d80670..d7525cf61d 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+#define ftello _ftelli64
+
 typedef int sigset_t;
 /* open for reading, writing, or both (not in fcntl.h) */
 #define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Johannes Schindelin via GitGitGadget
Quoted 13 lines · Johannes Schindelin via GitGitGadget

When redirecting stdout/stderr to the same file, we cannot guarantee that stdout will come first.

In fact, in this test case, it seems that an MSVC build always prints stderr first.

In any case, this test case does not want to verify the *order* but the *presence* of both outputs, so let's relax the test a little.

---

t/t0001-init.sh test_expect_success MINGW 'redirect std handles' ' · line 476

Quoted 4 lines
t/t0001-init.sh +2 -1
@@ -474,7 +474,8 @@ test_expect_success MINGW 'redirect std handles' '
        printf ".git\nfatal: Needed a single revision\n" >expect &&
-       test_cmp expect output.txt
+       sort <output.txt >output.sorted &&
+       test_cmp expect output.sorted

It was quite surprising to see this sorting only 'output' but not 'expect'. I see now that 'output' is already "sorted" (in that sense), but it feels fragile. More robust would be to sort 'expect' as well:

    printf ".git\nfatal: Needed a single revision\n" | sort >expect &&

This would protect against the next person who modifies the 'printf' testing on Unix and Windows/gcc and thinking all is well even though the change might make the test fail for an MSVC build.

#11 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Jeff Hostetler via GitGitGadget
Quoted 5 lines · Jeff Hostetler via GitGitGadget

In MSVC, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c, so let's use a less ambiguous constant there.

s/constant/macro name/ would be clearer.

Quoted 2 lines
#28 lore raw

Re: [PATCH 13/17] msvc: support building Git using MS Visual C++

In reply to Jeff Hostetler via GitGitGadget

compat/mingw.c static void maybe_redirect_std_handles(void) · line 2392

Quoted 10 lines · Jeff Hostetler via GitGitGadget
compat/mingw.c +2 -0
index d14d33308d..667285887a 100644
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
                                  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
+#ifdef _MSC_VER
+#ifdef _DEBUG

why not use DEBUG instead (without the leading underscore), then you could also drop the -D_DEBUG below

...

compat/mingw.c static void maybe_redirect_std_handles(void) · line 2393

Quoted 1 line
compat/mingw.c +1 -0
+       # BASIC_CLFAGS += -DUSE_MSVC_CRTDBG

typo

Carlo

#34 lore raw

Re: [PATCH 15/17] msvc: do not pretend to support all signals

In reply to Jeff Hostetler via GitGitGadget
Quoted 9 lines · Jeff Hostetler via GitGitGadget

This special-cases various signals that are not supported on Windows, such as SIGPIPE. These cause the UCRT to throw asserts (at least in debug mode).

---

compat/mingw.c int mingw_raise(int sig) · line 2128

Quoted 10 lines
compat/mingw.c +10 -0
@@ -2119,8 +2119,34 @@ int mingw_raise(int sig)
+#if defined(_MSC_VER)
+               /*
+                * <signal.h> in the CRT defines 8 signals as being
+                * supported on the platform.  Anything else causes
+                * an "Invalid signal or error" (which in DEBUG builds
+                * causes the Abort/Retry/Ignore dialog).  We by-pass
+                * the CRT for things we already know will fail.
+                */
+               /*case SIGINT:*/
+       case SIGILL:

Why is SIGINT commented out?

And, the comment block seems over-indented.

Quoted 10 lines
compat/mingw.c +10 -0
+       case SIGFPE:
+       case SIGSEGV:
+       case SIGTERM:
+       case SIGBREAK:
+       case SIGABRT:
+       case SIGABRT_COMPAT:
+               return raise(sig);
+       default:
+               errno = EINVAL;
+               return -1;
#4 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Eric Sunshine

Am 19.06.19 um 01:12 schrieb Eric Sunshine:

Quoted 13 lines

t/t0001-init.sh test_expect_success MINGW 'redirect std handles' ' · line 476

Quoted 6 lines · Johannes Schindelin via GitGitGadget
t/t0001-init.sh +2 -1
@@ -474,7 +474,8 @@ test_expect_success MINGW 'redirect std handles' '
        printf ".git\nfatal: Needed a single revision\n" >expect &&
-       test_cmp expect output.txt
+       sort <output.txt >output.sorted &&
+       test_cmp expect output.sorted

It was quite surprising to see this sorting only 'output' but not 'expect'. I see now that 'output' is already "sorted" (in that sense), but it feels fragile. More robust would be to sort 'expect' as well:

Quoted 1 line
    printf ".git\nfatal: Needed a single revision\n" | sort >expect &&

Following Dscho's recent objection elsewhere that tests tend to check for much more than regressions, wouldn't it be logical to write these as

	grep -F .git" output.txt &&
	test_i18n_grep "Needed a single rev" output.txt

without an 'expect' file at all?

-- Hannes

#5 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

Quoted 14 lines · Johannes Sixt

Am 19.06.19 um 01:12 schrieb Eric Sunshine:

Quoted 7 lines

t/t0001-init.sh test_expect_success MINGW 'redirect std handles' ' · line 478

Quoted 4 lines · Johannes Schindelin via GitGitGadget
        printf ".git\nfatal: Needed a single revision\n" >expect &&
t/t0001-init.sh +2 -1
-       test_cmp expect output.txt
+       sort <output.txt >output.sorted &&
+       test_cmp expect output.sorted
Quoted 5 lines

It was quite surprising to see this sorting only 'output' but not 'expect'. I see now that 'output' is already "sorted" (in that sense), but it feels fragile. More robust would be to sort 'expect' as well:

    printf ".git\nfatal: Needed a single revision\n" | sort >expect &&
Quoted 7 lines

Following Dscho's recent objection elsewhere that tests tend to check for much more than regressions, wouldn't it be logical to write these as

grep -F .git" output.txt && test_i18n_grep "Needed a single rev" output.txt

without an 'expect' file at all?

I considered suggesting that, as well, as being more obvious and less fragile (with the exception that "Needed a single rev" isn't currently localizable in builtin/rev-parse.c, so plain 'grep' instead of 'test_i18n_grep').

#30 lore raw

Re: [PATCH 13/17] msvc: support building Git using MS Visual C++

In reply to Jeff Hostetler via GitGitGadget
Quoted 12 lines · Jeff Hostetler via GitGitGadget

With this patch, Git can be built using the Microsoft toolchain, via:

        make MSVC=1 [DEBUG=1]

Third party libraries are built from source using the open source "vcpkg" tool set. See https://github.com/Microsoft/vcpkg

---

Makefile endif · line 1240

Quoted 2 lines
Makefile +1 -1
@@ -1240,7 +1240,7 @@ endif
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'

This seems like a distinct bug fix which should live in its own patch.

compat/mingw.c int wmain(int argc, const wchar_t **wargv) · line 2415

Quoted 13 lines
compat/mingw.c +10 -0
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
+#ifdef _MSC_VER
+#ifdef _DEBUG
+#include <crtdbg.h>
+#endif
+#endif
@@ -2405,6 +2411,12 @@ int wmain(int argc, const wchar_t **wargv)
+#ifdef _MSC_VER
+#ifdef USE_MSVC_CRTDBG
+       _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+#endif
+#endif

Shouldn't these changes be squashed into 16/17 (with the commit message of 16/17 adjusted accordingly), rather than being included in this patch?

compat/vcbuild/README line 3

Quoted 5 lines
compat/vcbuild/README +3 -0
@@ -1,3 +1,54 @@
+Alternatively, run `make MSVC=1 vcxproj` and then load the generated
+git.sln in Visual Studio. The initial build will install the vcpkg
+system and build the dependencies automatically. This will take a while.

Is this bit implemented yet, or will it be introduced by a subsequent patch series mentioned in the cover letter? If the latter, perhaps this README snippet belongs to that future patch series.

compat/vcbuild/README line 46

Quoted 3 lines
compat/vcbuild/README +3 -0
+Note that this will automatically add and commit the generated
+.sln and .vcxproj files to the repo.  You may want to drop this
+commit before submitting a Pull Request....

Yuck. An automatic commit as part of the build process has high surprise-factor, and it seems like it's creating extra work (and possibility for error) if the user needs to remember to drop it before submitting.

compat/vcbuild/README line 49

Quoted 2 lines
compat/vcbuild/README +2 -0
+Or maybe we should put the .sln/.vcxproj files in the .gitignore file
+and not do this.  I'm not sure.

Seems like a better choice.

compat/vcbuild/find_vs_env.bat line 4

Quoted 6 lines
compat/vcbuild/find_vs_env.bat +4 -0
@@ -0,0 +1,169 @@
+:not_2015
+   REM TODO....
+   echo TODO support older versions of VS. >&2
+   EXIT /B 1

As this is a user-facing error message, perhaps it could be worded differently. Maybe:

ERROR: unsupported VS version (older than VS2015)

or something.

#15 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Carlo Arenas

Hi Carlo,

Quoted 1 line · Carlo Arenas

shouldn't this also be a problem with builtin/blame.c?

Sharp eyes!

It does not *really* matter as much here, as that file defines that DEBUG constant if it has not yet been defined, but yes, it assumes that *if* it is defined, then it is set to 1. Which is the case, but it is fragile.

I changed it locally already, and it will be part of the next iteration.

Thanks,
Johannes

#12 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Eric Sunshine

Hi Eric,

Quoted 9 lines · Eric Sunshine
Quoted 5 lines · Jeff Hostetler via GitGitGadget

In MSVC, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c, so let's use a less ambiguous constant there.

s/constant/macro name/ would be clearer.

To me, "macro" always sounds as if it referred to executable code, or at least to something that expands to code.

I went with s/constant/name/ instead.

Thanks,
Dscho

Quoted 4 lines
Quoted 3 lines
#6 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Eric Sunshine

Hi Eric & Hannes,

Quoted 21 lines · Eric Sunshine
Quoted 20 lines · Johannes Sixt

Am 19.06.19 um 01:12 schrieb Eric Sunshine:

Quoted 7 lines

t/t0001-init.sh test_expect_success MINGW 'redirect std handles' ' · line 478

Quoted 4 lines · Johannes Schindelin via GitGitGadget
        printf ".git\nfatal: Needed a single revision\n" >expect &&
t/t0001-init.sh +2 -1
-       test_cmp expect output.txt
+       sort <output.txt >output.sorted &&
+       test_cmp expect output.sorted
Quoted 5 lines

It was quite surprising to see this sorting only 'output' but not 'expect'. I see now that 'output' is already "sorted" (in that sense), but it feels fragile. More robust would be to sort 'expect' as well:

    printf ".git\nfatal: Needed a single revision\n" | sort >expect &&

Following Dscho's recent objection elsewhere that tests tend to check for much more than regressions, wouldn't it be logical to write these as

grep -F .git" output.txt && test_i18n_grep "Needed a single rev" output.txt

Quoted 6 lines
Quoted 1 line

without an 'expect' file at all?

I considered suggesting that, as well, as being more obvious and less fragile (with the exception that "Needed a single rev" isn't currently localizable in builtin/rev-parse.c, so plain 'grep' instead of 'test_i18n_grep').

Valid points all around, thank you so much!

The next iteration will have the two greps instead, Dscho

#7 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

Hi Eric,

Quoted 23 lines · Johannes Schindelin
Quoted 21 lines · Eric Sunshine
Quoted 20 lines · Johannes Sixt

Am 19.06.19 um 01:12 schrieb Eric Sunshine:

Quoted 7 lines

t/t0001-init.sh test_expect_success MINGW 'redirect std handles' ' · line 478

Quoted 4 lines · Johannes Schindelin via GitGitGadget
        printf ".git\nfatal: Needed a single revision\n" >expect &&
t/t0001-init.sh +2 -1
-       test_cmp expect output.txt
+       sort <output.txt >output.sorted &&
+       test_cmp expect output.sorted
Quoted 5 lines

It was quite surprising to see this sorting only 'output' but not 'expect'. I see now that 'output' is already "sorted" (in that sense), but it feels fragile. More robust would be to sort 'expect' as well:

    printf ".git\nfatal: Needed a single revision\n" | sort >expect &&

Following Dscho's recent objection elsewhere that tests tend to check for much more than regressions, wouldn't it be logical to write these as

grep -F .git" output.txt && test_i18n_grep "Needed a single rev" output.txt

Quoted 6 lines
Quoted 6 lines
Quoted 1 line

without an 'expect' file at all?

I considered suggesting that, as well, as being more obvious and less fragile (with the exception that "Needed a single rev" isn't currently localizable in builtin/rev-parse.c, so plain 'grep' instead of 'test_i18n_grep').

Interesting side note: I just realized that t6050-replace.sh does indeed contain

test_i18ngrep "Needed a single revision" err

so I wonder why that works.

If anybody has an answer, I'd be curious, but for now I want to focus on this here patch series instead.

Ciao,
Dscho

Quoted 5 lines

Valid points all around, thank you so much!

The next iteration will have the two greps instead, Dscho

#29 lore raw

Re: [PATCH 13/17] msvc: support building Git using MS Visual C++

In reply to Carlo Arenas

Hi Carlo,

Quoted 10 lines · Carlo Arenas
Quoted 8 lines · Jeff Hostetler via GitGitGadget
compat/mingw.c +0 -0
index d14d33308d..667285887a 100644
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
                                  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
Quoted 5 lines

compat/mingw.c static void maybe_redirect_std_handles(void) · line 2392

Quoted 2 lines
compat/mingw.c +2 -0
+#ifdef _MSC_VER
+#ifdef _DEBUG

why not use DEBUG instead (without the leading underscore), then you could also drop the -D_DEBUG below

As per
https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019,
the convention is actually to define _DEBUG in debug mode and NDEBUG in release mode (yes, it's a bit redundant and possibly contradictory). My reading is that DEBUG is only a historical wart.

Quoted 4 lines

...

Quoted 1 line

+ # BASIC_CLFAGS += -DUSE_MSVC_CRTDBG

typo

Yep, and a copy-edited of the tyop I introduced in df5218b4c30b (config.mak.uname: support MSys2, 2016-01-13)!

Thanks!
Johannes

#31 lore raw

Re: [PATCH 13/17] msvc: support building Git using MS Visual C++

In reply to Eric Sunshine

Hi Eric,

Quoted 6 lines · Eric Sunshine
Quoted 4 lines · Jeff Hostetler via GitGitGadget

With this patch, Git can be built using the Microsoft toolchain, via:

        make MSVC=1 [DEBUG=1]
Quoted 12 lines

Makefile endif · line 1240

Quoted 10 lines

Third party libraries are built from source using the open source "vcpkg" tool set. See https://github.com/Microsoft/vcpkg

---

Makefile +1 -1
@@ -1240,7 +1240,7 @@ endif
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'

This seems like a distinct bug fix which should live in its own patch.

And so it did! And so it will do again, starting from the next iteration. Thanks.

Quoted 9 lines
Quoted 9 lines
compat/mingw.c +6 -0
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
+#ifdef _MSC_VER
+#ifdef _DEBUG
+#include <crtdbg.h>
+#endif
+#endif
@@ -2405,6 +2411,12 @@ int wmain(int argc, const wchar_t **wargv)
+#ifdef _MSC_VER
Quoted 8 lines

compat/mingw.c int wmain(int argc, const wchar_t **wargv) · line 2418

Quoted 4 lines
compat/mingw.c +4 -0
+#ifdef USE_MSVC_CRTDBG
+       _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+#endif
+#endif

Shouldn't these changes be squashed into 16/17 (with the commit message of 16/17 adjusted accordingly), rather than being included in this patch?

Not really. This has little to do with assertions, and much more with memory usage debugging support in Visual Studio.

I guess that this (together with the other USE_MSVC_CRTDBG) should go into its own, separate commit.

Quoted 9 lines

compat/vcbuild/README line 3

Quoted 5 lines
compat/vcbuild/README +3 -0
@@ -1,3 +1,54 @@
+Alternatively, run `make MSVC=1 vcxproj` and then load the generated
+git.sln in Visual Studio. The initial build will install the vcpkg
+system and build the dependencies automatically. This will take a while.

Is this bit implemented yet, or will it be introduced by a subsequent patch series mentioned in the cover letter? If the latter, perhaps this README snippet belongs to that future patch series.

It is actually implemented in a future patch series (I hinted at it in the cover letter).

Quoted 8 lines

compat/vcbuild/README line 46

Quoted 3 lines
compat/vcbuild/README +3 -0
+Note that this will automatically add and commit the generated
+.sln and .vcxproj files to the repo.  You may want to drop this
+commit before submitting a Pull Request....

Yuck. An automatic commit as part of the build process has high surprise-factor, and it seems like it's creating extra work (and possibility for error) if the user needs to remember to drop it before submitting.

The thing to keep in mind is that the *primary* reason for this Makefile target is to publish a version of the source code that works in Visual Studio out of the box. Read: without getting a (pretty heavy) Git for Windows SDK on top.

The idea is to avoid having to download several hundred megabytes of Git for Windows SDK, and instead run the tests in the Git Bash of a regular Git for Windows.

Of course, this only works when certain "build products" (such as shell scripts that have been copy-edited to their final form, or certain generated files that the test suite wants to see) are included.

If you do not include them, tough luck, you cannot run the tests.

Of course, I *do* want contributors to run the tests, even if they choose the convenience of a full-fledged IDE, and I don't want to punish them for it by forcing them to pick all of the bits and pieces for themselves.

To achieve that, I force-add those generated files and commit the whole bunch, and publish the result as vs/master at
https://github.com/git-for-windows/git. Actually, it is a trusty Azure Pipeline that does that.

Quoted 4 lines

compat/vcbuild/README line 49

Quoted 2 lines
compat/vcbuild/README +2 -0
+Or maybe we should put the .sln/.vcxproj files in the .gitignore file
+and not do this.  I'm not sure.

Seems like a better choice.

Unless you struggled for yourself to find all the missing files you need in order to run the test suite. Or to cobble together a working Git from the build output of Visual Studio (which only contains the compiled C code, after all, and as you and me know fully well, Git insists on support files such as templates, too, and yes, they have to be "generated" and are of course excluded via .gitignore).

Once you went through those struggles, you don't want to do it again. I went a step further, and I don't want anybody but me to have to go through that again, either.

That is why I think that your statement might have been made under the false impression that this is an easy decision.

In any case, this is indeed a discussion for the next patch series, not this one.

Quoted 10 lines

compat/vcbuild/find_vs_env.bat line 4

Quoted 6 lines
compat/vcbuild/find_vs_env.bat +4 -0
@@ -0,0 +1,169 @@
+:not_2015
+   REM TODO....
+   echo TODO support older versions of VS. >&2
+   EXIT /B 1

As this is a user-facing error message, perhaps it could be worded differently. Maybe:

Quoted 3 lines

ERROR: unsupported VS version (older than VS2015)

or something.

Good idea! I made it so.

Thanks!
Dscho

#13 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Eric Sunshine
Quoted 9 lines · Eric Sunshine
Quoted 5 lines · Jeff Hostetler via GitGitGadget

In MSVC, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c, so let's use a less ambiguous constant there.

s/constant/macro name/ would be clearer.

It is closer to the standard-kosher terminology, I would think; if somebody is in more pedantic mood, "C preprocessor macro" is probably the phrase to use.

#35 lore raw

Re: [PATCH 15/17] msvc: do not pretend to support all signals

In reply to Eric Sunshine

Hi Eric,

Quoted 15 lines · Eric Sunshine
Quoted 13 lines · Jeff Hostetler via GitGitGadget

This special-cases various signals that are not supported on Windows, such as SIGPIPE. These cause the UCRT to throw asserts (at least in debug mode).

---

compat/mingw.c +4 -0
@@ -2119,8 +2119,34 @@ int mingw_raise(int sig)
+#if defined(_MSC_VER)
+               /*
+                * <signal.h> in the CRT defines 8 signals as being
+                * supported on the platform.  Anything else causes
Quoted 8 lines

compat/mingw.c int mingw_raise(int sig) · line 2131

Quoted 6 lines
compat/mingw.c +6 -0
+                * an "Invalid signal or error" (which in DEBUG builds
+                * causes the Abort/Retry/Ignore dialog).  We by-pass
+                * the CRT for things we already know will fail.
+                */
+               /*case SIGINT:*/
+       case SIGILL:

Why is SIGINT commented out?

Whoops. The case before that already handles SIGINT, I think that's why... I removed it.

Quoted 1 line

And, the comment block seems over-indented.

Not really, as the case statements are indented one level less than the code (including the comments).

But I agree that it looks funny, and moved it within the case arm.

Thanks!
Dscho

Quoted 12 lines
Quoted 11 lines
compat/mingw.c +10 -0
+       case SIGFPE:
+       case SIGSEGV:
+       case SIGTERM:
+       case SIGBREAK:
+       case SIGABRT:
+       case SIGABRT_COMPAT:
+               return raise(sig);
+       default:
+               errno = EINVAL;
+               return -1;
#16 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Johannes Schindelin

while those two changes (from DEBUG to DEBUG_$foo) are worth doing in their own merit, I am more inclined to consider this as orthogonal since by your own description[1] the right name to use would be _DEBUG (with a preceding dash) and that would obviously not conflict here.

the only remaining change then would be to drop the -DDEBUG that gets added to your BASIC_CFLAGS

Carlo
[1] https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019

#8 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Johannes Schindelin

Am 19.06.19 um 13:30 schrieb Johannes Schindelin:

Quoted 6 lines

Interesting side note: I just realized that t6050-replace.sh does indeed contain

test_i18ngrep "Needed a single revision" err

so I wonder why that works.

Why should it not work? If GIT_TEST_GETTEXT_POISON is on, it pretends success; otherwise, it does a regular grep.

-- Hannes

#9 lore raw

Re: [PATCH 02/17] t0001 (mingw): do not expect a specific order of stdout/stderr

Hi Hannes,

Quoted 10 lines · Johannes Sixt

Am 19.06.19 um 13:30 schrieb Johannes Schindelin:

Quoted 6 lines

Interesting side note: I just realized that t6050-replace.sh does indeed contain

test_i18ngrep "Needed a single revision" err

so I wonder why that works.

Why should it not work? If GIT_TEST_GETTEXT_POISON is on, it pretends success; otherwise, it does a regular grep.

Ah, right, I vaguely remembered that Duy offered patches to do something funny with a pseudo locale (I never understood why not go for the much simpler ROT-13), and I guess that effort never went anywhere.

So essentially the test case in t6050 will succeed under GETTEXT_POISON for the wrong reasons.

Thanks for explaining this to me!

Ciao,
Dscho

#17 lore raw

Re: [PATCH 03/17] cache-tree.c: avoid reusing the DEBUG constant

In reply to Carlo Arenas

Hi Carlo,

Quoted 4 lines · Carlo Arenas

while those two changes (from DEBUG to DEBUG_$foo) are worth doing in their own merit, I am more inclined to consider this as orthogonal since by your own description[1] the right name to use would be _DEBUG (with a preceding dash) and that would obviously not conflict here.

Well, both _DEBUG and DEBUG appear to be defined by Visual Studio in debug mode. So we still need to rename them, and I'd rather rename them here than in the next patch series (that will (re-)add support to build in Visual Studio), as I want the command-line MSVC build to also define both constants.

Quoted 2 lines

the only remaining change then would be to drop the -DDEBUG that gets added to your BASIC_CFLAGS

No, for consistency with Visual Studio, I want to keep defining both DEBUG and _DEBUG. It's just easier that way, as I won't have to spend extra cycles to remember that those two ways to build Git are slightly different, when they shouldn't be.

Thanks,
Dscho

Quoted 4 lines

Carlo
[1] https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019

#38 lore raw

v2 cover Fix MSVC support, at long last

In reply to Johannes Schindelin via GitGitGadget

Philip Oakley and Jeff Hostetler worked quite a bit on getting Git to compile with MS Visual C again, and this patch series is the culmination of those efforts. With these patches, it is as easy as

make MSVC=1

Note: the patches went through quite the number of iterations. For example, for a long time we targeted Visual Studio 2015, and used NuGet packages for the dependencies (such as OpenSSL, cURL, etc), while the current iteration targets Visual Studio 2017 and uses vcpkg
[https://docs.microsoft.com/en-us/cpp/vcpkg?view=vs-2017] for dependencies. Hopefully I did not forget to remove any remnants of those previous versions.

Please also note that this patch series is part 1 of 3 in a bigger story:
the next patch series will add support to build Git in Microsoft Visual Studio, and the third patch series will add Continuous Testing by adding an MSVC build and a corresponding parallelized test job to our Azure Pipeline.

Changes since v1:

* The BASIC_CLFAGS typo was fixed.
* Instead of sorting the output of stdout/stderr, the fixed test case in t0001 now greps for the tell-tales it wants to be present.
* In addition to cache-tree.c's DEBUG constant, now also builtin/blame.c's is renamed.
* Two changes were factored out of the patch titled "msvc: support building Git using MS Visual C++": the support for spaces in SANE_TOOL_PATH, and the support for the compile time flag to enable CrtDbg's detailed heap diagnostics.
* A comment about the vcxproj target has been dropped; The corresponding change is slated for a future patch series.
* A left-over "TODO" comment was replaced by stating the final decision that only Visual Studio 2015 or later are supported for now.
* A left-over, commented-out SIGINT case label was removed, and an adjacent comment was moved so that its indentation no longer looks strange.

Jeff Hostetler (10):
cache-tree/blame: avoid reusing the DEBUG constant
msvc: mark a variable as non-const
msvc: do not re-declare the timespec struct
msvc: define ftello()
msvc: fix detect_msys_tty()
msvc: update Makefile to allow for spaces in the compiler path
msvc: support building Git using MS Visual C++
msvc: add a compile-time flag to allow detailed heap debugging
msvc: do not pretend to support all signals
msvc: ignore .dll and incremental compile output

Johannes Schindelin (7):
mingw: fix a typo in the msysGit-specific section Mark .bat files as requiring CR/LF endings
t0001 (mingw): do not expect a specific order of stdout/stderr
obstack: fix compiler warning
mingw: replace mingw_startup() hack
msvc: fix dependencies of compat/msvc.c
msvc: avoid debug assertion windows in Debug Mode

Philip Oakley (3):
msvc: include sigset_t definition
msvc: define O_ACCMODE
msvc: add pragmas for common warnings

 .gitattributes                     |   1 +
 .gitignore                         |   5 +
 Makefile                           |  40 +++++++
 builtin/blame.c                    |   6 +-
 cache-tree.c                       |  14 +--
 compat/mingw.c                     |  99 +++++++++++++----
 compat/mingw.h                     |  24 +++--
 compat/msvc.h                      |  10 ++
 compat/obstack.h                   |   2 +-
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  39 +++++++
 compat/vcbuild/find_vs_env.bat     | 168 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 compat/winansi.c                   |  13 +++
 config.mak.uname                   |  83 +++++++++++---
 git-compat-util.h                  |   9 ++
 t/t0001-init.sh                    |   4 +-
 19 files changed, 619 insertions(+), 61 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat

base-commit: b697d92f56511e804b8ba20ccbe7bdc85dc66810
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-149%2Fdscho%2Fmsvc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-149/dscho/msvc-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/149

Range-diff vs v1:

  -:  ---------- >  1:  9d4d6ae7db mingw: fix a typo in the msysGit-specific section
  1:  b30429e0a4 =  2:  aa27e7f9cb Mark .bat files as requiring CR/LF endings
  2:  d551cdeafb <  -:  ---------- t0001 (mingw): do not expect a specific order of stdout/stderr
  -:  ---------- >  3:  3e108cfb02 t0001 (mingw): do not expect a specific order of stdout/stderr
  3:  486297ec8c !  4:  90ac088d22 cache-tree.c: avoid reusing the DEBUG constant

@@ -1,16 +1,40 @@
Author: Jeff Hostetler <[email protected]>

- cache-tree.c: avoid reusing the DEBUG constant
+ cache-tree/blame: avoid reusing the DEBUG constant

- In MSVC, the DEBUG constant is set automatically whenever compiling with
- debug information.
+ In MS Visual C, the DEBUG constant is set automatically whenever
+ compiling with debug information.

- This is clearly not what was intended in cache-tree.c, so let's use a less
- ambiguous constant there.
+ This is clearly not what was intended in cache-tree.c nor in
+ builtin/blame.c, so let's use a less ambiguous name there.

+ diff --git a/builtin/blame.c b/builtin/blame.c
+ --- a/builtin/blame.c
+ +++ b/builtin/blame.c
+@@
+
+ static struct string_list mailmap = STRING_LIST_INIT_NODUP; +
+-#ifndef DEBUG
+-#define DEBUG 0
++#ifndef DEBUG_BLAME
++#define DEBUG_BLAME 0
+ #endif
+
+ static unsigned blame_move_score;
+@@
+ if (blame_copy_score)
+ sb.copy_score = blame_copy_score;
+
+- sb.debug = DEBUG;
++ sb.debug = DEBUG_BLAME;
+ sb.on_sanity_fail = &sanity_check_on_fail;
+
+ sb.show_root = show_root;
+
diff --git a/cache-tree.c b/cache-tree.c
--- a/cache-tree.c
+++ b/cache-tree.c

  4:  bda1c270c1 =  5:  71d85b58b6 obstack: fix compiler warning
  5:  47105b6350 =  6:  02f618464f mingw: replace mingw_startup() hack
  6:  250385d7a3 =  7:  5c383985a1 msvc: fix dependencies of compat/msvc.c
  7:  037fbed117 =  8:  0a2985dc3c msvc: include sigset_t definition
  8:  b23651a357 =  9:  99a2939cc2 msvc: define O_ACCMODE
  9:  e813075ade = 10:  1542e8abe5 msvc: mark a variable as non-const
 10:  2ed60dfffc = 11:  51d73c61d3 msvc: do not re-declare the timespec struct
 11:  e6cd002806 = 12:  7cfd0fc9b4 msvc: define ftello()
 12:  073dc01b14 = 13:  f528883d97 msvc: fix detect_msys_tty()
  -:  ---------- > 14:  63bf9f1f92 msvc: update Makefile to allow for spaces in the compiler path
 13:  587cbcf619 ! 15:  245f28ac3b msvc: support building Git using MS Visual C++
     @@ -46,8 +46,8 @@
       
       ifdef SANE_TOOL_PATH
       SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
     --BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
     -+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
     +-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
     ++BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
       PATH := $(SANE_TOOL_PATH):${PATH}
       else
       BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
     @@ -122,19 +122,6 @@
       /*
        * We implement wmain() and compile with -municode, which would
        * normally ignore main(), but we call the latter from the former
     -@@
     - 
     - 	trace2_initialize_clock();
     - 
     -+#ifdef _MSC_VER
     -+#ifdef USE_MSVC_CRTDBG
     -+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
     -+#endif
     -+#endif
     -+
     - 	maybe_redirect_std_handles();
     - 
     - 	/* determine size of argv and environ conversion buffer */
      
       diff --git a/compat/vcbuild/.gitignore b/compat/vcbuild/.gitignore
       new file mode 100644
     @@ -188,18 +175,6 @@
      +
      +================================================================
      +
     -+Alternatively, run `make MSVC=1 vcxproj` and then load the generated
     -+git.sln in Visual Studio. The initial build will install the vcpkg
     -+system and build the dependencies automatically. This will take a while.
     -+
     -+Note that this will automatically add and commit the generated
     -+.sln and .vcxproj files to the repo.  You may want to drop this
     -+commit before submitting a Pull Request....
     -+
     -+Or maybe we should put the .sln/.vcxproj files in the .gitignore file
     -+and not do this.  I'm not sure.
     -+
     -+================================================================
       The Steps of Build Git with VS2008
       
       1. You need the build environment, which contains the Git dependencies
     @@ -264,7 +239,7 @@
      +REM
      +REM Earlier Versions
      +REM ----------------
     -+REM TODO
     ++REM Currently unsupported.
      +REM
      +REM ================================================================
      +REM Note: Throughout this script we use "dir <path> && <cmd>" rather
     @@ -353,8 +328,7 @@
      +REM ================================================================
      +
      +:not_2015
     -+   REM TODO....
     -+   echo TODO support older versions of VS. >&2
     ++   echo "ERROR: unsupported VS version (older than VS2015)" >&2
      +   EXIT /B 1
      +
      +REM ================================================================
     @@ -702,8 +676,6 @@
      +endif
      +	BASIC_CFLAGS += $(sdk_libs) $(msvc_libs)
      +
     -+	# Optionally enable memory leak reporting.
     -+	# BASIC_CLFAGS += -DUSE_MSVC_CRTDBG
       	BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1
      +	# Always give "-Zi" to the compiler and "-debug" to linker (even in
      +	# release mode) to force a PDB to be generated (like RelWithDebInfo).
     @@ -726,23 +698,3 @@
       endif
       	X = .exe
       
     -
     - diff --git a/git-compat-util.h b/git-compat-util.h
     - --- a/git-compat-util.h
     - +++ b/git-compat-util.h
     -@@
     - #ifndef GIT_COMPAT_UTIL_H
     - #define GIT_COMPAT_UTIL_H
     - 
     -+#ifdef USE_MSVC_CRTDBG
     -+/*
     -+ * For these to work they must appear very early in each
     -+ * file -- before most of the standard header files.
     -+ */
     -+#include <stdlib.h>
     -+#include <crtdbg.h>
     -+#endif
     -+
     - #define _FILE_OFFSET_BITS 64
     - 
     - 
  -:  ---------- > 16:  e7888bedbd msvc: add a compile-time flag to allow detailed heap debugging
 14:  0cca8c3c68 = 17:  d9668558dd msvc: add pragmas for common warnings
 15:  1491ea4140 ! 18:  c883f037e0 msvc: do not pretend to support all signals

@@ -17,14 +17,6 @@
return 0;

+#if defined(_MSC_VER)
-+ /*
-+ * <signal.h> in the CRT defines 8 signals as being
-+ * supported on the platform. Anything else causes
-+ * an "Invalid signal or error" (which in DEBUG builds
-+ * causes the Abort/Retry/Ignore dialog). We by-pass
-+ * the CRT for things we already know will fail.
-+ */
-+ /*case SIGINT:*/
+ case SIGILL:
+ case SIGFPE:
+ case SIGSEGV:
@@ -32,6 +24,13 @@
+ case SIGBREAK:
+ case SIGABRT:
+ case SIGABRT_COMPAT:
++ /*
++ * The <signal.h> header in the MS C Runtime defines 8 signals
++ * as being supported on the platform. Anything else causes an
++ * "Invalid signal or error" (which in DEBUG builds causes the
++ * Abort/Retry/Ignore dialog). We by-pass the CRT for things we
++ * already know will fail.
++ */
+ return raise(sig);
+ default:
+ errno = EINVAL;

 16:  831d603e25 = 19:  51a20ff2df msvc: avoid debug assertion windows in Debug Mode
 17:  0c7f5479bf = 20:  4d44d1fab1 msvc: ignore .dll and incremental compile output
Signature · 1 line
gitgitgadget
#39 lore raw

PATCH v2 1/20 mingw: fix a typo in the msysGit-specific section

From: Johannes Schindelin <[email protected]>

The msysGit project (i.e. Git for Windows 1.x' SDK) is safely dead for *years* already. This is probably the reason why nobody caught this typo until Carlo Arenas spotted a copy-edited version of it nearby.

It is probably about time to rip out the remainders of msysGit/MSys1 support, but that can safely wait a bit longer, and we can at least fix the typo for now.

---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
config.mak.uname +1 -1
index b71688eeb7..9fc053cac0 100644
@@ -571,7 +571,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
 	INTERNAL_QSORT = YesPlease
 	HAVE_LIBCHARSET_H = YesPlease
 	NO_GETTEXT = YesPlease
-	COMPAT_CLFAGS += -D__USE_MINGW_ACCESS
+	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS
 else
 	ifneq ($(shell expr "$(uname_R)" : '1\.'),2)
 		# MSys2
Signature · 1 line
gitgitgadget
#40 lore raw

PATCH v2 2/20 Mark .bat files as requiring CR/LF endings

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Just like the natural line ending for Unix shell scripts consist of a single Line Feed, the natural line ending for (DOS) Batch scripts consists of a Carriage Return followed by a Line Feed.

It seems that both Unix shell script interpreters and the interpreter for Batch scripts (cmd.exe) are keen on seeing the "right" line endings.

---
 .gitattributes | 1 +
 1 file changed, 1 insertion(+)
.gitattributes +1 -0
index 9fa72ad450..b08a1416d8 100644
@@ -5,6 +5,7 @@
 *.pl eof=lf diff=perl
 *.pm eol=lf diff=perl
 *.py eol=lf diff=python
+*.bat eol=crlf
 /Documentation/**/*.txt eol=lf
 /command-list.txt eol=lf
 /GIT-VERSION-GEN eol=lf
Signature · 1 line
gitgitgadget
#41 lore raw

PATCH v2 3/20 t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When redirecting stdout/stderr to the same file, we cannot guarantee that stdout will come first.

In fact, in this test case, it seems that an MSVC build always prints stderr first.

In any case, this test case does not want to verify the *order* but the *presence* of both outputs, so let's test exactly that.

---
 t/t0001-init.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
t/t0001-init.sh +2 -2
index 77a224aafb..387e4e6b81 100755
@@ -473,8 +473,8 @@ test_expect_success MINGW 'redirect std handles' '
 		GIT_REDIRECT_STDOUT=output.txt \
 		GIT_REDIRECT_STDERR="2>&1" \
 		git rev-parse --git-dir --verify refs/invalid &&
-	printf ".git\nfatal: Needed a single revision\n" >expect &&
-	test_cmp expect output.txt
+	grep "^\\.git\$" output.txt &&
+	grep "Needed a single revision" output.txt
 '
 
 test_done
Signature · 1 line
gitgitgadget
#44 lore raw

PATCH v2 6/20 mingw: replace mingw_startup() hack

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Git for Windows has special code to retrieve the command-line parameters (and even the environment) in UTF-16 encoding, so that they can be converted to UTF-8. This is necessary because Git for Windows wants to use UTF-8 encoded strings throughout its code, and the main() function does not get the parameters in that encoding.

To do that, we used the __wgetmainargs() function, which is not even a Win32 API function, but provided by the MINGW "runtime" instead.

Obviously, this method would not work with any compiler other than GCC, and in preparation for compiling with Visual C++, we would like to avoid precisely that.

Lucky us, there is a much more elegant way: we can simply implement the UTF-16 variant of main(): wmain().

To make that work, we need to link with -municode. The command-line parameters are passed to wmain() encoded in UTF-16, as desired, and this method also works with GCC, and also with Visual C++ after adjusting the MSVC linker flags to force it to use wmain().

---
 compat/mingw.c   | 53 +++++++++++++++++++++++++++++++-----------------
 compat/mingw.h   | 22 ++++++++++----------
 config.mak.uname |  3 ++-
 3 files changed, 47 insertions(+), 31 deletions(-)
compat/mingw.c +34 -19
index 9b6d2400e1..0d8713e515 100644
@@ -2301,18 +2301,13 @@ static void setup_windows_environment(void)
 		setenv("TERM", "cygwin", 1);
 }
 
+#if !defined(_MSC_VER)
 /*
  * Disable MSVCRT command line wildcard expansion (__getmainargs called from
  * mingw startup code, see init.c in mingw runtime).
  */
 int _CRT_glob = 0;
-
-typedef struct {
-	int newmode;
-} _startupinfo;
-
-extern int __wgetmainargs(int *argc, wchar_t ***argv, wchar_t ***env, int glob,
-		_startupinfo *si);
+#endif
 
 static NORETURN void die_startup(void)
 {
@@ -2390,22 +2385,25 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
-void mingw_startup(void)
+/*
+ * We implement wmain() and compile with -municode, which would
+ * normally ignore main(), but we call the latter from the former
+ * so that we can handle non-ASCII command-line parameters
+ * appropriately.
+ *
+ * To be more compatible with the core git code, we convert
+ * argv into UTF8 and pass them directly to main().
+ */
+int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, argc;
-	char *buffer;
-	wchar_t **wenv, **wargv;
-	_startupinfo si;
+	int i, maxlen, exit_status;
+	char *buffer, **save;
+	const char **argv;
 
 	trace2_initialize_clock();
 
 	maybe_redirect_std_handles();
 
-	/* get wide char arguments and environment */
-	si.newmode = 0;
-	if (__wgetmainargs(&argc, &wargv, &wenv, _CRT_glob, &si) < 0)
-		die_startup();
-
 	/* determine size of argv and environ conversion buffer */
 	maxlen = wcslen(wargv[0]);
 	for (i = 1; i < argc; i++)
@@ -2415,9 +2413,16 @@ void mingw_startup(void)
 	maxlen = 3 * maxlen + 1;
 	buffer = malloc_startup(maxlen);
 
-	/* convert command line arguments and environment to UTF-8 */
+	/*
+	 * Create a UTF-8 version of w_argv. Also create a "save" copy
+	 * to remember all the string pointers because parse_options()
+	 * will remove claimed items from the argv that we pass down.
+	 */
+	ALLOC_ARRAY(argv, argc + 1);
+	ALLOC_ARRAY(save, argc + 1);
 	for (i = 0; i < argc; i++)
-		__argv[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+		argv[i] = save[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+	argv[i] = save[i] = NULL;
 	free(buffer);
 
 	/* fix Windows specific environment settings */
@@ -2436,6 +2441,16 @@ void mingw_startup(void)
 
 	/* initialize Unicode console */
 	winansi_init();
+
+	/* invoke the real main() using our utf8 version of argv. */
+	exit_status = main(argc, argv);
+
+	for (i = 0; i < argc; i++)
+		free(save[i]);
+	free(save);
+	free(argv);
+
+	return exit_status;
 }
 
 int uname(struct utsname *buf)
compat/mingw.h +11 -11
index 593bdbffe6..210f1b01a8 100644
@@ -562,18 +562,18 @@ int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen);
 extern CRITICAL_SECTION pinfo_cs;
 
 /*
- * A replacement of main() that adds win32 specific initialization.
+ * Git, like most portable C applications, implements a main() function. On
+ * Windows, this main() function would receive parameters encoded in the
+ * current locale, but Git for Windows would prefer UTF-8 encoded  parameters.
+ *
+ * To make that happen, we still declare main() here, and then declare and
+ * implement wmain() (which is the Unicode variant of main()) and compile with
+ * -municode. This wmain() function reencodes the parameters from UTF-16 to
+ * UTF-8 format, sets up a couple of other things as required on Windows, and
+ * then hands off to the main() function.
  */
-
-void mingw_startup(void);
-#define main(c,v) dummy_decl_mingw_main(void); \
-static int mingw_main(c,v); \
-int main(int argc, const char **argv) \
-{ \
-	mingw_startup(); \
-	return mingw_main(__argc, (void *)__argv); \
-} \
-static int mingw_main(c,v)
+int wmain(int argc, const wchar_t **w_argv);
+int main(int argc, const char **argv);
 
 /*
  * Used by Pthread API implementation for Windows
config.mak.uname +2 -1
index 9fc053cac0..6ddece0350 100644
@@ -401,7 +401,7 @@ ifeq ($(uname_S),Windows)
 		compat/win32/trace2_win32_process_info.o \
 		compat/win32/dirent.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
-	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE
+	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -ENTRY:wmainCRTStartup -SUBSYSTEM:CONSOLE
 	EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj
 	PTHREAD_LIBS =
 	lib =
@@ -548,6 +548,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	ETAGS_TARGET = ETAGS
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	DEFAULT_HELP_FORMAT = html
+	BASIC_LDFLAGS += -municode
 	COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
Signature · 1 line
gitgitgadget
#45 lore raw

PATCH v2 7/20 msvc: fix dependencies of compat/msvc.c

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The file compat/msvc.c includes compat/mingw.c, which means that we have to recompile compat/msvc.o if compat/mingw.c changes.

---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)
config.mak.uname +2 -0
index 6ddece0350..473613a20d 100644
@@ -414,6 +414,8 @@ else
 	BASIC_CFLAGS += -Zi -MDd
 endif
 	X = .exe
+
+compat/msvc.o: compat/msvc.c compat/mingw.c GIT-CFLAGS
 endif
 ifeq ($(uname_S),Interix)
 	NO_INITGROUPS = YesPlease
Signature · 1 line
gitgitgadget
#43 lore raw

PATCH v2 5/20 obstack: fix compiler warning

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

MS Visual C suggests that the construct

condition ? (int) i : (ptrdiff_t) d

is incorrect. Let's fix this by casting to ptrdiff_t also for the positive arm of the conditional.

---
 compat/obstack.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
compat/obstack.h +1 -1
index ced94d0118..ae36ed6a66 100644
@@ -496,7 +496,7 @@ __extension__								\
 ( (h)->temp.tempint = (char *) (obj) - (char *) (h)->chunk,		\
   ((((h)->temp.tempint > 0						\
     && (h)->temp.tempint < (h)->chunk_limit - (char *) (h)->chunk))	\
-   ? (int) ((h)->next_free = (h)->object_base				\
+   ? (ptrdiff_t) ((h)->next_free = (h)->object_base				\
 	    = (h)->temp.tempint + (char *) (h)->chunk)			\
    : (((obstack_free) ((h), (h)->temp.tempint + (char *) (h)->chunk), 0), 0)))
 
Signature · 1 line
gitgitgadget
#47 lore raw

PATCH v2 9/20 msvc: define O_ACCMODE

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

This constant is not defined in MSVC's headers.

In UCRT's fcntl.h, _O_RDONLY, _O_WRONLY and _O_RDWR are defined as 0, 1 and 2, respectively. Yes, that means that UCRT breaks with the tradition that O_RDWR == O_RDONLY | O_WRONLY.

It is a perfectly legal way to define those constants, though, therefore we need to take care of defining O_ACCMODE accordingly.

This is particularly important in order to keep our "open() can set errno to EISDIR" emulation working: it tests that (flags & O_ACCMODE) is not identical to O_RDONLY before going on to test specifically whether the file for which open() reported EACCES is, in fact, a directory.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 04b4750b87..d336d80670 100644
@@ -19,6 +19,8 @@
 #undef ERROR
 
 typedef int sigset_t;
+/* open for reading, writing, or both (not in fcntl.h) */
+#define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
 
 #include "compat/mingw.h"
 
Signature · 1 line
gitgitgadget
#48 lore raw

PATCH v2 10/20 msvc: mark a variable as non-const

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015 complains when using a const pointer in memcpy()/free().

---
 compat/mingw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
compat/mingw.c +4 -1
index 0d8713e515..d14d33308d 100644
@@ -1553,7 +1553,10 @@ static int try_shell_exec(const char *cmd, char *const *argv)
 	if (prog) {
 		int exec_id;
 		int argc = 0;
-		const char **argv2;
+#ifndef _MSC_VER
+		const
+#endif
+		char **argv2;
 		while (argv[argc]) argc++;
 		ALLOC_ARRAY(argv2, argc + 1);
 		argv2[0] = (char *)cmd;	/* full path to the script file */
Signature · 1 line
gitgitgadget
#50 lore raw

PATCH v2 12/20 msvc: define ftello()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

It is just called differently in MSVC's headers.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index d336d80670..d7525cf61d 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+#define ftello _ftelli64
+
 typedef int sigset_t;
 /* open for reading, writing, or both (not in fcntl.h) */
 #define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
Signature · 1 line
gitgitgadget
#56 lore raw

PATCH v2 16/20 msvc: add a compile-time flag to allow detailed heap debugging

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

MS Visual C comes with a few neat features we can use to analyze the heap consumption (i.e. leaks, max memory, etc).

With this patch, we introduce support via the build-time flag USE_MSVC_CRTDBG.

---
 compat/mingw.c    | 6 ++++++
 config.mak.uname  | 4 ++++
 git-compat-util.h | 9 +++++++++
 3 files changed, 19 insertions(+)
compat/mingw.c +6 -0
index c063ae62be..667285887a 100644
@@ -2411,6 +2411,12 @@ int wmain(int argc, const wchar_t **wargv)
 
 	trace2_initialize_clock();
 
+#ifdef _MSC_VER
+#ifdef USE_MSVC_CRTDBG
+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+#endif
+#endif
+
 	maybe_redirect_std_handles();
 
 	/* determine size of argv and environ conversion buffer */
config.mak.uname +4 -0
index b8c52e49d2..3fde48c64d 100644
@@ -448,6 +448,10 @@ else
 endif
 	BASIC_CFLAGS += $(sdk_libs) $(msvc_libs)
 
+ifneq ($(USE_MSVC_CRTDBG),)
+	# Optionally enable memory leak reporting.
+	BASIC_CFLAGS += -DUSE_MSVC_CRTDBG
+endif
 	BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1
 	# Always give "-Zi" to the compiler and "-debug" to linker (even in
 	# release mode) to force a PDB to be generated (like RelWithDebInfo).
git-compat-util.h +9 -0
index cc0e7e9733..83be89de0a 100644
@@ -1,6 +1,15 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#ifdef USE_MSVC_CRTDBG
+/*
+ * For these to work they must appear very early in each
+ * file -- before most of the standard header files.
+ */
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #define _FILE_OFFSET_BITS 64
 
 
Signature · 1 line
gitgitgadget
#57 lore raw

PATCH v2 17/20 msvc: add pragmas for common warnings

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

MSVC can be overzealous about some warnings. Disable them.

---
 compat/msvc.h | 4 ++++
 1 file changed, 4 insertions(+)
compat/msvc.h +4 -0
index d7525cf61d..1d7a8c6145 100644
@@ -6,6 +6,10 @@
 #include <malloc.h>
 #include <io.h>
 
+#pragma warning(disable: 4018) /* signed/unsigned comparison */
+#pragma warning(disable: 4244) /* type conversion, possible loss of data */
+#pragma warning(disable: 4090) /* 'function' : different 'const' qualifiers (ALLOC_GROW etc.)*/
+
 /* porting function */
 #define inline __inline
 #define __inline__ __inline
Signature · 1 line
gitgitgadget
#53 lore raw

PATCH v2 15/20 msvc: support building Git using MS Visual C++

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

With this patch, Git can be built using the Microsoft toolchain, via:

	make MSVC=1 [DEBUG=1]

Third party libraries are built from source using the open source "vcpkg" tool set. See https://github.com/Microsoft/vcpkg

On a first build, the vcpkg tools and the third party libraries are automatically downloaded and built. DLLs for the third party libraries are copied to the top-level (and t/helper) directory to facilitate debugging. See compat/vcbuild/README.

A series of .bat files are invoked by the Makefile to find the location of the installed version of Visual Studio and the associated compiler tools (essentially replicating the environment setup performed by a "Developer Command Prompt"). This should find the most recent VS2015 or VS2017 installation. Output from these scripts are used by the Makefile to define compiler and linker pathnames and -I and -L arguments.

The build produces .pdb files for both debug and release builds.

Note: This commit was squashed from an organic series of commits developed between 2016 and 2018 in Git for Windows' master branch. This combined commit eliminates the obsolete commits related to fetching NuGet packages for third party libraries. It is difficult to use NuGet packages for C/C++ sources because they may be built by earlier versions of the MSVC compiler and have CRT version and linking issues.

Additionally, the C/C++ NuGet packages that we were using tended to not be updated concurrently with the sources. And in the case of cURL and OpenSSL, this could expose us to security issues.

---
 Makefile                           |  42 +++++++-
 compat/mingw.c                     |   6 ++
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  39 +++++++
 compat/vcbuild/find_vs_env.bat     | 168 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 config.mak.uname                   |  72 +++++++++++--
 9 files changed, 474 insertions(+), 16 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat
Makefile +41 -1
index 3cf8cc8ffd..2b66d5a3f3 100644
@@ -1240,7 +1240,7 @@ endif
 
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
 PATH := $(SANE_TOOL_PATH):${PATH}
 else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
@@ -2873,6 +2873,33 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+ifdef MSVC
+	# We DO NOT install the individual foo.o.pdb files because they
+	# have already been rolled up into the exe's pdb file.
+	# We DO NOT have pdb files for the builtin commands (like git-status.exe)
+	# because it is just a copy/hardlink of git.exe, rather than a unique binary.
+	$(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ifndef DEBUG
+	$(INSTALL) $(vcpkg_rel_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_rel_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+else
+	$(INSTALL) $(vcpkg_dbg_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_dbg_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 	$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
@@ -3085,6 +3112,19 @@ endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
 	$(RM) GIT-USER-AGENT GIT-PREFIX
 	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
+ifdef MSVC
+	$(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS))
+	$(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(TEST_PROGRAMS))
+	$(RM) compat/vcbuild/MSVC-DEFS-GEN
+endif
 
 .PHONY: all install profile-clean cocciclean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
compat/mingw.c +6 -0
index d14d33308d..c063ae62be 100644
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
+#ifdef _MSC_VER
+#ifdef _DEBUG
+#include <crtdbg.h>
+#endif
+#endif
+
 /*
  * We implement wmain() and compile with -municode, which would
  * normally ignore main(), but we call the latter from the former
compat/vcbuild/.gitignore new file +3 -0
new file mode 100644
index 0000000000..8f8b794ef3
@@ -0,0 +1,3 @@
+/vcpkg/
+/MSVC-DEFS-GEN
+/VCPKG-DEFS
compat/vcbuild/README +39 -0
index 60fd873fe8..b633e7db98 100644
@@ -1,3 +1,42 @@
+The Steps to Build Git with VS2015 or VS2017 from the command line.
+
+1. Install the "vcpkg" open source package manager and build essential
+   third-party libraries.  The steps for this have been captured in a
+   set of convenience scripts.  These can be run from a stock Command
+   Prompt or from an SDK bash window:
+
+   $ cd <repo_root>
+   $ ./compat/vcbuild/vcpkg_install.bat
+
+   The vcpkg tools and all of the third-party sources will be installed
+   in this folder:
+      <repo_root>/compat/vcbuild/vcpkg/
+
+   A file will be created with a set of Makefile macros pointing to a
+   unified "include", "lib", and "bin" directory (release and debug) for
+   all of the required packages.  This file will be included by the main
+   Makefile:
+      <repo_root>/compat/vcbuild/MSVC-DEFS-GEN
+
+2. OPTIONALLY copy the third-party *.dll and *.pdb files into the repo
+   root to make it easier to run and debug git.exe without having to
+   manipulate your PATH.  This is especially true for debug sessions in
+   Visual Studio.
+
+   Use ONE of the following forms which should match how you want to
+   compile git.exe.
+
+   $ ./compat/vcbuild/vcpkg_copy_packages.bat debug
+   $ ./compat/vcbuild/vcpkg_copy_packages.bat release
+
+3. Build git using MSVC from an SDK bash window using one of the
+   following commands:
+
+   $ make MSVC=1
+   $ make MSVC=1 DEBUG=1
+
+================================================================
+
 The Steps of Build Git with VS2008
 
 1. You need the build environment, which contains the Git dependencies
compat/vcbuild/find_vs_env.bat new file +168 -0

168 lines not on this page · raw message

compat/vcbuild/scripts/clink.pl +36 -5

78 lines not on this page · raw message

compat/vcbuild/vcpkg_copy_dlls.bat new file +39 -0
new file mode 100644
index 0000000000..13661c14f8
@@ -0,0 +1,39 @@
+@ECHO OFF
+REM ================================================================
+REM This script is an optional step. It copies the *.dll and *.pdb
+REM files (created by vcpkg_install.bat) into the top-level directory
+REM of the repo so that you can type "./git.exe" and find them without
+REM having to fixup your PATH.
+REM
+REM NOTE: Because the names of some DLL files change between DEBUG and
+REM NOTE: RELEASE builds when built using "vcpkg.exe", you will need
+REM NOTE: to copy up the corresponding version.
+REM ================================================================
+
+	SETLOCAL EnableDelayedExpansion
+
+	@FOR /F "delims=" %%D IN ("%~dp0") DO @SET cwd=%%~fD
+	cd %cwd%
+
+	SET arch=x64-windows
+	SET inst=%cwd%vcpkg\installed\%arch%
+
+	IF [%1]==[release] (
+		echo Copying RELEASE mode DLLs to repo root...
+	) ELSE IF [%1]==[debug] (
+		SET inst=%inst%\debug
+		echo Copying DEBUG mode DLLs to repo root...
+	) ELSE (
+		echo ERROR: Invalid argument.
+		echo Usage: %~0 release
+		echo Usage: %~0 debug
+		EXIT /B 1
+	)
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\t\helper\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\t\helper\
+
+	EXIT /B 0
compat/vcbuild/vcpkg_install.bat new file +80 -0

80 lines not on this page · raw message

config.mak.uname +62 -10

121 lines not on this page · raw message

Signature · 1 line
gitgitgadget
#59 lore raw

PATCH v2 19/20 msvc: avoid debug assertion windows in Debug Mode

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

For regular debugging, it is pretty helpful when a debug assertion in a running application triggers a window that offers to start the debugger.

However, when running the test suite, it is not so helpful, in particular when the debug assertions are then suppressed anyway because we disable the invalid parameter checking (via invalidcontinue.obj, see the comment in config.mak.uname about that object for more information).

So let's simply disable that window in Debug Mode (it is already disabled in Release Mode).

---
 compat/mingw.c | 4 ++++
 1 file changed, 4 insertions(+)
compat/mingw.c +4 -0
index d01e88c2f8..433838391f 100644
@@ -2437,6 +2437,10 @@ int wmain(int argc, const wchar_t **wargv)
 	trace2_initialize_clock();
 
 #ifdef _MSC_VER
+#ifdef _DEBUG
+	_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG);
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
 #endif
Signature · 1 line
gitgitgadget
#60 lore raw

PATCH v2 20/20 msvc: ignore .dll and incremental compile output

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

Ignore .dll files copied into the top-level directory. Ignore MSVC incremental compiler output files.

---
 .gitignore | 5 +++++
 1 file changed, 5 insertions(+)
.gitignore +5 -0
index 2374f77a1a..ba0e52c4d1 100644
@@ -226,6 +226,11 @@
 *.user
 *.idb
 *.pdb
+*.ilk
+*.iobj
+*.ipdb
+*.dll
+.vs/
 /Debug/
 /Release/
 *.dSYM
Signature · 1 line
gitgitgadget
#51 lore raw

PATCH v2 13/20 msvc: fix detect_msys_tty()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

The ntstatus.h header is only available in MINGW.

---
 compat/winansi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
compat/winansi.c +13 -0
index f4f08237f9..11cd9b82cc 100644
@@ -544,7 +544,20 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
 #ifdef DETECT_MSYS_TTY
 
 #include <winternl.h>
+
+#if defined(_MSC_VER)
+
+typedef struct _OBJECT_NAME_INFORMATION
+{
+	UNICODE_STRING Name;
+	WCHAR NameBuffer[0];
+} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
+
+#define ObjectNameInformation 1
+
+#else
 #include <ntstatus.h>
+#endif
 
 static void detect_msys_tty(int fd)
 {
Signature · 1 line
gitgitgadget
#58 lore raw

PATCH v2 18/20 msvc: do not pretend to support all signals

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

This special-cases various signals that are not supported on Windows, such as SIGPIPE. These cause the UCRT to throw asserts (at least in debug mode).

---
 compat/mingw.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
compat/mingw.c +25 -0
index 667285887a..d01e88c2f8 100644
@@ -2119,8 +2119,33 @@ int mingw_raise(int sig)
 			sigint_fn(SIGINT);
 		return 0;
 
+#if defined(_MSC_VER)
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGTERM:
+	case SIGBREAK:
+	case SIGABRT:
+	case SIGABRT_COMPAT:
+		/*
+		 * The <signal.h> header in the MS C Runtime defines 8 signals
+		 * as being supported on the platform. Anything else causes an
+		 * "Invalid signal or error" (which in DEBUG builds causes the
+		 * Abort/Retry/Ignore dialog). We by-pass the CRT for things we
+		 * already know will fail.
+		 */
+		return raise(sig);
+	default:
+		errno = EINVAL;
+		return -1;
+
+#else
+
 	default:
 		return raise(sig);
+
+#endif
+
 	}
 }
 
Signature · 1 line
gitgitgadget
#42 lore raw

PATCH v2 4/20 cache-tree/blame: avoid reusing the DEBUG constant

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

In MS Visual C, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c nor in builtin/blame.c, so let's use a less ambiguous name there.

---
 builtin/blame.c |  6 +++---
 cache-tree.c    | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)
builtin/blame.c +3 -3
index 21cde57e71..50e3d4a265 100644
@@ -59,8 +59,8 @@ static size_t blame_date_width;
 
 static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 
-#ifndef DEBUG
-#define DEBUG 0
+#ifndef DEBUG_BLAME
+#define DEBUG_BLAME 0
 #endif
 
 static unsigned blame_move_score;
@@ -1062,7 +1062,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	if (blame_copy_score)
 		sb.copy_score = blame_copy_score;
 
-	sb.debug = DEBUG;
+	sb.debug = DEBUG_BLAME;
 	sb.on_sanity_fail = &sanity_check_on_fail;
 
 	sb.show_root = show_root;
cache-tree.c +7 -7
index b13bfaf71e..706ffcf188 100644
@@ -6,8 +6,8 @@
 #include "object-store.h"
 #include "replace-object.h"
 
-#ifndef DEBUG
-#define DEBUG 0
+#ifndef DEBUG_CACHE_TREE
+#define DEBUG_CACHE_TREE 0
 #endif
 
 struct cache_tree *cache_tree(void)
@@ -111,7 +111,7 @@ static int do_invalidate_path(struct cache_tree *it, const char *path)
 	int namelen;
 	struct cache_tree_sub *down;
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree invalidate <%s>\n", path);
 #endif
 
@@ -398,7 +398,7 @@ static int update_one(struct cache_tree *it,
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 		fprintf(stderr, "cache-tree update-one %o %.*s\n",
 			mode, entlen, path + baselen);
 #endif
@@ -421,7 +421,7 @@ static int update_one(struct cache_tree *it,
 
 	strbuf_release(&buffer);
 	it->entry_count = to_invalidate ? -1 : i - *skip_count;
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
 		it->entry_count, it->subtree_nr,
 		oid_to_hex(&it->oid));
@@ -462,7 +462,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
 	strbuf_add(buffer, path, pathlen);
 	strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%.*s> (%d ent, %d subtree) %s\n",
 			pathlen, path, it->entry_count, it->subtree_nr,
@@ -536,7 +536,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
 		size -= rawsz;
 	}
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%s> (%d ent, %d subtree) %s\n",
 			*buffer, it->entry_count, subtree_nr,
Signature · 1 line
gitgitgadget
#52 lore raw

PATCH v2 14/20 msvc: update Makefile to allow for spaces in the compiler path

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

It is quite common that MS Visual C++ is installed into a location whose path contains spaces, therefore we need to quote it.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index 8a7e235352..3cf8cc8ffd 100644
@@ -1240,7 +1240,7 @@ endif
 
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
 PATH := $(SANE_TOOL_PATH):${PATH}
 else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
Signature · 1 line
gitgitgadget
#49 lore raw

PATCH v2 11/20 msvc: do not re-declare the timespec struct

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015's headers already declare that struct.

---
 compat/mingw.h | 2 ++
 1 file changed, 2 insertions(+)
compat/mingw.h +2 -0
index 210f1b01a8..a03e40e6e2 100644
@@ -352,11 +352,13 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 #ifndef __MINGW64_VERSION_MAJOR
 #define off_t off64_t
 #define lseek _lseeki64
+#ifndef _MSC_VER
 struct timespec {
 	time_t tv_sec;
 	long tv_nsec;
 };
 #endif
+#endif
 
 struct mingw_stat {
     _dev_t st_dev;
Signature · 1 line
gitgitgadget
#46 lore raw

PATCH v2 8/20 msvc: include sigset_t definition

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

On MSVC (VS2008) sigset_t is not defined.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 29a8ce8204..04b4750b87 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+typedef int sigset_t;
+
 #include "compat/mingw.h"
 
 #endif
Signature · 1 line
gitgitgadget
#54 lore raw

Re: [PATCH v2 15/20] msvc: support building Git using MS Visual C++

In reply to Jeff Hostetler via GitGitGadget

Hi,

Makefile endif · line 1243

Quoted 10 lines · Jeff Hostetler via GitGitGadget
Makefile +1 -1
index 3cf8cc8ffd..2b66d5a3f3 100644
@@ -1240,7 +1240,7 @@ endif
 
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'

Aaargh! I totally managed to mess this split up. This line should *not* be touched by this patch.

Will fix locally, in preparation for the next iteration.

Sorry,
Dscho

#55 lore raw

Re: [PATCH v2 15/20] msvc: support building Git using MS Visual C++

Quoted 9 lines · Johannes Schindelin

Makefile endif · line 1243

Quoted 4 lines

ifdef SANE_TOOL_PATH
SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))

Makefile +1 -1
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'

Aaargh! I totally managed to mess this split up. This line should *not* be touched by this patch.

Will fix locally, in preparation for the next iteration.

Thanks for stopping me soon enough. I am in the middle of picking which ones should go 'next', as we can now start 'next' from the tip of 'master'---hat I pushed out some time ago has master and next with identical trees ;-)

#61 lore raw

v3 cover Fix MSVC support, at long last

In reply to Johannes Schindelin via GitGitGadget

Philip Oakley and Jeff Hostetler worked quite a bit on getting Git to compile with MS Visual C again, and this patch series is the culmination of those efforts. With these patches, it is as easy as

make MSVC=1

Note: the patches went through quite the number of iterations. For example, for a long time we targeted Visual Studio 2015, and used NuGet packages for the dependencies (such as OpenSSL, cURL, etc), while the current iteration targets Visual Studio 2017 and uses vcpkg
[https://docs.microsoft.com/en-us/cpp/vcpkg?view=vs-2017] for dependencies. Hopefully I did not forget to remove any remnants of those previous versions.

Please also note that this patch series is part 1 of 3 in a bigger story:
the next patch series will add support to build Git in Microsoft Visual Studio, and the third patch series will add Continuous Testing by adding an MSVC build and a corresponding parallelized test job to our Azure Pipeline.

Changes since v2:

* Fixed the incorrect split-out of the "msvc: update Makefile to allow for spaces in the compiler path" patch: I had accidentally reverted that change in a later patch in the series.

Changes since v1:

* The BASIC_CLFAGS typo was fixed.
* Instead of sorting the output of stdout/stderr, the fixed test case in t0001 now greps for the tell-tales it wants to be present.
* In addition to cache-tree.c's DEBUG constant, now also builtin/blame.c's is renamed.
* Two changes were factored out of the patch titled "msvc: support building Git using MS Visual C++": the support for spaces in SANE_TOOL_PATH, and the support for the compile time flag to enable CrtDbg's detailed heap diagnostics.
* A comment about the vcxproj target has been dropped; The corresponding change is slated for a future patch series.
* A left-over "TODO" comment was replaced by stating the final decision that only Visual Studio 2015 or later are supported for now.
* A left-over, commented-out SIGINT case label was removed, and an adjacent comment was moved so that its indentation no longer looks strange.

Jeff Hostetler (10):
cache-tree/blame: avoid reusing the DEBUG constant
msvc: mark a variable as non-const
msvc: do not re-declare the timespec struct
msvc: define ftello()
msvc: fix detect_msys_tty()
msvc: update Makefile to allow for spaces in the compiler path
msvc: support building Git using MS Visual C++
msvc: add a compile-time flag to allow detailed heap debugging
msvc: do not pretend to support all signals
msvc: ignore .dll and incremental compile output

Johannes Schindelin (7):
mingw: fix a typo in the msysGit-specific section Mark .bat files as requiring CR/LF endings
t0001 (mingw): do not expect a specific order of stdout/stderr
obstack: fix compiler warning
mingw: replace mingw_startup() hack
msvc: fix dependencies of compat/msvc.c
msvc: avoid debug assertion windows in Debug Mode

Philip Oakley (3):
msvc: include sigset_t definition
msvc: define O_ACCMODE
msvc: add pragmas for common warnings

 .gitattributes                     |   1 +
 .gitignore                         |   5 +
 Makefile                           |  42 +++++++-
 builtin/blame.c                    |   6 +-
 cache-tree.c                       |  14 +--
 compat/mingw.c                     |  99 +++++++++++++----
 compat/mingw.h                     |  24 +++--
 compat/msvc.h                      |  10 ++
 compat/obstack.h                   |   2 +-
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  39 +++++++
 compat/vcbuild/find_vs_env.bat     | 168 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 compat/winansi.c                   |  13 +++
 config.mak.uname                   |  83 +++++++++++---
 git-compat-util.h                  |   9 ++
 t/t0001-init.sh                    |   4 +-
 19 files changed, 620 insertions(+), 62 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat

base-commit: b697d92f56511e804b8ba20ccbe7bdc85dc66810
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-149%2Fdscho%2Fmsvc-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-149/dscho/msvc-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/149

Range-diff vs v2:

  1:  9d4d6ae7db =  1:  9d4d6ae7db mingw: fix a typo in the msysGit-specific section
  2:  aa27e7f9cb =  2:  aa27e7f9cb Mark .bat files as requiring CR/LF endings
  3:  3e108cfb02 =  3:  3e108cfb02 t0001 (mingw): do not expect a specific order of stdout/stderr
  4:  90ac088d22 =  4:  90ac088d22 cache-tree/blame: avoid reusing the DEBUG constant
  5:  71d85b58b6 =  5:  71d85b58b6 obstack: fix compiler warning
  6:  02f618464f =  6:  02f618464f mingw: replace mingw_startup() hack
  7:  5c383985a1 =  7:  5c383985a1 msvc: fix dependencies of compat/msvc.c
  8:  0a2985dc3c =  8:  0a2985dc3c msvc: include sigset_t definition
  9:  99a2939cc2 =  9:  99a2939cc2 msvc: define O_ACCMODE
 10:  1542e8abe5 = 10:  1542e8abe5 msvc: mark a variable as non-const
 11:  51d73c61d3 = 11:  51d73c61d3 msvc: do not re-declare the timespec struct
 12:  7cfd0fc9b4 = 12:  7cfd0fc9b4 msvc: define ftello()
 13:  f528883d97 = 13:  f528883d97 msvc: fix detect_msys_tty()
 14:  63bf9f1f92 = 14:  63bf9f1f92 msvc: update Makefile to allow for spaces in the compiler path
 15:  245f28ac3b ! 15:  fee0ad048e msvc: support building Git using MS Visual C++
     @@ -43,15 +43,6 @@
       --- a/Makefile
       +++ b/Makefile
      @@
     - 
     - ifdef SANE_TOOL_PATH
     - SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
     --BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
     -+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
     - PATH := $(SANE_TOOL_PATH):${PATH}
     - else
     - BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
     -@@
       	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
       	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
       	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 16:  e7888bedbd = 16:  993b6bcd0a msvc: add a compile-time flag to allow detailed heap debugging
 17:  d9668558dd = 17:  42f5b8cee8 msvc: add pragmas for common warnings
 18:  c883f037e0 = 18:  c9a6f6892c msvc: do not pretend to support all signals
 19:  51a20ff2df = 19:  7cb0434fac msvc: avoid debug assertion windows in Debug Mode
 20:  4d44d1fab1 = 20:  4a7ceb2172 msvc: ignore .dll and incremental compile output
Signature · 1 line
gitgitgadget
#62 lore raw

PATCH v3 1/20 mingw: fix a typo in the msysGit-specific section

From: Johannes Schindelin <[email protected]>

The msysGit project (i.e. Git for Windows 1.x' SDK) is safely dead for *years* already. This is probably the reason why nobody caught this typo until Carlo Arenas spotted a copy-edited version of it nearby.

It is probably about time to rip out the remainders of msysGit/MSys1 support, but that can safely wait a bit longer, and we can at least fix the typo for now.

---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
config.mak.uname +1 -1
index b71688eeb7..9fc053cac0 100644
@@ -571,7 +571,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
 	INTERNAL_QSORT = YesPlease
 	HAVE_LIBCHARSET_H = YesPlease
 	NO_GETTEXT = YesPlease
-	COMPAT_CLFAGS += -D__USE_MINGW_ACCESS
+	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS
 else
 	ifneq ($(shell expr "$(uname_R)" : '1\.'),2)
 		# MSys2
Signature · 1 line
gitgitgadget
#63 lore raw

PATCH v3 2/20 Mark .bat files as requiring CR/LF endings

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Just like the natural line ending for Unix shell scripts consist of a single Line Feed, the natural line ending for (DOS) Batch scripts consists of a Carriage Return followed by a Line Feed.

It seems that both Unix shell script interpreters and the interpreter for Batch scripts (cmd.exe) are keen on seeing the "right" line endings.

---
 .gitattributes | 1 +
 1 file changed, 1 insertion(+)
.gitattributes +1 -0
index 9fa72ad450..b08a1416d8 100644
@@ -5,6 +5,7 @@
 *.pl eof=lf diff=perl
 *.pm eol=lf diff=perl
 *.py eol=lf diff=python
+*.bat eol=crlf
 /Documentation/**/*.txt eol=lf
 /command-list.txt eol=lf
 /GIT-VERSION-GEN eol=lf
Signature · 1 line
gitgitgadget
#64 lore raw

PATCH v3 3/20 t0001 (mingw): do not expect a specific order of stdout/stderr

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When redirecting stdout/stderr to the same file, we cannot guarantee that stdout will come first.

In fact, in this test case, it seems that an MSVC build always prints stderr first.

In any case, this test case does not want to verify the *order* but the *presence* of both outputs, so let's test exactly that.

---
 t/t0001-init.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
t/t0001-init.sh +2 -2
index 77a224aafb..387e4e6b81 100755
@@ -473,8 +473,8 @@ test_expect_success MINGW 'redirect std handles' '
 		GIT_REDIRECT_STDOUT=output.txt \
 		GIT_REDIRECT_STDERR="2>&1" \
 		git rev-parse --git-dir --verify refs/invalid &&
-	printf ".git\nfatal: Needed a single revision\n" >expect &&
-	test_cmp expect output.txt
+	grep "^\\.git\$" output.txt &&
+	grep "Needed a single revision" output.txt
 '
 
 test_done
Signature · 1 line
gitgitgadget
#65 lore raw

PATCH v3 4/20 cache-tree/blame: avoid reusing the DEBUG constant

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

In MS Visual C, the DEBUG constant is set automatically whenever compiling with debug information.

This is clearly not what was intended in cache-tree.c nor in builtin/blame.c, so let's use a less ambiguous name there.

---
 builtin/blame.c |  6 +++---
 cache-tree.c    | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)
builtin/blame.c +3 -3
index 21cde57e71..50e3d4a265 100644
@@ -59,8 +59,8 @@ static size_t blame_date_width;
 
 static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 
-#ifndef DEBUG
-#define DEBUG 0
+#ifndef DEBUG_BLAME
+#define DEBUG_BLAME 0
 #endif
 
 static unsigned blame_move_score;
@@ -1062,7 +1062,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	if (blame_copy_score)
 		sb.copy_score = blame_copy_score;
 
-	sb.debug = DEBUG;
+	sb.debug = DEBUG_BLAME;
 	sb.on_sanity_fail = &sanity_check_on_fail;
 
 	sb.show_root = show_root;
cache-tree.c +7 -7
index b13bfaf71e..706ffcf188 100644
@@ -6,8 +6,8 @@
 #include "object-store.h"
 #include "replace-object.h"
 
-#ifndef DEBUG
-#define DEBUG 0
+#ifndef DEBUG_CACHE_TREE
+#define DEBUG_CACHE_TREE 0
 #endif
 
 struct cache_tree *cache_tree(void)
@@ -111,7 +111,7 @@ static int do_invalidate_path(struct cache_tree *it, const char *path)
 	int namelen;
 	struct cache_tree_sub *down;
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree invalidate <%s>\n", path);
 #endif
 
@@ -398,7 +398,7 @@ static int update_one(struct cache_tree *it,
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 		fprintf(stderr, "cache-tree update-one %o %.*s\n",
 			mode, entlen, path + baselen);
 #endif
@@ -421,7 +421,7 @@ static int update_one(struct cache_tree *it,
 
 	strbuf_release(&buffer);
 	it->entry_count = to_invalidate ? -1 : i - *skip_count;
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
 		it->entry_count, it->subtree_nr,
 		oid_to_hex(&it->oid));
@@ -462,7 +462,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
 	strbuf_add(buffer, path, pathlen);
 	strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr);
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%.*s> (%d ent, %d subtree) %s\n",
 			pathlen, path, it->entry_count, it->subtree_nr,
@@ -536,7 +536,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
 		size -= rawsz;
 	}
 
-#if DEBUG
+#if DEBUG_CACHE_TREE
 	if (0 <= it->entry_count)
 		fprintf(stderr, "cache-tree <%s> (%d ent, %d subtree) %s\n",
 			*buffer, it->entry_count, subtree_nr,
Signature · 1 line
gitgitgadget
#68 lore raw

PATCH v3 7/20 msvc: fix dependencies of compat/msvc.c

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The file compat/msvc.c includes compat/mingw.c, which means that we have to recompile compat/msvc.o if compat/mingw.c changes.

---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)
config.mak.uname +2 -0
index 6ddece0350..473613a20d 100644
@@ -414,6 +414,8 @@ else
 	BASIC_CFLAGS += -Zi -MDd
 endif
 	X = .exe
+
+compat/msvc.o: compat/msvc.c compat/mingw.c GIT-CFLAGS
 endif
 ifeq ($(uname_S),Interix)
 	NO_INITGROUPS = YesPlease
Signature · 1 line
gitgitgadget
#72 lore raw

PATCH v3 11/20 msvc: do not re-declare the timespec struct

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015's headers already declare that struct.

---
 compat/mingw.h | 2 ++
 1 file changed, 2 insertions(+)
compat/mingw.h +2 -0
index 210f1b01a8..a03e40e6e2 100644
@@ -352,11 +352,13 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 #ifndef __MINGW64_VERSION_MAJOR
 #define off_t off64_t
 #define lseek _lseeki64
+#ifndef _MSC_VER
 struct timespec {
 	time_t tv_sec;
 	long tv_nsec;
 };
 #endif
+#endif
 
 struct mingw_stat {
     _dev_t st_dev;
Signature · 1 line
gitgitgadget
#71 lore raw

PATCH v3 10/20 msvc: mark a variable as non-const

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

VS2015 complains when using a const pointer in memcpy()/free().

---
 compat/mingw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
compat/mingw.c +4 -1
index 0d8713e515..d14d33308d 100644
@@ -1553,7 +1553,10 @@ static int try_shell_exec(const char *cmd, char *const *argv)
 	if (prog) {
 		int exec_id;
 		int argc = 0;
-		const char **argv2;
+#ifndef _MSC_VER
+		const
+#endif
+		char **argv2;
 		while (argv[argc]) argc++;
 		ALLOC_ARRAY(argv2, argc + 1);
 		argv2[0] = (char *)cmd;	/* full path to the script file */
Signature · 1 line
gitgitgadget
#74 lore raw

PATCH v3 13/20 msvc: fix detect_msys_tty()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

The ntstatus.h header is only available in MINGW.

---
 compat/winansi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
compat/winansi.c +13 -0
index f4f08237f9..11cd9b82cc 100644
@@ -544,7 +544,20 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
 #ifdef DETECT_MSYS_TTY
 
 #include <winternl.h>
+
+#if defined(_MSC_VER)
+
+typedef struct _OBJECT_NAME_INFORMATION
+{
+	UNICODE_STRING Name;
+	WCHAR NameBuffer[0];
+} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
+
+#define ObjectNameInformation 1
+
+#else
 #include <ntstatus.h>
+#endif
 
 static void detect_msys_tty(int fd)
 {
Signature · 1 line
gitgitgadget
#75 lore raw

PATCH v3 14/20 msvc: update Makefile to allow for spaces in the compiler path

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

It is quite common that MS Visual C++ is installed into a location whose path contains spaces, therefore we need to quote it.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index 8a7e235352..3cf8cc8ffd 100644
@@ -1240,7 +1240,7 @@ endif
 
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
-BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
+BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix "$(SANE_TOOL_PATH_SQ)"|'
 PATH := $(SANE_TOOL_PATH):${PATH}
 else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
Signature · 1 line
gitgitgadget
#77 lore raw

PATCH v3 16/20 msvc: add a compile-time flag to allow detailed heap debugging

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

MS Visual C comes with a few neat features we can use to analyze the heap consumption (i.e. leaks, max memory, etc).

With this patch, we introduce support via the build-time flag USE_MSVC_CRTDBG.

---
 compat/mingw.c    | 6 ++++++
 config.mak.uname  | 4 ++++
 git-compat-util.h | 9 +++++++++
 3 files changed, 19 insertions(+)
compat/mingw.c +6 -0
index c063ae62be..667285887a 100644
@@ -2411,6 +2411,12 @@ int wmain(int argc, const wchar_t **wargv)
 
 	trace2_initialize_clock();
 
+#ifdef _MSC_VER
+#ifdef USE_MSVC_CRTDBG
+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+#endif
+#endif
+
 	maybe_redirect_std_handles();
 
 	/* determine size of argv and environ conversion buffer */
config.mak.uname +4 -0
index b8c52e49d2..3fde48c64d 100644
@@ -448,6 +448,10 @@ else
 endif
 	BASIC_CFLAGS += $(sdk_libs) $(msvc_libs)
 
+ifneq ($(USE_MSVC_CRTDBG),)
+	# Optionally enable memory leak reporting.
+	BASIC_CFLAGS += -DUSE_MSVC_CRTDBG
+endif
 	BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1
 	# Always give "-Zi" to the compiler and "-debug" to linker (even in
 	# release mode) to force a PDB to be generated (like RelWithDebInfo).
git-compat-util.h +9 -0
index cc0e7e9733..83be89de0a 100644
@@ -1,6 +1,15 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#ifdef USE_MSVC_CRTDBG
+/*
+ * For these to work they must appear very early in each
+ * file -- before most of the standard header files.
+ */
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #define _FILE_OFFSET_BITS 64
 
 
Signature · 1 line
gitgitgadget
#79 lore raw

PATCH v3 18/20 msvc: do not pretend to support all signals

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

This special-cases various signals that are not supported on Windows, such as SIGPIPE. These cause the UCRT to throw asserts (at least in debug mode).

---
 compat/mingw.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
compat/mingw.c +25 -0
index 667285887a..d01e88c2f8 100644
@@ -2119,8 +2119,33 @@ int mingw_raise(int sig)
 			sigint_fn(SIGINT);
 		return 0;
 
+#if defined(_MSC_VER)
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGTERM:
+	case SIGBREAK:
+	case SIGABRT:
+	case SIGABRT_COMPAT:
+		/*
+		 * The <signal.h> header in the MS C Runtime defines 8 signals
+		 * as being supported on the platform. Anything else causes an
+		 * "Invalid signal or error" (which in DEBUG builds causes the
+		 * Abort/Retry/Ignore dialog). We by-pass the CRT for things we
+		 * already know will fail.
+		 */
+		return raise(sig);
+	default:
+		errno = EINVAL;
+		return -1;
+
+#else
+
 	default:
 		return raise(sig);
+
+#endif
+
 	}
 }
 
Signature · 1 line
gitgitgadget
#76 lore raw

PATCH v3 15/20 msvc: support building Git using MS Visual C++

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

With this patch, Git can be built using the Microsoft toolchain, via:

	make MSVC=1 [DEBUG=1]

Third party libraries are built from source using the open source "vcpkg" tool set. See https://github.com/Microsoft/vcpkg

On a first build, the vcpkg tools and the third party libraries are automatically downloaded and built. DLLs for the third party libraries are copied to the top-level (and t/helper) directory to facilitate debugging. See compat/vcbuild/README.

A series of .bat files are invoked by the Makefile to find the location of the installed version of Visual Studio and the associated compiler tools (essentially replicating the environment setup performed by a "Developer Command Prompt"). This should find the most recent VS2015 or VS2017 installation. Output from these scripts are used by the Makefile to define compiler and linker pathnames and -I and -L arguments.

The build produces .pdb files for both debug and release builds.

Note: This commit was squashed from an organic series of commits developed between 2016 and 2018 in Git for Windows' master branch. This combined commit eliminates the obsolete commits related to fetching NuGet packages for third party libraries. It is difficult to use NuGet packages for C/C++ sources because they may be built by earlier versions of the MSVC compiler and have CRT version and linking issues.

Additionally, the C/C++ NuGet packages that we were using tended to not be updated concurrently with the sources. And in the case of cURL and OpenSSL, this could expose us to security issues.

---
 Makefile                           |  40 +++++++
 compat/mingw.c                     |   6 ++
 compat/vcbuild/.gitignore          |   3 +
 compat/vcbuild/README              |  39 +++++++
 compat/vcbuild/find_vs_env.bat     | 168 +++++++++++++++++++++++++++++
 compat/vcbuild/scripts/clink.pl    |  41 ++++++-
 compat/vcbuild/vcpkg_copy_dlls.bat |  39 +++++++
 compat/vcbuild/vcpkg_install.bat   |  80 ++++++++++++++
 config.mak.uname                   |  72 +++++++++++--
 9 files changed, 473 insertions(+), 15 deletions(-)
 create mode 100644 compat/vcbuild/.gitignore
 create mode 100644 compat/vcbuild/find_vs_env.bat
 create mode 100644 compat/vcbuild/vcpkg_copy_dlls.bat
 create mode 100644 compat/vcbuild/vcpkg_install.bat
Makefile +40 -0
index 3cf8cc8ffd..7777af7d0e 100644
@@ -2873,6 +2873,33 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+ifdef MSVC
+	# We DO NOT install the individual foo.o.pdb files because they
+	# have already been rolled up into the exe's pdb file.
+	# We DO NOT have pdb files for the builtin commands (like git-status.exe)
+	# because it is just a copy/hardlink of git.exe, rather than a unique binary.
+	$(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ifndef DEBUG
+	$(INSTALL) $(vcpkg_rel_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_rel_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+else
+	$(INSTALL) $(vcpkg_dbg_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(vcpkg_dbg_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 	$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
@@ -3085,6 +3112,19 @@ endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
 	$(RM) GIT-USER-AGENT GIT-PREFIX
 	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
+ifdef MSVC
+	$(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS))
+	$(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(OTHER_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(PROGRAMS))
+	$(RM) $(patsubst %.exe,%.pdb,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.iobj,$(TEST_PROGRAMS))
+	$(RM) $(patsubst %.exe,%.ipdb,$(TEST_PROGRAMS))
+	$(RM) compat/vcbuild/MSVC-DEFS-GEN
+endif
 
 .PHONY: all install profile-clean cocciclean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
compat/mingw.c +6 -0
index d14d33308d..c063ae62be 100644
@@ -2388,6 +2388,12 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
+#ifdef _MSC_VER
+#ifdef _DEBUG
+#include <crtdbg.h>
+#endif
+#endif
+
 /*
  * We implement wmain() and compile with -municode, which would
  * normally ignore main(), but we call the latter from the former
compat/vcbuild/.gitignore new file +3 -0
new file mode 100644
index 0000000000..8f8b794ef3
@@ -0,0 +1,3 @@
+/vcpkg/
+/MSVC-DEFS-GEN
+/VCPKG-DEFS
compat/vcbuild/README +39 -0
index 60fd873fe8..b633e7db98 100644
@@ -1,3 +1,42 @@
+The Steps to Build Git with VS2015 or VS2017 from the command line.
+
+1. Install the "vcpkg" open source package manager and build essential
+   third-party libraries.  The steps for this have been captured in a
+   set of convenience scripts.  These can be run from a stock Command
+   Prompt or from an SDK bash window:
+
+   $ cd <repo_root>
+   $ ./compat/vcbuild/vcpkg_install.bat
+
+   The vcpkg tools and all of the third-party sources will be installed
+   in this folder:
+      <repo_root>/compat/vcbuild/vcpkg/
+
+   A file will be created with a set of Makefile macros pointing to a
+   unified "include", "lib", and "bin" directory (release and debug) for
+   all of the required packages.  This file will be included by the main
+   Makefile:
+      <repo_root>/compat/vcbuild/MSVC-DEFS-GEN
+
+2. OPTIONALLY copy the third-party *.dll and *.pdb files into the repo
+   root to make it easier to run and debug git.exe without having to
+   manipulate your PATH.  This is especially true for debug sessions in
+   Visual Studio.
+
+   Use ONE of the following forms which should match how you want to
+   compile git.exe.
+
+   $ ./compat/vcbuild/vcpkg_copy_packages.bat debug
+   $ ./compat/vcbuild/vcpkg_copy_packages.bat release
+
+3. Build git using MSVC from an SDK bash window using one of the
+   following commands:
+
+   $ make MSVC=1
+   $ make MSVC=1 DEBUG=1
+
+================================================================
+
 The Steps of Build Git with VS2008
 
 1. You need the build environment, which contains the Git dependencies
compat/vcbuild/find_vs_env.bat new file +168 -0

168 lines not on this page · raw message

compat/vcbuild/scripts/clink.pl +36 -5

78 lines not on this page · raw message

compat/vcbuild/vcpkg_copy_dlls.bat new file +39 -0
new file mode 100644
index 0000000000..13661c14f8
@@ -0,0 +1,39 @@
+@ECHO OFF
+REM ================================================================
+REM This script is an optional step. It copies the *.dll and *.pdb
+REM files (created by vcpkg_install.bat) into the top-level directory
+REM of the repo so that you can type "./git.exe" and find them without
+REM having to fixup your PATH.
+REM
+REM NOTE: Because the names of some DLL files change between DEBUG and
+REM NOTE: RELEASE builds when built using "vcpkg.exe", you will need
+REM NOTE: to copy up the corresponding version.
+REM ================================================================
+
+	SETLOCAL EnableDelayedExpansion
+
+	@FOR /F "delims=" %%D IN ("%~dp0") DO @SET cwd=%%~fD
+	cd %cwd%
+
+	SET arch=x64-windows
+	SET inst=%cwd%vcpkg\installed\%arch%
+
+	IF [%1]==[release] (
+		echo Copying RELEASE mode DLLs to repo root...
+	) ELSE IF [%1]==[debug] (
+		SET inst=%inst%\debug
+		echo Copying DEBUG mode DLLs to repo root...
+	) ELSE (
+		echo ERROR: Invalid argument.
+		echo Usage: %~0 release
+		echo Usage: %~0 debug
+		EXIT /B 1
+	)
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\
+
+	xcopy /e/s/v/y %inst%\bin\*.dll ..\..\t\helper\
+	xcopy /e/s/v/y %inst%\bin\*.pdb ..\..\t\helper\
+
+	EXIT /B 0
compat/vcbuild/vcpkg_install.bat new file +80 -0

80 lines not on this page · raw message

config.mak.uname +62 -10

121 lines not on this page · raw message

Signature · 1 line
gitgitgadget
#78 lore raw

PATCH v3 17/20 msvc: add pragmas for common warnings

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

MSVC can be overzealous about some warnings. Disable them.

---
 compat/msvc.h | 4 ++++
 1 file changed, 4 insertions(+)
compat/msvc.h +4 -0
index d7525cf61d..1d7a8c6145 100644
@@ -6,6 +6,10 @@
 #include <malloc.h>
 #include <io.h>
 
+#pragma warning(disable: 4018) /* signed/unsigned comparison */
+#pragma warning(disable: 4244) /* type conversion, possible loss of data */
+#pragma warning(disable: 4090) /* 'function' : different 'const' qualifiers (ALLOC_GROW etc.)*/
+
 /* porting function */
 #define inline __inline
 #define __inline__ __inline
Signature · 1 line
gitgitgadget
#80 lore raw

PATCH v3 19/20 msvc: avoid debug assertion windows in Debug Mode

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

For regular debugging, it is pretty helpful when a debug assertion in a running application triggers a window that offers to start the debugger.

However, when running the test suite, it is not so helpful, in particular when the debug assertions are then suppressed anyway because we disable the invalid parameter checking (via invalidcontinue.obj, see the comment in config.mak.uname about that object for more information).

So let's simply disable that window in Debug Mode (it is already disabled in Release Mode).

---
 compat/mingw.c | 4 ++++
 1 file changed, 4 insertions(+)
compat/mingw.c +4 -0
index d01e88c2f8..433838391f 100644
@@ -2437,6 +2437,10 @@ int wmain(int argc, const wchar_t **wargv)
 	trace2_initialize_clock();
 
 #ifdef _MSC_VER
+#ifdef _DEBUG
+	_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG);
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
 #endif
Signature · 1 line
gitgitgadget
#81 lore raw

PATCH v3 20/20 msvc: ignore .dll and incremental compile output

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

Ignore .dll files copied into the top-level directory. Ignore MSVC incremental compiler output files.

---
 .gitignore | 5 +++++
 1 file changed, 5 insertions(+)
.gitignore +5 -0
index 2374f77a1a..ba0e52c4d1 100644
@@ -226,6 +226,11 @@
 *.user
 *.idb
 *.pdb
+*.ilk
+*.iobj
+*.ipdb
+*.dll
+.vs/
 /Debug/
 /Release/
 *.dSYM
Signature · 1 line
gitgitgadget
#73 lore raw

PATCH v3 12/20 msvc: define ftello()

In reply to Johannes Schindelin via GitGitGadget

From: Jeff Hostetler <[email protected]>

It is just called differently in MSVC's headers.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index d336d80670..d7525cf61d 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+#define ftello _ftelli64
+
 typedef int sigset_t;
 /* open for reading, writing, or both (not in fcntl.h) */
 #define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
Signature · 1 line
gitgitgadget
#70 lore raw

PATCH v3 9/20 msvc: define O_ACCMODE

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

This constant is not defined in MSVC's headers.

In UCRT's fcntl.h, _O_RDONLY, _O_WRONLY and _O_RDWR are defined as 0, 1 and 2, respectively. Yes, that means that UCRT breaks with the tradition that O_RDWR == O_RDONLY | O_WRONLY.

It is a perfectly legal way to define those constants, though, therefore we need to take care of defining O_ACCMODE accordingly.

This is particularly important in order to keep our "open() can set errno to EISDIR" emulation working: it tests that (flags & O_ACCMODE) is not identical to O_RDONLY before going on to test specifically whether the file for which open() reported EACCES is, in fact, a directory.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 04b4750b87..d336d80670 100644
@@ -19,6 +19,8 @@
 #undef ERROR
 
 typedef int sigset_t;
+/* open for reading, writing, or both (not in fcntl.h) */
+#define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
 
 #include "compat/mingw.h"
 
Signature · 1 line
gitgitgadget
#66 lore raw

PATCH v3 5/20 obstack: fix compiler warning

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

MS Visual C suggests that the construct

condition ? (int) i : (ptrdiff_t) d

is incorrect. Let's fix this by casting to ptrdiff_t also for the positive arm of the conditional.

---
 compat/obstack.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
compat/obstack.h +1 -1
index ced94d0118..ae36ed6a66 100644
@@ -496,7 +496,7 @@ __extension__								\
 ( (h)->temp.tempint = (char *) (obj) - (char *) (h)->chunk,		\
   ((((h)->temp.tempint > 0						\
     && (h)->temp.tempint < (h)->chunk_limit - (char *) (h)->chunk))	\
-   ? (int) ((h)->next_free = (h)->object_base				\
+   ? (ptrdiff_t) ((h)->next_free = (h)->object_base				\
 	    = (h)->temp.tempint + (char *) (h)->chunk)			\
    : (((obstack_free) ((h), (h)->temp.tempint + (char *) (h)->chunk), 0), 0)))
 
Signature · 1 line
gitgitgadget
#69 lore raw

PATCH v3 8/20 msvc: include sigset_t definition

In reply to Johannes Schindelin via GitGitGadget

From: Philip Oakley <[email protected]>

On MSVC (VS2008) sigset_t is not defined.

---
 compat/msvc.h | 2 ++
 1 file changed, 2 insertions(+)
compat/msvc.h +2 -0
index 29a8ce8204..04b4750b87 100644
@@ -18,6 +18,8 @@
 
 #undef ERROR
 
+typedef int sigset_t;
+
 #include "compat/mingw.h"
 
 #endif
Signature · 1 line
gitgitgadget
#67 lore raw

PATCH v3 6/20 mingw: replace mingw_startup() hack

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Git for Windows has special code to retrieve the command-line parameters (and even the environment) in UTF-16 encoding, so that they can be converted to UTF-8. This is necessary because Git for Windows wants to use UTF-8 encoded strings throughout its code, and the main() function does not get the parameters in that encoding.

To do that, we used the __wgetmainargs() function, which is not even a Win32 API function, but provided by the MINGW "runtime" instead.

Obviously, this method would not work with any compiler other than GCC, and in preparation for compiling with Visual C++, we would like to avoid precisely that.

Lucky us, there is a much more elegant way: we can simply implement the UTF-16 variant of main(): wmain().

To make that work, we need to link with -municode. The command-line parameters are passed to wmain() encoded in UTF-16, as desired, and this method also works with GCC, and also with Visual C++ after adjusting the MSVC linker flags to force it to use wmain().

---
 compat/mingw.c   | 53 +++++++++++++++++++++++++++++++-----------------
 compat/mingw.h   | 22 ++++++++++----------
 config.mak.uname |  3 ++-
 3 files changed, 47 insertions(+), 31 deletions(-)
compat/mingw.c +34 -19
index 9b6d2400e1..0d8713e515 100644
@@ -2301,18 +2301,13 @@ static void setup_windows_environment(void)
 		setenv("TERM", "cygwin", 1);
 }
 
+#if !defined(_MSC_VER)
 /*
  * Disable MSVCRT command line wildcard expansion (__getmainargs called from
  * mingw startup code, see init.c in mingw runtime).
  */
 int _CRT_glob = 0;
-
-typedef struct {
-	int newmode;
-} _startupinfo;
-
-extern int __wgetmainargs(int *argc, wchar_t ***argv, wchar_t ***env, int glob,
-		_startupinfo *si);
+#endif
 
 static NORETURN void die_startup(void)
 {
@@ -2390,22 +2385,25 @@ static void maybe_redirect_std_handles(void)
 				  GENERIC_WRITE, FILE_FLAG_NO_BUFFERING);
 }
 
-void mingw_startup(void)
+/*
+ * We implement wmain() and compile with -municode, which would
+ * normally ignore main(), but we call the latter from the former
+ * so that we can handle non-ASCII command-line parameters
+ * appropriately.
+ *
+ * To be more compatible with the core git code, we convert
+ * argv into UTF8 and pass them directly to main().
+ */
+int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, argc;
-	char *buffer;
-	wchar_t **wenv, **wargv;
-	_startupinfo si;
+	int i, maxlen, exit_status;
+	char *buffer, **save;
+	const char **argv;
 
 	trace2_initialize_clock();
 
 	maybe_redirect_std_handles();
 
-	/* get wide char arguments and environment */
-	si.newmode = 0;
-	if (__wgetmainargs(&argc, &wargv, &wenv, _CRT_glob, &si) < 0)
-		die_startup();
-
 	/* determine size of argv and environ conversion buffer */
 	maxlen = wcslen(wargv[0]);
 	for (i = 1; i < argc; i++)
@@ -2415,9 +2413,16 @@ void mingw_startup(void)
 	maxlen = 3 * maxlen + 1;
 	buffer = malloc_startup(maxlen);
 
-	/* convert command line arguments and environment to UTF-8 */
+	/*
+	 * Create a UTF-8 version of w_argv. Also create a "save" copy
+	 * to remember all the string pointers because parse_options()
+	 * will remove claimed items from the argv that we pass down.
+	 */
+	ALLOC_ARRAY(argv, argc + 1);
+	ALLOC_ARRAY(save, argc + 1);
 	for (i = 0; i < argc; i++)
-		__argv[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+		argv[i] = save[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen);
+	argv[i] = save[i] = NULL;
 	free(buffer);
 
 	/* fix Windows specific environment settings */
@@ -2436,6 +2441,16 @@ void mingw_startup(void)
 
 	/* initialize Unicode console */
 	winansi_init();
+
+	/* invoke the real main() using our utf8 version of argv. */
+	exit_status = main(argc, argv);
+
+	for (i = 0; i < argc; i++)
+		free(save[i]);
+	free(save);
+	free(argv);
+
+	return exit_status;
 }
 
 int uname(struct utsname *buf)
compat/mingw.h +11 -11
index 593bdbffe6..210f1b01a8 100644
@@ -562,18 +562,18 @@ int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen);
 extern CRITICAL_SECTION pinfo_cs;
 
 /*
- * A replacement of main() that adds win32 specific initialization.
+ * Git, like most portable C applications, implements a main() function. On
+ * Windows, this main() function would receive parameters encoded in the
+ * current locale, but Git for Windows would prefer UTF-8 encoded  parameters.
+ *
+ * To make that happen, we still declare main() here, and then declare and
+ * implement wmain() (which is the Unicode variant of main()) and compile with
+ * -municode. This wmain() function reencodes the parameters from UTF-16 to
+ * UTF-8 format, sets up a couple of other things as required on Windows, and
+ * then hands off to the main() function.
  */
-
-void mingw_startup(void);
-#define main(c,v) dummy_decl_mingw_main(void); \
-static int mingw_main(c,v); \
-int main(int argc, const char **argv) \
-{ \
-	mingw_startup(); \
-	return mingw_main(__argc, (void *)__argv); \
-} \
-static int mingw_main(c,v)
+int wmain(int argc, const wchar_t **w_argv);
+int main(int argc, const char **argv);
 
 /*
  * Used by Pthread API implementation for Windows
config.mak.uname +2 -1
index 9fc053cac0..6ddece0350 100644
@@ -401,7 +401,7 @@ ifeq ($(uname_S),Windows)
 		compat/win32/trace2_win32_process_info.o \
 		compat/win32/dirent.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
-	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE
+	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -ENTRY:wmainCRTStartup -SUBSYSTEM:CONSOLE
 	EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj
 	PTHREAD_LIBS =
 	lib =
@@ -548,6 +548,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	ETAGS_TARGET = ETAGS
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	DEFAULT_HELP_FORMAT = html
+	BASIC_LDFLAGS += -municode
 	COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
Signature · 1 line
gitgitgadget
#82 lore raw
In reply to Johannes Schindelin via GitGitGadget

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

Quoted 3 lines · writes:

Philip Oakley and Jeff Hostetler worked quite a bit on getting Git to compile with MS Visual C again, and this patch series is the culmination of those efforts.

Thanks, all. Relative to the previous round, it seems there is only one patch changed, which is a sign that it is stabilizing rather quickly ;-).

Quoted 10 lines

Please also note that this patch series is part 1 of 3 in a bigger story:
the next patch series will add support to build Git in Microsoft Visual Studio, and the third patch series will add Continuous Testing by adding an MSVC build and a corresponding parallelized test job to our Azure Pipeline.

Changes since v2:

* Fixed the incorrect split-out of the "msvc: update Makefile to allow for spaces in the compiler path" patch: I had accidentally reverted that change in a later patch in the series.

Yup. BROKEN_PATH_FIX stuff appears only once now in the series ;-)

Will queue. Again, thanks all.