cargo-asm icon indicating copy to clipboard operation
cargo-asm copied to clipboard

cargo asm ignores flags set in .cargo/config

Open yodaldevoid opened this issue 6 years ago • 9 comments

I am building an embedded project that has a few flags set in a ".cargo/config" file. cargo asm is unable to build my project as it ignores the rustflags field set for the target.

yodaldevoid avatar Jul 18 '18 01:07 yodaldevoid

Thanks for filling the issue.

cargo asm is just a script that calls cargo, so if cargo picks these flags in .cargo/config, cargo asm should too, unless one of the options passed to cargo is incompatible with some of the flags that are set there.

Do you have a reproducer? Or could you upload to github a tiny project that I can use to reproduce this?

gnzlbg avatar Jul 18 '18 06:07 gnzlbg

Looking at your code it seems like the reason that the flags set in .cargo/config are being ignored is because you are explicitly setting the RUSTFLAGS environment variable. Setting RUSTFLAGS overrides the settings in .cargo/config as per the docs.

As for how to fix this, I'm not entirely sure. BuildContext::rustflag_args in the cargo crate might do what you need, but the documentation is lacking. If that fails, I think you could do something similar to how cargo does it. The env_args function builds the flags up and the relevant functions seem to be available publicly in the cargo crate.

I will see if I can't get you a project to test with later today.

yodaldevoid avatar Jul 18 '18 13:07 yodaldevoid

As for how to fix this, I'm not entirely sure. BuildContext::rustflag_args in the cargo crate might do what you need, but the documentation is lacking.

Just note that we are not using cargo as a library here.

Looking at your code it seems like the reason that the flags set in .cargo/config are being ignored is because you are explicitly setting the RUSTFLAGS environment variable.

IIRC there is an alternative way of passing RUSTFLAGS to cargo using cargo rustc ... -- ...RUSTFLAGS go here..., maybe we could migrate to using that instead, but that cargo API did not have as many features as the cargo build API back when this was built. Hopefully that has changed. What I don't know is if cargo rustc respects the flags in .cargo/config or not.

gnzlbg avatar Jul 18 '18 13:07 gnzlbg

Just note that we are not using cargo as a library here.

that cargo API did not have as many features as the cargo build API back when this was built.

Yes, I noticed that you were not calling cargo as a library and instead was calling it as a normal program. Do you know what features were missing in the past from the library API that you required?

I think I see another project forming to add to my ever growing list...

yodaldevoid avatar Jul 18 '18 13:07 yodaldevoid

Yes, I noticed that you were not calling cargo as a library and instead was calling it as a normal program.

FWIW there is a good reason for this, and that is that this allows you to don't have to re-compile and re-install cargo asm when you upgrade your Rust toolchain, or switch toolchains (e.g. from stable to nightly and back). That is, if you want to check the asm generated by two different Rust toolchain versions, you just use rustup to switch version, and cargo asm picks the proper cargo for that Rust version automatically. It also significantly reduces cargo asm compile-times... but that's a bonus.

Do you know what features were missing in the past from the library API that you required?

I just remember that I know that the cargo rustc API exists because I tried to use it here, but that did not work, but I don't remember why (lack of support for --feature ... maybe? I really can't remember). Maybe if I try to use that today it would work. Trying it out should be as simple as replacing cargo build with cargo rustc in the build file, and instead of setting the RUSTFLAGS environment variable, passing them to cargo rustc as the last argument .arg(format!("-- {}", rustflags)) or similar (e.g. one at a time after the --).

I think I see another project forming to add to my ever growing list...

I think that cargo metadata might have improved a lot in the past year to support IDEs, so maybe everything that we need for this might be already available. I (or somebody) just needs to check it out.

gnzlbg avatar Jul 18 '18 14:07 gnzlbg

My past self actually thought about using cargo rustc and left a comment: https://github.com/gnzlbg/cargo-asm/blob/master/src/build.rs#L42

But I still don't know if cargo rustc respects .cargo/config or not... That should probably be the first thing to check out before proceeding here, because if it doesn't, then changing the code to use it won't fix this issue.

gnzlbg avatar Jul 18 '18 14:07 gnzlbg

cargo rustc does indeed look at .cargo/config; for example, if you set rustflags = ["-C", "target-cpu=native"] (or whatever) under the [build] section and run cargo rustc --verbose, the printed invocation contains those flags (after all the ones passed by Cargo).

cole-miller avatar Aug 23 '20 18:08 cole-miller

Would be nice if this was fixed. I just spent a lot of time trying to find out why the portable intrinsic I was using didn't compile to the correct instruction, only to discover it was because cargo-asm didn't use the assigned target feature.

lassemoldrup avatar Feb 09 '21 14:02 lassemoldrup

+1 to fix this.

I also get confused a lot with the ASM ouput (when using --target-cpu=native in my config for instance). I don't get the proper instruction. The only workaround I found is to repeat the flags in the environment variable just before calling cargo asm like: RUSTFLAGS='-C target-cpu=native' cargo asm.

Generally, I think it's a good practice to avoid configuration duplication.

In any case thanks for this crate, super useful when developping performance driven applications.

jimzer avatar Feb 23 '21 12:02 jimzer