cuviper / autocfg

Automatic cfg for Rust compiler features
Apache License 2.0
96 stars 24 forks source link

Doesn't tries to determine correct target #68

Open boozook opened 1 month ago

boozook commented 1 month ago

As we all know, env TARGET contains rustc's "short target name", not exactly target-triple. It can be triple only if builtin target requested. So autoconf can't use it to probe if some custom target requested, e.g. for --target=my-spec.json env TARGET will be "my-spec".

There are many problems about it, here is just one for example.

It could be great to

  1. first of all, determine "is that target valid/existing?" - probably just call rustc --print cfg --target my-spec or any other "tell me something about this target" (?)
    • exit-code 0 if yes
    • exit-code != 0 if unknown / not found, that means that spec is not in $PWD or rustc's target dir (or just invalid)
    • note that rustc _can use $PWD/my-spec.json for requested --target my-spec (without ext)
    • note, for build-scripts $PWD point to the crate root in most cases, so it where we probably can find my-spec.json if needed, or not 🤷🏻‍♂️
  2. if rustc returns error we can try to construct kinda "compatible base target" (?) with env like this:
    use std::env::var;
    let maybe_triple = format!(
       "{}-{}-{}-{}",
       var("CARGO_CFG_TARGET_ARCH").unwrap(),
       var("CARGO_CFG_TARGET_VENDOR").unwrap(),
       var("CARGO_CFG_TARGET_OS").unwrap(),
       var("CARGO_CFG_TARGET_ABI").unwrap()
    );

    , also changing unknown vendor and OS to unknown and none respectively.

  3. and of course, we can try to find a file by name at $PWD/my-spec.json (or $CARGO_MANIFEST_DIR/my-spec.json) and then use full path of it as target for probes

Actual error on probe:

[my_crate 0.1.0] error[E0463]: can't find crate for `core`
[my_crate 0.1.0]   |
[my_crate 0.1.0]   = note: the `my-spec` target may not be installed
[my_crate 0.1.0]   = help: consider downloading the target with `rustup target add my-spec`
[my_crate 0.1.0]   = help: consider building the standard library from source with `cargo build -Zbuild-std`
boozook commented 1 month ago

As an first ugly hot-fix I could suggest something like following.

Instead of that let target = env::var_os("TARGET"); do following:

let target = env::var_os("AUTOCONF_TARGET").or_else(|| env::var_os("TARGET"))?;

So we can override the target passed to autoconf with env AUTOCONF_TARGET.

cuviper commented 1 month ago

I think would prefer to have Cargo pass through the --target option in the environment, either directly in TARGET or in a new variable if that "short name" behavior can't be changed.

But even with the right target spec (especially if it can be found in $PWD), you're probably use -Zbuild-std, right? So then the issue is more like #34?

boozook commented 1 month ago

I agree, but this issue is about more than just build-std, you know.
It's about "respect and use CARGO_CFG_TARGET_... and try to configure compiler similarly to requested target as possible for probe".

boozook commented 4 weeks ago

I've did some research and realized that it's impossible to guess where spec-file is in most cases, as well as some other necessary info. So I filled proposal-issue rust-lang/cargo/issues 14208.

cuviper commented 3 weeks ago

I'm considering that override hack, but one thing is that I think it should only take effect for TARGET != HOST, because an external environment can't make that distinction while you're building crates natively for build scripts and proc-macros.

In theory, we should also emit cargo::rerun-if-env-changed=NAME for this, and maybe also a PATH one if we see it's a spec file, but that's troublesome from a library since having any rerun-if output disables the default that watches all files. So I think instead we'd have to leave this as a documented caveat about the override.

boozook commented 3 weeks ago

TARGET != HOST

For this we can compare env vars TARGET with HOST.

In theory, we should also emit cargo::rerun-if-env-changed=NAME for this, and maybe also a PATH one if we see it's a spec file, but that's troublesome from a library since having any rerun-if output disables the default that watches all files. So I think instead we'd have to leave this as a documented caveat about the override.

If I remember correctly... Doesn't cargo independently monitor variables (actually cfg- values) that are significant for compilation? Env vars produced from the config, and when you change the config (all is the config!), the cargo invalidates the existing artifacts and rebuild starting from them. So, we brolly shouldn't care about it, if I right.

if we see it's a spec file

Here is the problem. We can't know for sure what it is. And we can't guess because it's very likely that we won't find the json file (our WPD != initial PWD, same for .ext, also there can be symlinks), because only cargo knows the absolute file path. You know.

cuviper commented 3 weeks ago

Doesn't cargo independently monitor variables (actually cfg- values) that are significant for compilation? Env vars produced from the config, and when you change the config (all is the config!), the cargo invalidates the existing artifacts and rebuild starting from them.

It doesn't know that AUTOCFG_OVERRIDE_WHATEVER is significant. So if you tried to build once and failed all your autocfg probes (but still exit 0 from the build script), and then you try to set a target override, Cargo needs to know that the build script should be rerun. Or, we just document that the override isn't tracked and you should clean first.

For anything that comes from your cargo proposal, yes it should automatically handle changes.

boozook commented 3 weeks ago

Yes, now Ive got it, thanks for explanation. I suppose we should try to tell cargo to track, firstly. If it really break incremental builds (causes full rebuild every time), so will think. May be that isn't good decision.

But I think that in ordinary pipeline the AUTOCFG_OVERRIDE_WHATEVER will be changed simultaneously with TARGET, so I hope there won't be any problems here.