VS2017 vcpkg support. #1304

jeffhostetler · 2017-09-19T18:08:58Z

Use the new vcpkg package manager to build (fresh) third party libraries
rather than NuGet packages when building with MSVC. The vcpkg setup
can run with either a Command Prompt or a SDK bash window.

I have built git using VS2017 (using "make MSVC=1") and all seems well.
I have not tried it with VS2015.

@dscho I left a TODO note in the README about updating the PERL code to
generate the .sln/.vcxproj files. It should be relatively easy, but PERL is not my
thing.

jeffhostetler · 2017-09-19T18:13:09Z

I should have mentioned that the 3 or 4 vcpkg_*.bat setup scripts only need
to be run once in the repo. Then the Makefile will just use the existing installation.
I did not want to try to automatically download and build vcpkg, cmake, and all the
third-party libraries as a subordinate task implicitly referenced by the Makefile.

compat/vcbuild/vcpkg_defines.bat

+@ECHO OFF
+REM ================================================================
+REM This script builds a set of MAKEFILE definitions to allow a
+REM normal "make MSVC=1 vcpkg" to find the vcpkg-version of the


jeffhostetler · 2017-10-13T19:39:01Z

TODO It turns out that when you build with "make MSVC=1 DEBUG=1", that the zlib .lib/.dll are named zlibd rather than zlib. I need to update the bat file that installs them to remove the 'd' -- or to fix the ld step to know about the debug flag.

dscho · 2017-10-13T19:48:00Z

Awesome! I'll have a look next week.

dscho · 2017-11-29T22:45:45Z

I'll have a look next week.

Or six weeks later...

Sorry!

Anyway, I gave this a look-over, and also worked on using the vcpkg stuff from inside Visual Studio. I think I succeeded... Could you have a look @jeffhostetler?

One time too many did this developer call the `generate` script passing a `--make-out=<PATH>` option that was happily ignored (because there should be a space, not an equal sign, between `--make-out` and the path). And one time too many, this script not only ignored it but did not even complain. Let's fix that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The script assumes that we're in the top-level directory of the checkout. That does not need to be true. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

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

Dependencies such as cURL and OpenSSL are necessary to build and run Git. Previously, we obtained those dependencies by fetching NuGet packages. However, it is notoriously hard to keep NuGet packages of C/C++ libraries up-to-date, as the toolsets for different Visual Studio versions are different, and the NuGet packages would have to ship them all. That is the reason why the NuGet packages we use are quite old, and even insecure in the case of cURL and OpenSSL (the versions contain known security flaws that have been addressed by later versions for which no NuGet packages are available). The better way to handle this situation is to use the vcpkg system: https://github.com/Microsoft/vcpkg The idea is that a single Git repository contains enough supporting files to build up-to-date versions of a large number of Open Source libraries on demand, including cURL and OpenSSL. We integrate this system via four new .bat files to 1) initialize the vcpkg system, 2) build the packages, 4) set up Git's Makefile system to find the build artifacts, and 3) copy the artifacts into the top-level directory Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

We no longer use NuGet packages... Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

As we do not consume NuGet packages any longer, there is no sense to try to point PATH to their unpacked .dll files, either. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

It really does depend on libgit. It does not hurt to let it depend on xdiff, and it makes the code simpler. It is necessary to get this dependency chain right, because we will introduce a change where the vcpkg system is initialized before building libgit. The vcpkg system will then build the dependencies needed by Git (and thereby make the include headers available): As the vcpkg system cannot be run in parallel (it does not lock, wreaking havoc with files being accessed and written at the same time, letting the vcpkg processes stumble over each others' toes. We prevent that by ensuring that only one project is built at first: libgit. And this project's PreBuildEvent will be used to initialize vcpkg and build all dependencies. Subsequently, the other projects can be built in parallel. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The .vcxproj's text nodes do not actually need to URL-encode double quotes. So let's not do that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2017-12-07T17:26:37Z

Okay, I worked a bit more on this, and now it even copies the .dll and .pdb files as intended. I had minor problems running the test suite, but those might be unrelated problems, as I ran the test suite in a plain Git for Windows Bash (i.e. not the SDK).

Sadly, a quick git clean -dfx made all of my records go away, so I do not even remember which tests those were ;-)

Anyway, could you have another look, @jeffhostetler?

jeffhostetler · 2017-12-08T17:24:37Z

@dscho I didn't see what you did to the installer for the DLLs and PDBs, but I pushed a commit to copy the 3rd-party DLLs and PDBs during the install. I just tried a "make MSVC=1 HOME=/d/foo install" and was able to use the resulting installation in /d/foo/bin, so I think we're good.

Building from the SDK/bash worked fine and automatically fetched vcpkg, cmake, and the libraries, so I think were good there.

I did not try the .sln portion this morning.

dscho · 2017-12-16T15:50:56Z

So... I ran the test suite and it turns out that t6500 is failing.

Today, I found some time to investigate a bit further and it would appear as if Git uses fscanf() wrong: it parses a string of up to 256 characters via %250c. That is incorrect, because that says "precisely 256 characters", and not NUL terminated.

Funnily enough, further digging brings up afe2fab... which is a commit that made it into v2.14.3, but this here branch is based on an earlier commit. So I simply backported the fix.

IIRC there was one other test failure, I am re-running the entire test suite now.

dscho · 2017-12-18T14:43:53Z

I added another patch that was required here to let t7814 pass; it triggered a thread-related bug in make_environment_block(), and I think it is the correct way to fix it. @jeffhostetler, could you please have a look?

dscho · 2017-12-18T14:46:59Z

BTW there is one more thing I would love to do before merging this PR: the <Copy> task in the VS projects interferes between projects built in parallel, because they all want to copy .dll files either into the top-level directory or into t/helper/. I would love to find a way to make that conditional on the timestamps. (Later, though, I need to take a break now.)

PhilipOakley · 2017-12-18T15:48:06Z

An Aside: What is the minimum needed workflow when installing the VS2017 community edition (15.5.2).

I'm guessing that the first offered option of the Universal Windows Development Platform" will give every thing and the kitchen sink...

I'm more thinking that "Desktop Development with C++" is the more appropriate (better /minimal) option for the GfW usage (this is for a W7 64 bit machine)

Any thoughts?

dscho · 2017-12-18T16:52:57Z

I pushed yet another commit, this one fixing a bug in one of my old fixups. Sorry for the breakage!

dscho · 2017-12-18T22:23:45Z

What is the minimum needed workflow when installing the VS2017 community edition (15.5.2).

My intention is that you should be able to build the solution, and it does everything you need to allow running the tests in a Git Bash (from your regular Git for Windows installation, no Git for Windows SDK required).

I'm guessing that the first offered option of the Universal Windows Development Platform" will give every thing and the kitchen sink...

It should. It also should successfully install the "vcpkg" system into a subdirectory, and build the necessary dependencies there.

I'm more thinking that "Desktop Development with C++" is the more appropriate (better /minimal) option for the GfW usage (this is for a W7 64 bit machine)

Possibly. I don't know, quite honestly. If you could try it, and if you can then verify that it eases the initial install workload noticeably, I think that would make for a fine contribution to the wiki page.

compat/mingw.c

+#include "qsort.c"
+#endif
+
+/* Compare only keys */


compat/mingw.c

