Are you open to de-duplicating the build.rs build probe code?

[RalfJung]

Doing build probes in build.rs is hard to get right. It's easy to make the common case work, but it's just as easy to forget about properly handling --target (#249) or RUSTC_WRAPPER (also #249), new relevant flags get added every now and then (#358) and then when the crate becomes a dependency of rustc itself things may still break (#320). Meanwhile, both anyhow, thiserror and proc-macro2 have copies of basically the same build probe logic, so every time there is an issue it needs to be fixed in all of these crates. I'm kind of tired of having to go around and debug build probe issues, so it'd be really great if there was one shared crate for build probes where fixes have to be done only once, and which we can easily test in CI of tools that tend to run into issues like this (such as Miri).

autocfg could be such a crate, it has pretty well-tested build probe logic and is checked on Miri CI. If cuviper/autocfg#24 were resolved, it could be used by anyhow, thiserror and proc-macro2. autocfg does not have any dependencies and works on every stable Rust version since 1.0.

@dtolnay would you be open to taking autocfg as a dependency for build probe logic, should cuviper/autocfg#24 get resolved?

[dtolnay]

I am open to it, but I might recommend giving it a while longer for the best build probe logic to get shaken out. The latest iteration of this code is quite recent (which tracks with the perception of high recent churn). I can't see cuviper/autocfg#24 being adequate for what this crate needs (in regard to rerun-if-env-changed=RUSTC_BOOTSTRAP) and I don't think we are at the stage where I could propose a robust, futureproof API for inclusion in autocfg.

In the meantime, proc-macro2 is more widely used than autocfg. Maybe Miri's CI could be expanded to cover it?

I am subscribed to the autocfg repo, which is how I found out about #358 from cuviper/autocfg#56

[RalfJung]

Miri's CI covers anyhow, which I think has the same logic as proc-macro2? But sure we can add proc-macro2 as well.

Ideally we'd have some way of checking whether the build probe actually ran successfully, but I haven't thought of how to do that yet. At least we'll know it didn't explode, but maybe it just always disables the nightly features on Miri.

I am subscribed to the autocfg repo, which is how I found out about #358 from cuviper/autocfg#56

Ah, I was already wondering -- it couldn't be a coincidence that you added RUSTC_WORKSPACE_WRAPPER just at the same time as that showed up in autocfg.^^

I can't see cuviper/autocfg#24 being adequate for what this crate needs (in regard to rerun-if-env-changed=RUSTC_BOOTSTRAP)

That sounds like autocfg should emit the rerun-if-env-changed for that?

[dtolnay]

Autocfg emitting the rerun-if-env-changed sounds reasonable but is not a straightforward best choice. That requires build scripts to correctly emit all other "rerun-if" they rely on, which would not have been a requirement without it. Consider a build script like this:

// build.rs

use std::{env, fs, path::PathBuf};

fn main() {
    let ac = autocfg::new();
    ac.emit_probe(r#"  ...  "#, "foo");

    let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
    let data = fs::read("input.dat").unwrap();
    fs::write(out_dir.join("data.rs"), do(data)).unwrap();
}

Normally, build scripts do not need to be concerned with "rerun-if" and Cargo will automatically rerun them if rustc flags or anything in the crate's contents is modified, which works for the above.

As soon as any "rerun-if" gets printed, the semantics change to "rerun only if".

It can be problematic if some other build-dependency you rely on doesn't bother with "rerun-if" and doesn't expose any way to do it correctly yourself:

fn main() {
    let ac = autocfg::new();
    ac.emit_probe(r#"  ...  "#, "foo");

    weird_protobuf_thing::build();
}

[RalfJung]

I wonder if cuviper/autocfg#59 could still be useful -- the handling of rerun-if-env-changed would still have to be done in anyhow, but at least running the compiler could be done by shared infrastructure.

[dtolnay]

The logic on whether or not to emit rerun-if-env-changed=RUSTC_BOOTSTRAP is coupled with stuff that happens while choosing how to build the probe. A simple true/false result from a probe function is not enough to do rerun-if-env-changed correctly.

[cuviper]

Autocfg emitting the rerun-if-env-changed sounds reasonable but is not a straightforward best choice.

It doesn't do that implicitly, and I'm convinced by your argument that it would be problematic. The crate does have rerun_env() and rerun_path() as light helpers, but the main point of those is reduce the chance of syntax errors.

The new probe_raw is meant to be naive and simplistic -- try the code precisely as given and return the result, nothing more. I haven't added an emit_ version of this -- emit_raw_cfg? -- but even if I did it would only write that cfg. Either way, you could use that API and still have your own logic for rerun.

[cuviper]

Hmm, I don't have a way to do this though:

anyhow/build.rs

Lines 117 to 119 in 028cbee

	if !rustc_bootstrap {
	cmd.env_remove("RUSTC_BOOTSTRAP");
	}