The generated MSVC-DEFS-GEN is incomplete

[PhilipOakley]

Newer versions of Visual Studio have moved some of the core include and library files required to compile with MSVC=1.

When doing such a compile the MSVC-DEFS-GEN is auto generated somewhere in vcpkg (see

git/compat/vcbuild/README

Line 19 in e3538d8

<repo_root>/compat/vcbuild/MSVC-DEFS-GEN

)

Where is that 'somewhere' that generates the file? @jeffhostetler - do you remember?, to avoid me dropping down the rabbit hole, see also #2179

[jeffhostetler]

On my phone, so I’m doing this from memory... caveat emptor... In compat/vcbuild are a series of BAT files that setup a temporary developer CMD environment and find and dump the various MSVC path and lib values and generate a GnuMake include file. This is referenced in config.mak.uname near the top. There’s some subtle ifdef’d makefile trickery there to run the generator script if necessary and then #include the generated file. Hope this helps. Jeff

…

________________________ j (from iPhone)

On May 10, 2019, at 7:27 PM, Philip Oakley ***@***.***> wrote: Newer versions of Visual Studio have moved some of the core include and library files required to compile with MSVC=1. When doing such a compile the MSVC-DEFS-GEN is auto generated somewhere in vcpkg (see https://github.com/git-for-windows/git/blob/e3538d8eb3036fad3d396f07b0a514f77e242755/compat/vcbuild/README#L19) Where is that 'somewhere' that generates the file? @jeffhostetler - do you remember?, to avoid me dropping down the rabbit hole, see also #2179 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[PhilipOakley]

On 11/05/2019 00:44, Jeff Hostetler wrote: On my phone, so I’m doing this from memory... caveat emptor... In compat/vcbuild are a series of BAT files that setup a temporary developer CMD environment and find and dump the various MSVC path and lib values and generate a GnuMake include file. This is referenced in config.mak.uname near the top. There’s some subtle ifdef’d makefile trickery there to run the generator script if necessary and then #include the generated file. Hope this helps. Jeff

Thanks, that helps a lot. In my searching I'd missed some of those linkages. I think I need to look at [2] where the wrong (insufficient) paths are discovered - I'll need to find out what will discover the right paths (see [3] from #2179, and [4] regarding 'breaking changes') ... Thanks again, Philip [1] https://github.com/git-for-windows/git/blob/e3538d8eb3036fad3d396f07b0a514f77e242755/config.mak.uname#L21-L23 [2] https://github.com/git-for-windows/git/blob/e3538d8eb3036fad3d396f07b0a514f77e242755/compat/vcbuild/find_vs_env.bat#L103-L105 [3] https://groups.google.com/forum/#!topic/git-for-windows/Y99a0dzlVJY [4] https://devblogs.microsoft.com/cppblog/introducing-the-universal-crt/

…

> On May 10, 2019, at 7:27 PM, Philip Oakley ***@***.***> wrote: > > Newer versions of Visual Studio have moved some of the core include and library files required to compile with MSVC=1. > > When doing such a compile the MSVC-DEFS-GEN is auto generated somewhere in vcpkg (see > https://github.com/git-for-windows/git/blob/e3538d8eb3036fad3d396f07b0a514f77e242755/compat/vcbuild/README#L19) > > Where is that 'somewhere' that generates the file? @jeffhostetler - do you remember?, to avoid me dropping down the rabbit hole, see also #2179 >

[PhilipOakley]

Finally got some moments to properly look through this / my notes.

It looks like I was stuck inside a little loop between the Makefile (config.mak.uname #L21) target of
compat/vcbuild/MSVC-DEFS-GEN: compat/vcbuild/find_vs_env.bat
and the fact that I already had one (that file target), but had updated the VS install relative to some time in the past when it was generated.

So. When I renamed (so as not to lose) the file (added _0 to the name), a new was generated, which I then compared with the old version (with my amendments), and yes the new version does look like it should work.

In summary, 'we' need a note to say that when one updates Visual Studio, one should delete the
MSVC-DEFS-GEN file. If only one had known...

Looks like make MSVC=1 has worked without error. So +1 to the team that that created all the support code.
(I just need to re-clone all my pack 'cos of a crc mistake earlier - doh)

[dscho]

In summary, 'we' need a note to say that when one updates Visual Studio, one should delete the
MSVC-DEFS-GEN file.

I wonder whether we can somehow detect that situation and Do The Right Thing instead of asking the user to delete the file manually.

[jeffhostetler]

I have code in the Makefile so that if you do "make MSVC=1 clean" it will delete the MSVC-DEFS-GEN file, but that still requires explicit action by the user.

[jeffhostetler]

We could:
[] change config.mak.uname to always run find_vs_env.bat (instead of only when MSVC-DEFS-GEN does not exist).
[] change find_vs_env.bat to only write the file when it doesn't exist or when the values have changed.
[] change Makefile and the GIT-BUILD-OPTIONS mechanism to be aware of the file. For example, append the contents (or hash) of MSVC-DEFS-GEN to the candidate GIT-BUILD-OPTIONS+ ($@+) file, so that it is reflected in the "@if cmp $@+ $@" line at the bottom.

My only concern is the speed of the find_vs_env.bat script. It takes a few seconds to lookup that data.
But I think something like that would work.

I'm still digging from vacation, so I don't have time to dig into this now, but that's what I would try first.

[PhilipOakley]

I'm on vacation too ;-) I have a small update to the README in my repo, but should also include the bit about the clean option.

I also had the thought that the clean also removed all the existing vcpkg bits, which maybe overzealous as it took 'minutes' to get all of them for me, so getting the selectivity right will be something to look at once we are all back ...

[PhilipOakley]

I'm on vacation too ;-)

Oops that sounded wrong. I mean, I'm now on a holiday, so I'm also indisposed...

[dscho]

I have a small update to the README in my repo, but should also include the bit about the clean option.

That sounds good.

I also had the thought that the clean also removed all the existing vcpkg bits, which maybe overzealous as it took 'minutes' to get all of them for me

Indeed, I think this is a bit much of the cleaning. The vcpkg stuff is basically providing third-party libraries that Git expects to somehow be available already, so with the idea that make clean cleans Git's source code, I think it should not remove vcpkg's stuff.

Besides, if you care to look at https://dev.azure.com/git/git/_build?definitionId=9 (which builds vcpkg every two weeks, offering the result as an artifact to be consumed by Git's Azure Pipeline when it builds with MSVC), you will see that the successful runs take over 10 minutes. That's a hefty price to pay for a simple make clean, I would say.

So better not touch compat\vcbuild\vcpkg in make clean.

[dscho]

Closing this stale ticket.