search

rust-lang-nursery / lazy-static.rs

A small macro for defining lazy evaluated static variables in Rust.
Apache License 2.0
1.9k stars 111 forks source link

remove unwrap in build.rs #121

Closed xMAC94x closed 5 years ago

xMAC94x commented 5 years ago

Dear Devs, i currently had a bug in the version_check crate which causes lazy_static not to compile. Having some env. variables set makes version_check fail. and in build.rs we have a unwrap() on the version. May i ask what would happen when the rustc version is below 1.22 ? if it would fail anyway why do we need the unwrap here at all ? However, if there is a good reason for the unwrap, i see the version check is only important if the nightly feature is disables. so you we just make the unwrap only in case nightly is disables ? e.g.

    let version_geq_122 = version_check::is_min_version("1.22.0");
    let drop_in_static_supported = nightly_feature_enabled || version_geq_122.unwrap().0;

this way we wouldnt call the unwrap when nightly_feature_enabled is true.

Thank you very much

BurntSushi commented 5 years ago

Can you show an example of how to reproduce the bug that causes a build failure?

xMAC94x commented 5 years ago

sure, in this project https://gitlab.com/veloren/game init all submodules and then run:

( cd voxygen && time RUST_BACKTRACE=1 RUST_LOG=cargo=warning,server=debug,server-cli=debug,common=debug,region::vol_per=debug,world=debug cargo run --release)

but probably it would also be reproduceable in the lazy_static code if you compile it with RUST_BACKTRACE=1 RUST_LOG=cargo=warning,server=debug,server-cli=debug,common=debug,region::vol_per=debug,world=debug cargo run --release

this is the bug i opened in the version_check crate: https://github.com/SergioBenitez/version_check/issues/4

KodrAus commented 5 years ago

Hi @xMAC94x, are you able to build lazy-static now that a patch of version_check has been released?

KodrAus commented 5 years ago

Thanks again for the report @xMAC94x, I'll go ahead and close this one now because I think we'll keep the unwrap behaviour for now. If it becomes a bigger problem in the future then we'll work out what we want to do about it then.