MAKEFLAGS / MFLAGS env vars are set for nmake.exe which breaks it

[Arnavion]

Visual Studio 2017 Community Edition
rustc 1.19.0-nightly (3d5b8c626 2017-06-09)
cargo 0.20.0-nightly (bfe80cfa8 2017-06-07)

Running cargo install --force cargo-tree fails with:

error: failed to compile `cargo-tree v0.12.0`, intermediate artifacts can be found at `C:\Users\Arnavion\AppData\Local\Temp\cargo-install.47VUzEoolGS3`

Caused by:
  failed to run custom build command for `libz-sys v1.0.13`
process didn't exit successfully: `C:\Users\Arnavion\AppData\Local\Temp\cargo-install.47VUzEoolGS3\release\build\libz-sys-a0376854548c6a52\build-script-build` (exit code: 101)
--- stdout
running: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Community\\VC\\Tools\\MSVC\\14.10.25017\\bin\\HostX64\\x64\\nmake.exe" "/nologo" "/f" "C:\\Users\\Arnavion\\AppData\\Local\\Temp\\cargo-install.47VUzEoolGS3\\release\\build\\libz-sys-0598af38eaebd697\\out\\build/win32/Makefile.msc" "zlib.lib"

--- stderr

Microsoft (R) Program Maintenance Utility Version 14.10.25019.0
Copyright (C) Microsoft Corporation.  All rights reserved.

NMAKE : fatal error U1065: invalid option '-'
Stop.

The reason is that cargo sets the following env vars when invoking libz-sys's build-script-build.exe

MAKEFLAGS="--jobserver-fds=__rust_jobserver_semaphore_3996332434 --jobserver-auth=__rust_jobserver_semaphore_3996332434"
MFLAGS="--jobserver-fds=__rust_jobserver_semaphore_3996332434 --jobserver-auth=__rust_jobserver_semaphore_3996332434"

which nmake doesn't understand.

I can repro manually by setting up the environment as cargo sets it and running build-script-build.exe, and if I remove those two env vars then build-script-build.exe runs successfully.

[pravic]

The same for VS 2015.

Stable cargo builds libz successfully, but recent nightly fails, so all dependent crates are broken now :)

It seems, it was happened at 2017-06-09 nightly, because 2017-06-08 one is okay.

Working version:
cargo 0.20.0-nightly (38ca9b702 2017-05-14)

Broken:
cargo 0.20.0-nightly (bfe80cfa8 2017-06-07)

[Arnavion]

https://github.com/rust-lang/cargo/blob/263548d9/src/cargo/ops/cargo_rustc/context.rs#L111 creates the jobserver, so there's always a jobserver defined. Then this jobserver is used to configure all subcommands, by defining those two env vars on them.

I'm not sure it's a good idea to set MAKEFLAGS from cargo since whether make is used or not depends on the build script. Perhaps it would be better to set a CARGO_JOBSERVER env var that build scripts (eg gcc-rs) can then convert to MAKEFLAGS?

[pravic]

cc rust-lang/libz-sys#18

[alexcrichton]

Thanks for the report! This was caused by #4110 and this is also a beta regression for rust-lang/rust so I've filed rust-lang/rust#42635 to track that. It's also worth pointing out that this is not the first time we've run into this, we've had problems when cargo is invoked from make as well historically!

FWIW I've fixed a few projects recently related to this:

• libz-sys
• curl-sys
• lzma-sys

These can all be classified into a two failure modes so far:

• nmake.exe interpreting MAKEFLAGS and choking because it's not intended for nmake.exe
• make.exe as the MSYS make which is interpreting MAKEFLAGS incorrectly. The MinGW make uses Windows semaphores (what MAKEFLAGS looks like) whereas MSYS somehow uses file descriptors. The MSYS version fails.

Surprisingly, though, crates which use cmake to build native code aren't actually failing. The crate currently generates MSYS makefiles which I believe uses make.exe instead of mingw32-make.exe. I wonder if cmake unconditionally removes this env var? Unsure.

In any case I see a few paths forward:

• Leave as-is. In any world I would want all of the above commits to land in projects. It's just inconvenient if you can't call cargo from make! The upside of this is that make should "naturally" work in more cases. The downside is that all current and future projects will need to be fixed.
• Move to a separate environment variable. The upside of this is that there's no breakage but the downside is that now all build scripts would need to go out of their way to leverage this jobserver information.

I'm personally still leaning towards the former stance. All code already should handle this case (avoiding MAKEFLAGS when using nmake.exe and the MSYS make.exe), so this is just an extra nudge to doing so. The fix is also quite simple, so I'm hoping that we can track down any breakage as it's happening.

Currently there is no make crate to invoke make, but it may be around this time that one starts to exist!

[pravic]

Thanks for explanation.

Alex, is it possible to eliminate jobserver feature at all, on "windows-msvc" toolchain? Apparently nobody won't build any crate using mingw or msys under MSVC toolchain, it is impossible.

Or I missed something?

[alexcrichton]

@pravic true yeah that's a possibility. I'd like to integrate the compiler itself into the same jobserver, but this isn't necessary for build scripts. I don't know of any tools that by default on MSVC would use the jobserver, so it's perhaps plausible to exclude MSVC entirely.

That being said it's also slightly inconsistent, and it means that non-nmake tools which want to interact with our jobserver (e.g. if gcc-rs wanted to use jobserver to interact) wouldn't be able to.

[alexcrichton]

@rust-lang/cargo any thoughts about this regression? Do others feel we should revert the change?

[matklad]

Do others feel we should revert the change?

Looks like fixing build scripts is a proper solution, so I would leave it as is. The release was recently, so if fixing build scripts turns out to be harder then it seems, we could revert later (by the way, this looks like a good example of a case where having a version ( in Cargo.toml or in the index) would likely help us to phase in the change more smoothly, so that all new versions of a crate get better behavior, and old versions get old behavior, without any explicit opt-in or opt-out from crate authors).

[Arnavion]

If build scripts need to be changed to accommodate MAKEFLAGS, why can't they be changed to accommodate CARGO_JOBSERVER ? The number of build scripts that are affected is the same in both cases. The former is opt-out and will break again in the future if cargo sets another env var that the script didn't expect, whereas the latter is opt-in and can be enabled by the build script only when it wants to use it ("I know that cargo invokes me with a jobserver, so I will pass down CARGO_JOBSERVER as MAKEFLAGS to my make subprocess, and not do anything for my nmake subprocess.").

[Arnavion]

Sorry, re-reading @alexcrichton 's comment I now see the scenario of make used to invoke cargo. So indeed a build script that uses nmake has to always sanitize the environment itself, and then it also automatically handles this jobserver scenario.

[alexcrichton]

@Arnavion ah yeah my thinking is that if you want your crate buildable from make itself then you already need to handle these env vars. Now I think there's a definitely a case to be made for "who really does that?" because I suspect the number is relatively small.

That being said, I'd also imagine that the number of build scripts which need to take this into account is relatively small as well...

[brson]

I've seen this error all over the place and had to do cargo upgrades of various things to get around it. Seems like this is going to affect pretty much every Rust project on windows.

[retep998]

In my opinion cargo should never set environment variables for build scripts that could change behavior without the build script's explicit knowledge. Thus environment variables should always be namespaced, so the build script can opt in to passing that information on to stuff it spawns such as make or nmake. After all, breakage only occurs when cargo does pass MAKEFLAGS, and there is absolutely nothing stopping build scripts which want those flags from forwarding a namespaced environment variable as MAKEFLAGS to their make invocations.