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

Cargo will always invoke the 'true' rustc on Windows, rather than a rustc shim #5960

Closed
aidanhs opened this issue Sep 2, 2018 · 2 comments
Closed
Labels
O-windows OS: Windows

Comments

@aidanhs
Copy link
Member

aidanhs commented Sep 2, 2018

(by a 'rustc shim' I'm referring to things like rustup - it puts a shim rustc on the path which invokes the true rustc)

As part of invoking rustc for compilations, cargo will put a number of paths in the 'library environment', performed here (called from Compilation::rustc_process, which is called from prepare_rustc): https://github.com/rust-lang/cargo/blob/d4ee795/src/cargo/core/compiler/compilation.rs#L188-L204

However, a few things conspire against us:

  • the paths are added to the beginning of the variable, i.e. with the highest priority
  • on Windows the environment variable these new paths get added to (util::dylib_path_envvar()) is the same as the binary search path (i.e. PATH)
  • on Windows, the sysroot dylibs for the compiler are in the same directory as the actual binaries (i.e. rustc.exe lives alongside dylibs like rustc_typeck-4d5d5a72bda8f430.dll)

This means that when actually performing the compilation, an attempted invocation of rustc will jump straight to the sysroot binaries rather than using the shim - anything that the shim does (like telemetry, in the case of the rustup) gets completely skipped for any compilations. You can observe this with a build plan on Windows:

{
    "invocations": [
        {
            "package_name": "cde",
            "package_version": "0.1.0",
            "target_kind": [
                "bin"
            ],
            "kind": "Host",
            "deps": [],
            "outputs": [
                "C:\\X\\cde\\target\\release\\deps\\cde-63a3117d8b95571a.exe",
                "C:\\X\\cde\\target\\release\\deps\\cde-63a3117d8b95571a.pdb"
            ],
            "links": {
                "C:\\X\\cde\\target\\release\\cde.exe": "C:\\X\\cde\\target\\release\\deps\\cde-63a3117d8b95571a.exe",
                "C:\\X\\cde\\target\\release\\cde.pdb": "C:\\X\\cde\\target\\release\\deps\\cde-63a3117d8b95571a.pdb"
            },
            "program": "rustc",
            "args": [
                "--crate-name",
                "cde",
                "src\\main.rs",
                "--crate-type",
                "bin",
                "--emit=dep-info,link",
                "-C",
                "opt-level=3",
                "-C",
                "metadata=63a3117d8b95571a",
                "-C",
                "extra-filename=-63a3117d8b95571a",
                "--out-dir",
                "C:\\X\\cde\\target\\release\\deps",
                "-L",
                "dependency=C:\\X\\cde\\target\\release\\deps"
            ],
            "env": {
                "CARGO": "\\\\?\\C:\\Users\\aidanhs\\.rustup\\toolchains\\nightly-2018-08-19-x86_64-pc-windows-msvc\\bin\\cargo.exe",
                "CARGO_MANIFEST_DIR": "C:\\X\\cde",
                [...]
                "CARGO_PRIMARY_PACKAGE": "1",
                "PATH": "C:\\X\\cde\\target\\release\\deps;C:\\Users\\aidanhs\\.rustup\\toolchains\\nightly-2018-08-19-x86_64-pc-windows-msvc\\bin;C:\\Users\\aidanhs\\.cargo\\bin;C:\\Users\\aidanhs\\.rustup\\toolchains\\nightly-2018-08-19-x86_64-pc-windows-msvc\\bin;[...]"
            },
            "cwd": "C:\\X\\cde"
        }
    ],
    "inputs": [
        "C:\\X\\cde\\Cargo.toml"
    ]
}

Note the C:\\Users\\aidanhs\\.rustup\\toolchains\\nightly-2018-08-19-x86_64-pc-windows-msvc\\bin with a higher priority than C:\\Users\\aidanhs\\.cargo\\bin in the PATH.

There is an additional oddity though - when performing configuration tests at cargo startup (

let mut process = rustc.process();
process
.arg("-")
.arg("--crate-name")
.arg("___")
.arg("--print=file-names")
.args(&rustflags)
.env_remove("RUST_LOG");
), cargo will use the correct binary, i.e. the shim. This makes sense (this is the way it actually discovers the sysroot to add to the environment), but is a bit odd (since you then get two entries in rustup telemetry).

Quick thoughts:

  • I'm not clear why we need to add the sysroot dylib path to the environment - on Linux the rustc binary has an rpath, and Windows will apparently attempt to use the directory of the executable first (https://msdn.microsoft.com/en-us/library/7d83bc18.aspx)
  • Adding paths at the end of the variable could cause oddities with unusual setups, but I would imagine the risks are fairly low given the hashes added to the end of rustc-generated dylibs.
  • Move dylibs into a different directory in the rust distribution - likely to make standalone rustc calls fail.
@alexcrichton
Copy link
Member

Thanks for the report! I suspect that at least on windows we could avoid adding the sysroot to PATH as being in the same cwd as rustc.exe should make it not necessary. I forget now at this point why we do it for other platforms, but I think it has something to do with build scripts, for example, linking to proc_macro which is a dynamic library.

In that sense we can probably avoid doing this for all rustc invocations, but I think we may need to continue to do so for other non-rustc invocations? (the handling of dynamic library lookup paths is a bit of a mess...)

@dwijnand dwijnand added the O-windows OS: Windows label Dec 25, 2018
@ehuss
Copy link
Contributor

ehuss commented May 23, 2023

I'm going to close, since as of #11917 we are intentionally circumventing the rustup proxies. Also, since this was filed, things like telemetry have been removed from rustup. There's also been some changes to the priority of how things are added to PATH. There's still some work left to do (like rust-lang/rustup#3036), but those are tracked elsewhere.

@ehuss ehuss closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows OS: Windows
Projects
None yet
Development

No branches or pull requests

4 participants