-				maxlen += strlen(deltaenv[k]) + 1;
-				nr_delta_ins++;
-			}
+	if (!deltaenv || !*deltaenv) {


compat/mingw.c

-	 * We will use the wblock_ins as the final result.
+	 * If there is a deltaenv, let's accumulate all keys into `array`,
+	 * sort them using the stable git_qsort() and then copy, skipping
+	 * duplicate keys


compat/mingw.c

+	/* convert the deltaenv, appending to array */
+	for (i = 0, p3 = wdeltaenv; deltaenv[i]; i++) {
+		size_t s = strlen(deltaenv[i]) + 1, wlen;
+		wlen = xutftowcs(p3, deltaenv[i], s * 2);


VS2017 vcpkg support.

jeffhostetler commented Sep 26, 2017

View reviewed changes

dscho force-pushed the vs2017_vcpkg branch from df1b2d4 to 5c3a6c1 Compare November 29, 2017 22:43

jeffhostetler force-pushed the vs2017_vcpkg branch from ef6d253 to 4b0abad Compare November 30, 2017 21:50

dscho and others added 8 commits December 5, 2017 12:47

contrib/buildsystems: redirect stderr into the correct directory

9494fd0

The script assumes that we're in the top-level directory of the checkout. That does not need to be true. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Mark .bat files as requiring CR/LF endings

e760a82

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

msvc: cleanup obsolete nuget files

65ca451

We no longer use NuGet packages... Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

msvc: get rid of the MSVC_DEPS variable

ce94fab

As we do not consume NuGet packages any longer, there is no sense to try to point PATH to their unpacked .dll files, either. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

vcxproj: do not use " unnecessarily

04f2fb6

The .vcxproj's text nodes do not actually need to URL-encode double quotes. So let's not do that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho force-pushed the vs2017_vcpkg branch from ae5bd6d to c63c5d2 Compare December 7, 2017 17:23

dscho force-pushed the vs2017_vcpkg branch from 5cbe27a to 18a9182 Compare December 18, 2017 16:50

dscho force-pushed the vs2017_vcpkg branch from 18a9182 to 51c94ce Compare December 18, 2017 22:18

jeffhostetler commented Dec 18, 2017

View reviewed changes

compat/mingw.c

maxlen += strlen(deltaenv[k]) + 1;

nr_delta_ins++;

}

if (!deltaenv || !*deltaenv) {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

jeffhostetler commented Dec 18, 2017

View reviewed changes

dscho added a commit that referenced this pull request May 29, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

f932d25

VS2017 vcpkg support.

PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

f719523

VS2017 vcpkg support.

PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

04b9acb

VS2017 vcpkg support.

PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

a9d74fd

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Aug 22, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

430a265

VS2017 vcpkg support.

dscho added a commit to dscho/git that referenced this pull request Aug 22, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

3f25f58

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Aug 23, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

401daf8

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Aug 23, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

bdb369d

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Aug 23, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

f2723fc

VS2017 vcpkg support.

jamill pushed a commit to jamill/git that referenced this pull request Aug 28, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

4fedc46

VS2017 vcpkg support.

jamill pushed a commit to jamill/git that referenced this pull request Sep 5, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

c979e57

VS2017 vcpkg support.

git-for-windows-ci pushed a commit that referenced this pull request Sep 10, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

be679f1

VS2017 vcpkg support.

jamill pushed a commit to jamill/git that referenced this pull request Sep 11, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

a755078

VS2017 vcpkg support.

git-for-windows-ci pushed a commit that referenced this pull request Sep 24, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

7994d5f

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Oct 10, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

3e2cf54

VS2017 vcpkg support.

dscho added a commit to dscho/git that referenced this pull request Oct 12, 2018

Merge pull request git-for-windows#1304 from jeffhostetler/vs2017_vcpkg

af70464

VS2017 vcpkg support.

dscho added a commit that referenced this pull request Oct 12, 2018

Merge pull request #1304 from jeffhostetler/vs2017_vcpkg

289d256

VS2017 vcpkg support.

weekly-digest bot mentioned this pull request Oct 14, 2018

Weekly Digest (7 October, 2018 - 14 October, 2018) #1882

Closed

jeffhostetler deleted the vs2017_vcpkg branch July 1, 2019 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VS2017 vcpkg support. #1304

VS2017 vcpkg support. #1304

jeffhostetler commented Sep 19, 2017

jeffhostetler commented Sep 19, 2017

This comment was marked as off-topic.

jeffhostetler commented Oct 13, 2017

dscho commented Oct 13, 2017

dscho commented Nov 29, 2017

dscho commented Dec 7, 2017

jeffhostetler commented Dec 8, 2017

dscho commented Dec 16, 2017

dscho commented Dec 18, 2017

dscho commented Dec 18, 2017

PhilipOakley commented Dec 18, 2017

dscho commented Dec 18, 2017

dscho commented Dec 18, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

VS2017 vcpkg support. #1304

VS2017 vcpkg support. #1304

Conversation

jeffhostetler commented Sep 19, 2017

jeffhostetler commented Sep 19, 2017

This comment was marked as off-topic.

jeffhostetler commented Oct 13, 2017

dscho commented Oct 13, 2017

dscho commented Nov 29, 2017

dscho commented Dec 7, 2017

jeffhostetler commented Dec 8, 2017

dscho commented Dec 16, 2017

dscho commented Dec 18, 2017

dscho commented Dec 18, 2017

PhilipOakley commented Dec 18, 2017

dscho commented Dec 18, 2017

dscho commented Dec 18, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.