Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace version_check with our own parser to generate better errors #26

Merged
merged 2 commits into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,4 @@ members = ["test_suite"]
[lib]
proc-macro = true

[build-dependencies]
version_check = "0.9.2"

[dependencies]
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub const fn feature() {

## Alternatives

This crate is proc-macro, but is very lightweight, depending only on [`version_check`](https://github.com/SergioBenitez/version_check) to determine the compiler version.
This crate is proc-macro, but is very lightweight, and has no dependencies.

You can manually define declarative macros with similar functionality (see [`if_rust_version`](https://github.com/ogoffart/if_rust_version#examples)), or [you can define the same function twice with different cfg](https://github.com/crossbeam-rs/crossbeam/blob/0b6ea5f69fde8768c1cfac0d3601e0b4325d7997/crossbeam-epoch/src/atomic.rs#L340-L372). (Note: the former approach requires more macros to be defined depending on the number of version requirements, the latter approach requires more functions to be maintained manually)

Expand Down
61 changes: 53 additions & 8 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
use std::{env, fs, path::PathBuf};
use std::{
env, fs,
path::{Path, PathBuf},
process::Command,
str,
};

// The rustc-cfg strings below are *not* public API. Please let us know by
// opening a GitHub issue if your build environment requires some way to enable
// these cfgs other than by executing our build script.
fn main() {
println!("cargo:rustc-cfg=const_fn_has_build_script");

let version = match Version::new() {
Some(version) => format!("{:#?}\n", version),
None => panic!("unexpected output from `rustc --version`"),
let rustc = env::var_os("RUSTC").map_or_else(|| "rustc".into(), PathBuf::from);
let version = match Version::from_rustc(&rustc) {
Ok(version) => format!("{:#?}\n", version),
Err(e) => panic!("{}", e),
};

let out_dir = env::var_os("OUT_DIR").map(PathBuf::from).expect("OUT_DIR not set");
Expand All @@ -23,11 +29,50 @@ struct Version {
nightly: bool,
}

// Based on https://github.com/cuviper/autocfg/blob/1.0.1/src/version.rs
//
// Using our own parser instead of the existing crates to generate better errors.
impl Version {
fn new() -> Option<Self> {
let (version, channel, _date) = version_check::triple()?;
let (_major, minor, patch) = version.to_mmp();
let nightly = channel.is_nightly() || channel.is_dev();
// from the verbose version output
fn from_vv(vv: &str) -> Option<Self> {
// Find the release line in the verbose version output.
let release = vv
.lines()
.find(|line| line.starts_with("release: "))
.map(|line| &line["release: ".len()..])?;
Comment on lines +36 to +42
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parses the "release: " line of the verbose version output.
This line doesn't contain extra info like the commit date, so it may be a bit more robust than the previous way.

(I suspect that the cause of #24 may be that rustc installed by yum added its own information to the output of rustc --version. So, this may solve #24 completely)

Copy link

@APIPLM APIPLM Aug 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taiki-e Okay. I will merge those to local project, and test out. I have come back to the situation, I mean install rustc and cargo version 1.45.2 by yum not by rustup.

Copy link

@APIPLM APIPLM Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taiki-e Yes. after coming back to the incorrect situation, where install rustc and cargo version 1.45.2 by yum NOT rustup. I build the older version (0.4.1)of taiki-e/const_fn, the errors came out again. That means we reduplicate that case. Then download the latest version (0.4.2) of taiki-e/const_fn, and build it and success. Then move on to build time-rs/time with updating taiki-e/const_fn the latest version (0.4.2). It success. As I check the updated source code, it like remove the dependence version_check v0.9.2 by having your code inside version check in the file build.rs. One more thing I noticed that the other crate in time-rs/time v0.2.16 has the dependence version_check v0.9.2 as I saw the downloading version_check v0.9.2 and compiling, but there is no compile issue with the case(installing rustc and cargo version 1.45.2 by yum NOT rustup).
Anyway. Thanks for your help

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@APIPLM Thanks for confirming it. Anyway, I believe #24 is completely resolved so I'll close that.

One more thing I noticed that the other crate in time-rs/time v0.2.16 has the dependence version_check v0.9.2 as I saw the downloading version_check v0.9.2 and compiling, but there is no compile issue with the case(installing rustc and cargo version 1.45.2 by yum NOT rustup).

Even if time depends on version_check, it doesn't fail to compile due to the same problem, because the build script is written so that compile will continue even if version_check fails to determine version. AFAIK, this is a common design used by most crates.
The reason const-fn is designed to make this fail is that it is likely a bug in version parser or this crate itself. (rustversion is also designed to stop compilation as well)


// Split the version and channel info.
let mut version_channel = release.split('-');
let version = version_channel.next().unwrap();
let channel = version_channel.next();

// Split the version into semver components.
let mut digits = version.splitn(3, '.');
let major = digits.next()?;
if major != "1" {
return None;
}
let minor = digits.next()?.parse().ok()?;
let patch = digits.next().unwrap_or("0").parse().ok()?;

let nightly = channel.map_or(false, |c| c == "dev" || c == "nightly");
Some(Version { minor, patch, nightly })
}

fn from_rustc(rustc: &Path) -> Result<Self, String> {
let output =
Command::new(rustc).args(&["--version", "--verbose"]).output().map_err(|e| {
format!("failed to run `{} --version --verbose`: {}", rustc.display(), e)
})?;
if !output.status.success() {
return Err("could not execute rustc".to_string());
}
let output = str::from_utf8(&output.stdout).map_err(|e| {
format!("failed to parse output of `{} --version --verbose`: {}", rustc.display(), e)
})?;

Self::from_vv(output).ok_or_else(|| {
format!("unexpected output from `{} --version --verbose`: {}", rustc.display(), output)
})
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
//!
//! # Alternatives
//!
//! This crate is proc-macro, but is very lightweight, depending only on [`version_check`](https://github.com/SergioBenitez/version_check) to determine the compiler version.
//! This crate is proc-macro, but is very lightweight, and has no dependencies.
//!
//! You can manually define declarative macros with similar functionality (see [`if_rust_version`](https://github.com/ogoffart/if_rust_version#examples)), or [you can define the same function twice with different cfg](https://github.com/crossbeam-rs/crossbeam/blob/0b6ea5f69fde8768c1cfac0d3601e0b4325d7997/crossbeam-epoch/src/atomic.rs#L340-L372).
//! (Note: the former approach requires more macros to be defined depending on the number of version requirements, the latter approach requires more functions to be maintained manually)
Expand Down