Update Visual Studio build instructions #2355

bachase · 2018-01-25T17:56:16Z

This changeset removes the old VS2015 project files and adds Visual Studio 2017 instructions based on CMake.

The first two commits are from #2348, which will start requiring us too use boost 1.66.0.

In b29eddc, I added a default CMakeSettings.json file to the top-level directory, which means if you installed boost and openssl in the recommended C:\lib directories, the integrated cmake will work properly when opening the top-level folder in VS2017. However, the current convention is to include the boost version in the directory name, i.e. C:\lib\boost_1_66_0, so this file would need to change if you upgrade boost.

bachase · 2018-01-25T17:57:56Z

I tagged our other main Windows developers, but appreciate anyone else having time to test these steps out. @manojsdoshi in particular might have some good feedback.

mellery451 · 2018-01-25T19:14:36Z

Builds/VisualStudio2017/README.md

+Any version of Visual Studio 2017 may be used to build rippled. The **Visual
+Studio 2017 Community** edition is available free of charge (see [the product
+page](https://www.visualstudio.com/products/visual-studio-community-vs) for
+licensing details), while paid editions may be used for an free initial trial


consider "an initial free-trial period" instead.

mellery451 · 2018-01-25T19:17:52Z

Builds/VisualStudio2017/README.md

+licensing details), while paid editions may be used for an free initial trial
+period.
+
+As of this writing, the latest version of Visual Studio 2017 for Windows is


I don't object to this being here, but also don't think there is any benefit to mentioning current latest versions of software (one of them will surely be eclipsed within a short time). What could be useful is a table of known-to-work versions for each dependency. Perhaps this is what you were getting at here - if that's the case, I'd consider just making a table of each prerequisite and the version(s) that are known to work.

seelabs · 2018-01-25T19:18:29Z

CMakeSettings.json

+      "variables": [
+        {
+          "name": "BOOST_ROOT",
+          "value": "C:\\lib\\boost_1_66_0"


I don't love that we're hard coding the boost version here. Is there a better way? Maybe just lib/boost?

I agree and that was part of my concern I wrote about in the first comment. Perhaps we just recommend C:\\lib\boost as you say, and users can do some type of symlink if they want to manage multiple versions.

ripplelabs-jenkins · 2018-01-25T19:21:10Z

Jenkins Build Summary

Built from this commit

Built at 20180129 - 23:11:18

Test Results

Build Type	Result	Status
clang.debug.unity	985 cases, 0 failed, t: 442s	PASS ✅
coverage	985 cases, 0 failed, t: 574s	PASS ✅
clang.debug.nounity	983 cases, 0 failed, t: 350s	PASS ✅
gcc.debug.unity	985 cases, 0 failed, t: 396s	PASS ✅
gcc.debug.nounity	983 cases, 0 failed, t: 273s	PASS ✅
clang.release.unity	984 cases, 0 failed, t: 476s	PASS ✅
gcc.release.unity	984 cases, 0 failed, t: 506s	PASS ✅

mellery451 · 2018-01-25T19:21:16Z

Builds/VisualStudio2017/README.md

+
+1. 64-bit. Use this if you are running 64-bit windows. As of this writing, the
+   link is called: "Win64 OpenSSL v1.0.2n".
+2. 64-bit light - Don't use this. It is missing files needed to build rippled.


consider adding emphasis to "Don't use this."

seelabs · 2018-01-25T19:24:24Z

Builds/VisualStudio2017/README.md

+### Install OpenSSL
+
+[Download OpenSSL.](http://slproweb.com/products/Win32OpenSSL.html) There will
+be four variants available:


You mention four variants then only list two. There will also be a link to download openssl 1.1, and people may be tempted to use it. However, rippled does not compile with openssl 1.1, only the 1.0 branch (at least last I checked, is this still true?)

That is still true AFAIK. I'll fix.

seelabs · 2018-01-25T19:27:36Z

Builds/VisualStudio2017/README.md

+the bash prompt:
+
+```powershell
+git clone git@github.com:ripple/rippled.git


We probably want to add the --recursive flag to get the submodules.

Which components have submodules? I think after #2343 , docca will no longer be used. Are there others?

Yeah, according to .gitmodules:

[submodule "src/nudb/extras/beast"] path = src/nudb/extras/beast url = https://github.com/vinniefalco/Beast.git [submodule "src/nudb/extras/rocksdb"] path = src/nudb/extras/rocksdb url = https://github.com/facebook/rocksdb.git [submodule "src/nudb/doc/docca"] path = src/nudb/doc/docca url = https://github.com/vinniefalco/docca.git

Are those actually needed for nudb though? Or just to build nudb examples and docs?

I'm not sure we need them, actually. If it works without the recursive them I'm fine leaving it off.

My test build did not do a --recursive clone, so I don't think they are. I'll leave it out unless someone else discovers otherwise.

miguelportilla · 2018-01-25T19:58:50Z

CMakeSettings.json

+      "name": "x64-Debug",
+      "generator": "Visual Studio 15 2017 Win64",
+      "configurationType": "Debug",
+      "inheritEnvironments": [ "msvc_x64_x64" ],


Is msvc_x64_x64 correct? I use msvc_x64.

I think they added it after we started using it https://docs.microsoft.com/en-us/cpp/ide/cmake-tools-for-visual-cpp.

msvc_x64_x64 | Compile for AMD64 using 64-bit tools

miguelportilla · 2018-01-25T20:03:17Z

CMakeSettings.json

+      ]
+    }
+  ]
+}


Nit. Add a carriage return for git happiness.

miguelportilla · 2018-01-25T20:06:04Z

Builds/VisualStudio2017/README.md

+* [OpenSSL Library](README.md#install-openssl)
+* [Boost library](README.md#build-boost)
+* (Optional) [Cmake for Windows](README.md#optional-install-cmake-for-windows) -
+  Only needed if not using the integrated CMake in VS 2017.


Not sure if it is worth mentioning that having Cmake for Windows will allow you to create VS project/solution files.

seelabs · 2018-01-29T16:25:03Z

Builds/CMake/CMakeFuncs.cmake

@@ -357,6 +357,12 @@ macro(use_boost)
      if(NOT Boost_FOUND)
        message(WARNING "Boost directory found, but not all components. May not be able to build.")
      endif()
+	  if(MSVC14)
+	    # VS2017 with boost <= 1.66.0 requires a flag to suppress warnings


Nit: weird indentation here

seelabs · 2018-01-29T16:25:27Z

Builds/CMake/CMakeFuncs.cmake

-      link_directories($ENV{OPENSSL_ROOT}/lib)
+	if ((NOT DEFINED OPENSSL_ROOT) AND (DEFINED ENV{OPENSSL_ROOT}))
+        set(OPENSSL_ROOT $ENV{OPENSSL_ROOT})
+    endif()


Nit: weird indentation here as well

seelabs · 2018-01-29T16:32:03Z

👍 After the indentation issues in CMakeFuncs.cmake are resolved

miguelportilla · 2018-01-29T17:14:09Z

👍

seelabs · 2018-01-29T21:15:55Z

@bachase I just noticed that this PR doesn't interact well with scons vcxproj. We still need to support scons in 0.90.0.

This reverts commit c85c433.

bachase · 2018-01-29T21:47:41Z

scons vcxproj will still generate VS2015 project files, which can be opened with VS2017 too. I restored the deleted solution files for now and just note that they will be removed when we switch to cmake and remove scons support.

ximinez

👍

seelabs · 2018-01-30T02:53:31Z

In 0.90.0-b5

seelabs and others added 6 commits January 25, 2018 12:20

Convert code to use boost::beast

cb06cc7

Remove pre-boost beast

33c12c9

[FOLD] Remove VS2015 files

c85c433

[FOLD] Suppress boost warnings with VS17

c1383d9

[FOLD] Add default CmakeSettings.json

b29eddc

[FOLD] Add VisualStudio2017 build instructions

42ebcdf

bachase requested review from miguelportilla and ximinez January 25, 2018 17:59

bachase assigned miguelportilla and ximinez Jan 25, 2018

mellery451 reviewed Jan 25, 2018

View reviewed changes

seelabs reviewed Jan 25, 2018

View reviewed changes

mellery451 reviewed Jan 25, 2018

View reviewed changes

seelabs reviewed Jan 25, 2018

View reviewed changes

[FOLD] Address PR comments

8b55956

miguelportilla reviewed Jan 25, 2018

View reviewed changes

CMakeSettings.json Outdated

]

}

]

}

Copy link

Contributor

miguelportilla Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Add a carriage return for git happiness.

bachase reacted with thumbs up emoji

miguelportilla reviewed Jan 25, 2018

View reviewed changes

[FOLD] Address additional PR comments

2a655b6

seelabs reviewed Jan 29, 2018

View reviewed changes

[FOLD] Fix whitespace

9fce4c0

seelabs closed this Jan 29, 2018

seelabs reopened this Jan 29, 2018

Revert "[FOLD] Remove VS2015 files"

4e0cd68

This reverts commit c85c433.

[FOLD] Address PR comments

2376138

ximinez approved these changes Jan 29, 2018

View reviewed changes

ximinez added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jan 29, 2018

seelabs closed this Jan 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Visual Studio build instructions #2355

Update Visual Studio build instructions #2355

bachase commented Jan 25, 2018

bachase commented Jan 25, 2018 •

edited

Loading

mellery451 Jan 25, 2018

mellery451 Jan 25, 2018 •

edited

Loading

seelabs Jan 25, 2018

bachase Jan 25, 2018

ripplelabs-jenkins commented Jan 25, 2018 •

edited

Loading

mellery451 Jan 25, 2018 •

edited

Loading

seelabs Jan 25, 2018

bachase Jan 25, 2018

seelabs Jan 25, 2018

bachase Jan 25, 2018

seelabs Jan 25, 2018 •

edited

Loading

bachase Jan 25, 2018 •

edited

Loading

seelabs Jan 25, 2018

bachase Jan 25, 2018

miguelportilla Jan 25, 2018

bachase Jan 25, 2018

miguelportilla Jan 25, 2018

miguelportilla Jan 25, 2018

seelabs Jan 29, 2018

seelabs Jan 29, 2018

seelabs commented Jan 29, 2018

miguelportilla commented Jan 29, 2018

seelabs commented Jan 29, 2018

bachase commented Jan 29, 2018

ximinez left a comment

seelabs commented Jan 30, 2018

Update Visual Studio build instructions #2355

Update Visual Studio build instructions #2355

Conversation

bachase commented Jan 25, 2018

bachase commented Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

mellery451 Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ripplelabs-jenkins commented Jan 25, 2018 • edited Loading

Jenkins Build Summary

Test Results

mellery451 Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seelabs Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

bachase Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seelabs commented Jan 29, 2018

miguelportilla commented Jan 29, 2018

seelabs commented Jan 29, 2018

bachase commented Jan 29, 2018

ximinez left a comment

Choose a reason for hiding this comment

seelabs commented Jan 30, 2018

bachase commented Jan 25, 2018 •

edited

Loading

mellery451 Jan 25, 2018 •

edited

Loading

ripplelabs-jenkins commented Jan 25, 2018 •

edited

Loading

mellery451 Jan 25, 2018 •

edited

Loading

seelabs Jan 25, 2018 •

edited

Loading

bachase Jan 25, 2018 •

edited

Loading