-
Notifications
You must be signed in to change notification settings - Fork 8
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
Comments
If you use what's in HEAD it should support VS2019: Also use |
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:
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 |
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.
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. |
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 |
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. |
@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. |
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 |
I had hoped that CMake would have a sensible default generator on Windows. That sounds like a much better plan. |
- Depend on default cmake behavior (see #23)
Tentatively made in |
- Depend on default cmake behavior (see #23)
- Depend on default cmake behavior (see #23)
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:
My current workaround is to
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
The text was updated successfully, but these errors were encountered: