Re: [PATCH 13/17] msvc: support building Git using MS Visual C++
From: Eric Sunshine <hidden>
Date: 2019-06-19 08:36:08
On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 [...] Signed-off-by: Jeff Hostetler <redacted> Signed-off-by: Johannes Schindelin <redacted> ---diff --git a/Makefile b/Makefile@@ -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.
quoted hunk ↗ jump to hunk
diff --git a/compat/mingw.c b/compat/mingw.c@@ -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?
quoted hunk ↗ jump to hunk
diff --git a/compat/vcbuild/README b/compat/vcbuild/README@@ -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.
+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.
+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.
quoted hunk ↗ jump to hunk
diff --git a/compat/vcbuild/find_vs_env.bat b/compat/vcbuild/find_vs_env.bat@@ -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.