-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Make Git for Windows start builds in modern Visual Studio #3306
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
Changes from all commits
03b742e
204e0d5
2c669d0
0e1dc0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -243,3 +243,4 @@ Release/ | |
/git.VC.db | ||
*.dSYM | ||
/contrib/buildsystems/out | ||
CMakeSettings.json | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,11 @@ Note: Visual Studio also has the option of opening `CMakeLists.txt` | |
directly; Using this option, Visual Studio will not find the source code, | ||
though, therefore the `File>Open>Folder...` option is preferred. | ||
|
||
Visual Studio does not produce a .sln solution file nor the .vcxproj files | ||
that may be required by VS extension tools. | ||
|
||
To generate the .sln/.vcxproj files run CMake manually, as described below. | ||
|
||
Instructions to run CMake manually: | ||
|
||
mkdir -p contrib/buildsystems/out | ||
|
@@ -22,7 +27,7 @@ Instructions to run CMake manually: | |
|
||
This will build the git binaries in contrib/buildsystems/out | ||
directory (our top-level .gitignore file knows to ignore contents of | ||
this directory). | ||
this directory). The project .sln and .vcxproj files are also generated. | ||
|
||
Possible build configurations(-DCMAKE_BUILD_TYPE) with corresponding | ||
compiler flags | ||
|
@@ -35,17 +40,16 @@ empty(default) : | |
NOTE: -DCMAKE_BUILD_TYPE is optional. For multi-config generators like Visual Studio | ||
this option is ignored | ||
|
||
This process generates a Makefile(Linux/*BSD/MacOS) , Visual Studio solution(Windows) by default. | ||
This process generates a Makefile(Linux/*BSD/MacOS), Visual Studio solution(Windows) by default. | ||
Run `make` to build Git on Linux/*BSD/MacOS. | ||
Open git.sln on Windows and build Git. | ||
|
||
NOTE: By default CMake uses Makefile as the build tool on Linux and Visual Studio in Windows, | ||
to use another tool say `ninja` add this to the command line when configuring. | ||
`-G Ninja` | ||
|
||
NOTE: By default CMake will install vcpkg locally to your source tree on configuration, | ||
to avoid this, add `-DNO_VCPKG=TRUE` to the command line when configuring. | ||
|
||
The Visual Studio default generator changed in v16.6 from its Visual Studio | ||
implemenation to `Ninja` This required changes to many CMake scripts. | ||
|
||
]] | ||
cmake_minimum_required(VERSION 3.14) | ||
|
||
|
@@ -59,14 +63,28 @@ endif() | |
|
||
if(NOT DEFINED CMAKE_EXPORT_COMPILE_COMMANDS) | ||
set(CMAKE_EXPORT_COMPILE_COMMANDS TRUE) | ||
message("settting CMAKE_EXPORT_COMPILE_COMMANDS: ${CMAKE_EXPORT_COMPILE_COMMANDS}") | ||
endif() | ||
|
||
if(USE_VCPKG) | ||
set(VCPKG_DIR "${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg") | ||
message("WIN32: ${WIN32}") # show its underlying text values | ||
message("VCPKG_DIR: ${VCPKG_DIR}") | ||
message("VCPKG_ARCH: ${VCPKG_ARCH}") # maybe unset | ||
message("MSVC: ${MSVC}") | ||
message("CMAKE_GENERATOR: ${CMAKE_GENERATOR}") | ||
message("CMAKE_CXX_COMPILER_ID: ${CMAKE_CXX_COMPILER_ID}") | ||
message("CMAKE_GENERATOR_PLATFORM: ${CMAKE_GENERATOR_PLATFORM}") | ||
message("CMAKE_EXPORT_COMPILE_COMMANDS: ${CMAKE_EXPORT_COMPILE_COMMANDS}") | ||
message("ENV(CMAKE_EXPORT_COMPILE_COMMANDS): $ENV{CMAKE_EXPORT_COMPILE_COMMANDS}") | ||
PhilipOakley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if(NOT EXISTS ${VCPKG_DIR}) | ||
message("Initializing vcpkg and building the Git's dependencies (this will take a while...)") | ||
execute_process(COMMAND ${CMAKE_SOURCE_DIR}/compat/vcbuild/vcpkg_install.bat ${VCPKG_ARCH}) | ||
endif() | ||
if(NOT EXISTS ${VCPKG_ARCH}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems a bit weird to do this after we print out all the other build configuration information and run the vcpkg_install script using this option anyways, is there a reason it is printed out here rather than with the others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The other printouts are as a reflection of the implicit Build Options. Dscho has asked that I include the The reason I put the message here is because the default arch is set within vcpkg_install script and I was keeping that dependency logic, so if a user ran vcpkg_install first, manually, without parameters, then we'd have the same end result. Dscho made a similar comment wrt detecting the Visual Studio default, and the same logic should apply that we should only have one source of truth. Hmm. Just rechecking. The on-line gui suggests you only highlighted one line, but maybe not. I'll need to check that the rebase onto upstream hasn't created some out of sequence interaction - I see an will re-check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Looks like I goofed up the rebase / conflict. I'd also thought that the MSVC dependency had gone, because Visual Studio is now using Ninja, but maybe my memory is getting confused. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, so I had a patch make it into seen that does do some work in regards to that dependency here. Where we generate the compile_commands.json file by default. So many tools can operate off of CMake builds with little work. More relevant here we add the USE_VCPKG option, here so maybe it's better to base this off of that work as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Definitely. I had it in my mind that that work was already in GfW, and so would have been picked up when I rebased my series onto GfW (my upstream)... I did a quick tweak of script to assume MSVC was true (by deleting it!), and VS has loaded vcpkg - Yay. I'm preping to be away for a four-day weekend. Hopefully have something done on Wedn next. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I lost track. Does this still need to be resolved, or has it been addressed already? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this concern has been addressed as part of a reply to a different comment. |
||
message("VCPKG_ARCH: unset, using 'x64-windows'") | ||
set(VCPKG_ARCH "x64-windows") # default from vcpkg_install.bat | ||
PhilipOakley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
endif() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you want to print out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good idea. Will add. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still a good idea? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this concern has been addressed as part of a reply to a different comment. |
||
list(APPEND CMAKE_PREFIX_PATH "${VCPKG_DIR}/installed/${VCPKG_ARCH}") | ||
|
||
# In the vcpkg edition, we need this to be able to link to libcurl | ||
|
Uh oh!
There was an error while loading. Please reload this page.