Skip to content
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

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

Closed
jacobsacco opened this issue Oct 7, 2019 · 9 comments

Comments

@jacobsacco
Copy link

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\\jackfan.us.kg-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\jackfan.us.kg-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
Copy link
Owner

jeikabu commented Oct 7, 2019

If you use what's in HEAD it should support VS2019:
#22

Also use cmake-vs2019 feature.

@jacobsacco
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

neachdainn commented Oct 7, 2019

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
Copy link
Collaborator

neachdainn commented Oct 7, 2019

@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
Copy link
Owner

jeikabu commented Oct 8, 2019

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
Copy link
Collaborator

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

jeikabu added a commit that referenced this issue Oct 8, 2019
- Depend on default cmake behavior (see #23)
@jeikabu
Copy link
Owner

jeikabu commented Oct 11, 2019

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).

@jeikabu jeikabu closed this as completed Oct 11, 2019
jeikabu added a commit that referenced this issue Oct 11, 2019
- Depend on default cmake behavior (see #23)
jeikabu added a commit that referenced this issue Oct 11, 2019
- Depend on default cmake behavior (see #23)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants