search

rust-cli / human-panic

Panic messages for humans.
https://docs.rs/human-panic
Apache License 2.0
1.66k stars 65 forks source link

Determine rustc version and add it to the report #81

Open TheTechRobo opened 2 years ago

TheTechRobo commented 2 years ago

This is a 🙋 feature.

Checklist

Context

Fixes #31

Semver Changes

Major release - it changes the Report struct. I don't think 2.0 is released yet, so that would fit perfectly!

I have very little experience contributing to Rust crates, so if I made any mistakes, let me know!

bjorn3 commented 2 years ago

Cargo also sets the RUSTC env var to the path to rustc. cargo -vV may not match rustc -vV. Especially when using a locally built toolchain using rustup toolchain link as in that case the default cargo (which is probably an official version) will be combined with the local rustc which in most cases has a huge version skew.

TheTechRobo commented 2 years ago

Cargo also sets the RUSTC env var to the path to rustc.

Did not know that! I'll update that right now.

TheTechRobo commented 2 years ago

@bjorn3 - getting this issue:

   Compiling human-panic v1.0.4-alpha.0 (/home/thetechrobo/human-panic)
error: environment variable `RUSTC` not defined
 --> /home/thetechrobo/human-panic/build.rs:6:22
  |
6 |     let cargo_path = env!("RUSTC");
  |                      ^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `human-panic` due to previous error

I'm on nightly.

bjorn3 commented 2 years ago

You should use std::env::var("RUSTC").unwrap() instead. RUSTC is passed at runtime of the build script, but not at compile time.

TheTechRobo commented 2 years ago

@bjorn3 - All done.

TheTechRobo commented 2 years ago

Should I rebase this PR, or are the maintainers fine with 7 (maybe more later if there's more review) commits?

bjorn3 commented 2 years ago

It probably won't hurt to squash. No idea what the maintainers prefer though.

TheTechRobo commented 1 year ago

I'll probably have time to rebase this on Sunday.