On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > 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 <jeffhost@xxxxxxxxxxxxx> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> > --- > 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. > 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? > 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. > 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.
