use cargo rustc for macro expansion so -Zbuild-std can be supported#5214
use cargo rustc for macro expansion so -Zbuild-std can be supported#5214skrap wants to merge 1 commit into
cargo rustc for macro expansion so -Zbuild-std can be supported#5214Conversation
skrap
force-pushed
the
bugfix/allow-zbuild-std-macro-expansion
branch
4 times, most recently
from
0689c20 to
b5da051
Compare
| &mut cargo_toml, | ||
| r#" | ||
| [package] | ||
| name = "libc" |
There was a problem hiding this comment.
This will need to be configurable:
- Move this function body to a new
expand_with_namethat takes aname: Option<&str>parameter - Use that parameter in the template here, or
ctest-expansionifNone expandcan then callexpand_with_namewithNoneto keep the public API- Add a
crate_name(&str)function toTestGeneratorthat saves it in the config - Replace internal calls to
expandwithexpand_with_name, using the storedcrate_nameif available
There was a problem hiding this comment.
Since it works as is, what's the need for this change? I think everything using this is either "libc" or it doesn't care if it is called "libc" for the purposes of macro expansion.
There was a problem hiding this comment.
It's going to be confusing if ctest users unrelated to libc get an error compiling libc, and also other users could be setting crate_name
There was a problem hiding this comment.
Ah, I didn't realize this crate was used outside of the libc context. Thanks. Also, slightly terrifying for me, ha.
There was a problem hiding this comment.
This change also drops the ability to use the RUSTC env var to use a custom rustc. I don't know if there's any way to keep that when using cargo rustc or not. Given that there's external users, I will have to check on that.
There was a problem hiding this comment.
Not a lot of places, but at least a few important places 🙂 https://github.com/search?q=path%3ACargo.toml+%2F%5Cbctest%5Cb.*%3D%2F&type=code. Certainly nothing to worry about, though, it's a low pressure crate.
|
Cc @mbyx for ctest |
tgross35
added
the
stable-unneeded
skrap
force-pushed
the
bugfix/allow-zbuild-std-macro-expansion
branch
from
b5da051 to
1100563
Compare
| [lib] | ||
| path = '{}' | ||
| [lints.rust] | ||
| unexpected_cfgs = "allow" |
There was a problem hiding this comment.
Add a toml comment about why
| name = "libc" | ||
| edition = "{EDITION}" | ||
| [lib] | ||
| path = '{}' |
There was a problem hiding this comment.
Use a regular string here and escape it by replacing \->\\ and "->\", since this will act weird in the (unlikely) case the path contains a '
| .arg("--edition") | ||
| .arg(EDITION) // By default, -Zunpretty=expanded uses 2015 edition. | ||
| .arg(canonicalize(crate_path)?); | ||
| .env("RUSTFLAGS", "") // ignore RUSTFLAGS so we don't get -Dwarnings |
There was a problem hiding this comment.
Add a TestGenerator::expansion_rustflags(&str) and set RUSTFLAGS only if that has been called, otherwise just let it inherit. Since there might be some global options needed to build.
|
|
||
| if env::var("LIBC_CI_ZBUILD_STD").is_ok() { | ||
| cmd.arg("-Zbuild-std=core,std"); | ||
| } |
There was a problem hiding this comment.
Add TestGenerator::build_std(bool) and TestGenerator::build_core(bool) to control this, which should set -Zbuild-std=std / -Zbuild-std=core.
| let mut cargo_toml = std::fs::File::create(&cargo_toml_path)?; | ||
| writeln!( | ||
| &mut cargo_toml, |
There was a problem hiding this comment.
Nit: just construct the string with format! then use fs::write to avoid bothering with the File object.
| cfg: &[(String, Option<String>)], | ||
| target: String, | ||
| ) -> Result<String> { | ||
| let rustc = env::var("RUSTC").unwrap_or_else(|_| String::from("rustc")); |
There was a problem hiding this comment.
Keep something similar to this but with CARGO for setting something not in-path.
|
Reminder, once the PR becomes ready for a review, use |
3 participants
Description
Address #5204 by invoking macro expansion through
cargo rustcinstead of rustc directly. This allows the use of the-Zbuild-stdflag, enabling targets without a prebuilt std (e.g. tier 3 targets) to build the libc tests.Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI