Cargo doesn't support config relative paths for rustc|rustc_wrapper|rustc_workspace_wrapper|rustdoc

[rickvanprim]

Problem
When attempting to set a relative path for rustc|rustc_wrapper|rustc_workspace_wrapper|rustdoc in the Cargo Config [build] section, the build is dependent on the working directory Cargo chooses for any given build step. As a result, running cargo build --manifest-path=my_workspace_crate/Cargo.toml from the workspace root doesn't work because part of the process looks for rustc relative to the root of the workspace, and part looks relative to the root of my_workspace_crate.

Steps

1. Set a relative path for rustc inside .cargo/config
2. Attempt to build a crate where working directory is the root of that crate.
3. Run cargo clean.
4. Attempt to build the same crate from a different working directory by specifying --manifest-path or as part of a workspace.
5. Observe build fails due to not being able to find rustc.

Possible Solution(s)
Inside of src/cargo/util/config/mod.rs change CargoBuildConfig's rustc|rustc_wrapper|rustc_workspace_wrapper|rustdoc from Option<PathBuf> to Option<ConfigRelativePath>

Notes

Output of cargo version: cargo 1.44.0-nightly (8751eb3 2020-04-21)

[ehuss]

I don't think the meaning can be changed, as that would be a breaking change.

We have two changes we are planning that might help:

• Adding substitutions like {cwd} in config paths. We could probably include something like {cfg} for config-relative? Or {workspace} for workspace-relative? This is roughly tracked in Allow place holders in build.target-dir #6210 and Cargo makes target-dir path relative to user's home directory #7843 among other issues.
• Add a -C DIR CLI option which will chdir at the start. This can be used as an alternative to --manifest-path where it will give a little more control over config files and relative paths.

Unfortunately we don't really have much of a design written down for these, yet.

[rickvanprim]

I'm totally new to this code, but from the following inside of ConfigRelativePath, it seems like there shouldn't be a behavioral change for the default value of rustc. Does this help with the concern that this would be a breaking change?

/// Resolves this configuration-relative path to either an absolute path or
/// something appropriate to execute from `PATH`.
///
/// Values which don't look like a filesystem path (don't contain `/` or
/// `\`) will be returned as-is, and everything else will fall through to an
/// absolute path.
pub fn resolve_program(self, config: &Config) -> PathBuf {
    config.string_to_path(self.0.val, &self.0.definition)
}

[rickvanprim]

Making this change locally appears to work correctly for my test workspace with a relative path rustc, and passes the Cargo repo's cargo test (with the exception of the crosspiling tests that I haven't set up my machine to handle).

[solb]

Actually, this is (or is related to) a regression (or at least jarring behavior change) in stable Cargo. For my thesis work, I have a custom rustc wrapper script under .cargo/ and a .cargo/config file that includes:

[build]
rustc = ".cargo/rustc"

This actually works on cargo up through at least 0.40.0 (aka "1.39.0" according to --version), but broke sometime between then and 0.43.1 (aka "1.42.1"). Specifically, under the latter version, it now fails to find the script while building dependencies, as in this example:

$ cargo run
   Compiling libc v0.2.95
error: could not compile `libc`.

Caused by:
  could not execute process `.cargo/rustc --crate-name build_script_build /home/user/.cargo/registry/src/github.spider-man.dpdns.org-1ecc6299db9ec823/libc-0.2.95/build.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=da404d5fe542deb9 -C extra-filename=-da404d5fe542deb9 --out-dir /home/user/minimal/target/debug/build/libc-da404d5fe542deb9 -L dependency=/home/user/minimal/target/debug/deps --cap-lints allow` (never executed)

Caused by:
  No such file or directory (os error 2)

Running it under strace -feexecve,chdir -esignal=none, part of the culprit appears to be that Cargo now performs a chdir() into the dependency source tree that it didn't used to.

I did this entire comparison with rustc 1.48.0: note that changing only the version of Cargo causes my build to fail.

[solb]

Actually, I take back what I said about chdir(): it looks like 0.40.0 still changes to the dependency's directory, but goes on to execute the rustc script from the root project's .cargo/ directory:

[pid 24715] chdir("/home/user/.cargo/registry/src/github.spider-man.dpdns.org-1ecc6299db9ec823/libc-0.2.95") = 0
[pid 24715] execve("/home/user/minimal/.cargo/rustc", ["/home/user/minimal/.cargo/rustc", "--crate-name", "build_script_build", "/home/user/.cargo/registry/src/g"..., "--color", "always", "--crate-type", "bin", "--emit=dep-info,link", "-C", "debuginfo=2",
 "--cfg", "feature=\"default\"", "--cfg", "feature=\"std\"", "-C", "metadata=da404d5fe542deb9", "-C", "extra-filename=-da404d5fe542deb9", "--out-dir", "/home/user/minimal/target/debug/"..., "-L", "dependency=/home/user/minimal/ta"..., "--cap-lints", "allo
w"], 0x7fe4f8004dd0 /* 43 vars */) = 0

Here's a minimal example for reproducing this:
Cargo.toml

[package]
name = "minimal"
version = "0.0.0"

[dependencies]
libc = "*"

.cargo/config
(as shown in my above post)

.cargo/rustc
(symlink to the real rustc)

[solb]

Another interesting tidbit about @rickvanprim's PathBuf discovery...
Looking into the history of those CargoBuildConfig lines, I found that they predate (the working, for me) version 0.40.0:

$ git log -L1810,1812:mod.rs
...
commit e50985181
Author: Diogo Sousa <diogogsousa@gmail.com>
Date:   Sun Nov 10 15:02:15 2019 +0000

    Fix unused configuration key warning for a few keys under `build`.
    
    Recently cargo started to warn about configuration keys that he doesn't know
    about.  However, there are a few keys under `build` that were used in a
    dynamic way (`rustc`, `rustdoc`, and `rustc_wrapper`) by
    `Config::maybe_get_tool()`.
    
    Since these keys are not known to exist when `Config` is deserialized, cargo
    was emitting unused warnings.
    
    This commit makes those config keys explicit.  Note that by doing so there is a
    small breaking change: before it was possible to have `build.rustc_wrapper` in
    the configuration file (even though the documented key uses kebak-case), and
    now that key will be ignored.  (Good thing we have warnings for unrecognized
    keys!)

diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs
--- a/src/cargo/util/config/mod.rs
+++ b/src/cargo/util/config/mod.rs
@@ -1476,0 +1463,2 @@
+    pub rustc_wrapper: Option<PathBuf>,
+    pub rustc: Option<PathBuf>,
$ git describe e50985181
0.37.0-766-ge50985181

I'm curious to know whether @rickvanprim's issue reproduces on 0.40.0; if so, these lines are not the cause of the regression even though they are an apparent fix, and if not, my issue is actually a different one.

[rickvanprim]

@solb I'm not sure. I've stopped using Cargo for anything other than projects that happen to exactly fit Cargo's view of the world. I found some other working directory stuff here #8506 (comment).

I think you may be able to work around this behavior using the improved rust-toolchain.toml stuff. Either by giving that a path to your wrapper (as it resolves relative to the TOML file), or by defining your own toolchains that are defined in and local to your repo, and then letting the host resolution pick the appropriate version.
https://rust-lang.github.io/rustup/overrides.html#the-toolchain-file

[solb]

Interesting workaround. I don't currently use rustup, though, and don't really want to introduce it as a dependency.
@ehuss This seems like a Cargo regression that occurred several versions ago. Is there any way to work around it just using Cargo, without waiting for config path substitutions?

[ehuss]

Sorry, when this was opened I think I misunderstood the situation. I have posted #9566 to fix this.