Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MSVC support, at long last #149

Closed
wants to merge 20 commits into from
Closed

Fix MSVC support, at long last #149

wants to merge 20 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 25, 2019

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 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.

Cc: Carlo Arenas carenas@gmail.com, Eric Sunshine sunshine@sunshineco.com, Johannes Sixt j6t@kdbg.org

@PhilipOakley
Copy link

in git-for-windows#2098 I'm not succeeding with getting a clean build from a plain make MSVC=1, and even then I'm not sure of how to make the leap from the output of that make to running the code on Visual Studio 2017 (equiv. to the good old project.sln mechanism 😃 )

But it does look like progress!

@jeffhostetler
Copy link

When using "make MSVC=1" or "make MSVC=1 DEBUG=1" you get a regular GCC-like result with *.exe and friends, just built using MSVC's CL..exe rather than GCC. This also uses "vcpkg" to download and compile from source all of the third-party libraries that we use, since we can't (or don't want to) use the mingw-shipped .libs/.dlls.

The net result is an old-fashioned set of .exe's and third-party .dll's.

You can then "open project" in VS and give it the name of the "git.exe". (Who knew, right?)
This is not as nice as opening a real .vcproj/.sln, but we're boot strapping here. Anyway, open
the .exe and then use the menu "Debug > git.exe Properties" to set the command line options
and etc. and then you can run git.exe as usual.

Also, you can use "open file" to open any .c source file and set breakpoints. So set a breakpoint
in main() or cmd_main() or whatever and then you should be back in familiar territory (just without
the Solution Explorer window to help you).

(If you get a runtime error about not finding a third-party dll, run
compat/vcbuild/vcpkg_copy_dlls.bat and it will put copies of the dlls next to the exes.)

@jeffhostetler
Copy link

There is a separate step that can synthesize a .sln/.vcproj (by running the makefile in a dry-run mode and then grinding on the output with perl). You can generate it from the makefile if you want. See the "vcxproj" target in config.mak.uname. IIRC, you can just do "make vcxproj" and watch the fun.
This will create a new commit, so be careful.

Or just checkout the vs/master branch, which is one commit ahead of master and has these
files.

But be advised, the .sln/.vcproj files are a synthetic and derivative work, since the makefile
itself is the source of truth WRT the code, so treat it as read-only. That is, if you need to add
a new source file, reset head to get rid of the generated commit, add the file to the makefile,
commit it, and then regenerate the .sln/.vcproj.

@dscho dscho removed the ready to submit Has commits that have not been submitted yet label Apr 10, 2019
@dscho

This comment has been minimized.

@dscho dscho added the needs-work These patches have pending issues that need to be resolved before submitting label May 20, 2019
@dscho dscho added ready to submit Has commits that have not been submitted yet and removed needs-work These patches have pending issues that need to be resolved before submitting labels Jun 5, 2019
@dscho
Copy link
Member Author

dscho commented Jun 18, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 18, 2019

Submitted as pull.149.git.gitgitgadget@gmail.com

cache-tree.c Show resolved Hide resolved
cache-tree.c Show resolved Hide resolved
Makefile Show resolved Hide resolved
compat/mingw.c Show resolved Hide resolved
Makefile Show resolved Hide resolved
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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@gitgitgadget
Copy link

gitgitgadget bot commented Jun 21, 2019

This patch series was integrated into pu via git@5bee56e.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 24, 2019

This patch series was integrated into pu via git@e3c9020.

@dscho
Copy link
Member Author

dscho commented Jun 25, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 25, 2019

Submitted as pull.149.v3.git.gitgitgadget@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 25, 2019

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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 ;-).

> 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.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 25, 2019

This patch series was integrated into pu via git@451e35c.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 26, 2019

This patch series was integrated into pu via git@ea7c658.

@dscho dscho removed the ready to submit Has commits that have not been submitted yet label Jun 27, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Jun 27, 2019

This patch series was integrated into pu via git@4339e25.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 28, 2019

This patch series was integrated into pu via git@aba8f7f.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 28, 2019

This patch series was integrated into pu via git@19a5761.

@gitgitgadget
Copy link

gitgitgadget bot commented Jun 28, 2019

This patch series was integrated into next via git@5a16e3d.

@gitgitgadget gitgitgadget bot added the next label Jun 28, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Jun 28, 2019

This patch series was integrated into pu via git@3f640d0.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 1, 2019

This patch series was integrated into pu via git@d3278f7.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 2, 2019

This patch series was integrated into pu via git@0ebe136.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 3, 2019

This patch series was integrated into pu via git@1fff726.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 3, 2019

This patch series was integrated into pu via git@8806953.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 9, 2019

This patch series was integrated into pu via git@3008a7c.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 10, 2019

This patch series was integrated into pu via git@88b1075.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 10, 2019

This patch series was integrated into next via git@88b1075.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 10, 2019

This patch series was integrated into master via git@88b1075.

@gitgitgadget gitgitgadget bot added the master label Jul 10, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Jul 10, 2019

Closed via 88b1075.

@gitgitgadget gitgitgadget bot closed this Jul 10, 2019
@dscho dscho deleted the msvc branch July 11, 2019 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants