jeikabu / nng-rust

MIT License
18 stars 8 forks source link

Does not build on Windows when Visual Studio 2019 is installed instead of VS 2017 #23

Closed jacobsacco closed 5 years ago

jacobsacco commented 5 years ago

nng-rust seems to look for specifically VS 2017 when building, and fails on the newer version of VS. Note that nng itself works when I build and install it to my system, so I know that cmake and nng are working.

Terminal output when I try to build an example nng program with the build-nng feature left enabled:

D:\Seafile\My Library\rust\nng_pls_work>"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat"
**********************************************************************
** Visual Studio 2019 Developer Command Prompt v16.3.2
** Copyright (c) 2019 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

D:\Seafile\My Library\rust\nng_pls_work>cargo build
   Compiling nng-sys v1.1.1-rc.1
error: failed to run custom build command for `nng-sys v1.1.1-rc.1`

Caused by:
  process didn't exit successfully: `D:\Seafile\My Library\rust\nng_pls_work\target\debug\build\nng-sys-9a35c4a1c91ac9f1\build-script-build` (exit code: 101)
--- stdout
running: "cmake" "C:\\Users\\sacc0004\\.cargo\\registry\\src\\github.com-1ecc6299db9ec823\\nng-sys-1.1.1-rc.1\\nng" "-Thost=x64" "-Ax64" "-G" "Visual Studio 15 2017 Win64" "-DNNG_TESTS=OFF" "-DNNG_TOOLS=OFF" "-DNNG_ENABLE_STATS=OFF" "-DNNG_ENABLE_TLS=OFF" "-DCMAKE_INSTALL_PREFIX=D:\\Seafile\\My Library\\rust\\nng_pls_work\\target\\debug\\build\\nng-sys-63bd593ea8685f68\\out" "-DCMAKE_C_FLAGS= -nologo -MD -Brepro" "-DCMAKE_CXX_FLAGS= -nologo -MD -Brepro" "-DCMAKE_BUILD_TYPE=Debug"
-- Configuring incomplete, errors occurred!
See also "D:/Seafile/My Library/rust/nng_pls_work/target/debug/build/nng-sys-63bd593ea8685f68/out/build/CMakeFiles/CMakeOutput.log".

--- stderr
CMake Error at CMakeLists.txt:30 (project):
  Generator

    Visual Studio 15 2017 Win64

  could not find any instance of Visual Studio.

thread 'main' panicked at '
command did not execute successfully, got: exit code: 1

build script failed, must exit now', C:\Users\sacc0004\.cargo\registry\src\github.com-1ecc6299db9ec823\cmake-0.1.42\src\lib.rs:861:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

D:\Seafile\My Library\rust\nng_pls_work>

My current workaround is to

  1. disable build-nng in Cargo.toml
  2. build and install nng for windows in the typical manner
  3. add the path to nng.lib to the rustc-link-search in build.rs

With the workaround everything works as expected, but it would be nice to not have to have my users install nng separately from everything else

jeikabu commented 5 years ago

If you use what's in HEAD it should support VS2019: https://github.com/jeikabu/nng-rust/pull/22

Also use cmake-vs2019 feature.

jacobsacco commented 5 years ago

Ahh, thank you for pointing that out, and also for the very fast response! It looks like applications that depend only on the nng-sys crate actually build with no special work, but applications that depend on the nng crate do not. To make things weirder, I found that I can bypass the problems with the nng crate by adding a direct dependency to nng-sys in my Cargo.toml, but the cmake-vs2019 feature can't be found (for some reason). My Cargo.toml has this entry in it:

[dependencies]
nng-sys = { version = "1.1.1-rc.1", features = ["cmake-ninja"] }

It seems that whatever version of nng-sys 1.1.1-rc.1 that cargo grabs doesn't actually have the cmake-vs2019 feature (which I verified by clearing my .cargo cache, re-downloading, and then checking the Cargo.toml). It does, however, have the cmake-ninja feature, which causes both nng-sys and nng to build properly in a stub project with no extra work.

Basically, adding the cmake-ninja feature is a suitable solution to my problem, but there is definitely some weirdness going on that I do not understand.

Thanks

neachdainn commented 5 years ago

It looks like applications that depend only on the nng-sys crate actually build with no special work, but applications that depend on the nng crate do not.

The nng-rs crate can only depend on the latest published version of nng-sys, which is not at the current HEAD. NNG v1.2 has seemed close to being released so I have been largely waiting for that to happen before pushing a new version of the nng-sys, but if this is an issue that is impacting people I can make that update tonight.

To make things weirder, I found that I can bypass the problems with the nng crate by adding a direct dependency to nng-sys in my Cargo.toml

I do not like the idea of "infectious" features and I do not want nng-rs to have to expose every build option that nng-sys does. Because nng-sys uses defaults that usually just work, the official way to do non-default builds is to directly depend on nng-sys and specify them there.

jacobsacco commented 5 years ago

That seems reasonable to me. So long as this problem and solution are publicly posted, I'm fine. Initially I assumed that the problem was with my installs, it took a lot of investigation to arrive at this solution, and it was not obvious.

@neachdainn @jeikabu thank you both for your help

neachdainn commented 5 years ago

HEAD has been pushed to Crates.io and nng-rs will use v1.1.1-rc.2 unless otherwise specified. The README of nng-rs has the information on non-default builds but I don't intend to update the Crates.io version until the NNG v1.2 release.

neachdainn commented 5 years ago

@jeikabu Perhaps something like vswhere (crate, binary) would be useful here? We should still allow the user to specify a version but finding an installed VS version would be better than assuming it is 2017.


EDIT: Just a heads-up, the CI is failing for master due to a single whitespace issue caused by an update in Rustfmt. The build is not broken otherwise.

EDIT: I have pushed a fix for this.

jeikabu commented 5 years ago

If we try to make it too smart, it will still be different from cmake. Which would introduce further confusion.

We should probably only call .generator() if one of the feature flags is actually set (with the default being none of them). With no generator specified cmake will use it's own default behavior. In this case picking the highest version of VS. This would be the "least surprising"- except to people that are relying on the current "always VS2017" default.

neachdainn commented 5 years ago

I had hoped that CMake would have a sensible default generator on Windows. That sounds like a much better plan.

jeikabu commented 5 years ago

Tentatively made in cmake_default_gen branch should anyone want them now. Because this may cause issues for anyone that now has VS2019 installed, we'll merge it with other "breaking" changes that will come with nng 1.2 (feature/nng_1.2 branch).