-
Notifications
You must be signed in to change notification settings - Fork 258
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
Update to cfg-if 1.0 #417
Update to cfg-if 1.0 #417
Conversation
Oops, I started working on this without noticing your PR. In mine I allowed using either 0.1 or 1.0 to avoid duplicate dependencies - do you mind doing that here? $ git show
commit 610df5155f906f00e446c9ad2f7a02041869f68c (HEAD -> cfg-if)
Author: Joshua Nelson <[email protected]>
Date: Wed Oct 14 10:18:06 2020 -0400
Update cfg-if to 1.0
There are many crates the depend on log, and if they depend on `cfg-if 1.0`, depending on 0.1 will introduce a duplicate dependency. This
allows removing the duplication.
To be even more flexible, since `log` compiles with both cfg-if 0.1 and 1.0,
this allows compiling with either, avoiding more duplication for crates
which have not yet upgraded to 1.0.
diff --git a/Cargo.toml b/Cargo.toml
index 4021fb0..e62d7e8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -49,7 +49,7 @@ kv_unstable = []
kv_unstable_sval = ["kv_unstable", "sval/fmt"]
[dependencies]
-cfg-if = "0.1.2"
+cfg-if = ">=0.1.2, ~1"
serde = { version = "1.0", optional = true, default-features = false }
sval = { version = "0.5.2", optional = true, default-features = false } |
This doesn't work as intended. When there are multiple requirements, Cargo will only allow versions that meet all of the requirements. So this is compatible with 1.0.0 (which matches both parts), but not with 0.1.2 (which matches only the first part):
Even if Cargo used “or” instead of “and” to combine the requirements, this would still be incorrect, because then it would allow versions like 3.4.5, which matches the first requirement but not the second. A valid way to do this would be: log = ">= 0.1.2, < 2.0.0" I'll let the |
Oh, I see. It looks like cargo doesn't allow
Sure, you could fix that with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this as-is. It is a little unfortunate to have 2 versions compiling in a depdency graph, but that's mostly a point-in-time issue since cfg-if
has been around so long and is so widely used. The best way to fix it is to upgrade crates to cfg-if
1.0
(it looks like there was only one small patch difference rust-lang/cfg-if#37)
@KodrAus do you know when this will be released? Thanks! |
From @passcod : * Make bin-dir an Option * Use cargo-release as a test case * WIP: Try multiple default bin dirs * Update bins.rs * Update cargo_toml_binstall.rs From @NobodyXu : * Optimize & Prepare for support multi `bin-path`s * Ensure quickinstall bin_dir work as usual. * Rm dup entries in `FULL_FILENAMES` * Add second version of `NOVERSION_FILENAMES` * Impl multiple default `bin-dir`s * Improve doc for `BinFile::from_product` * Fix tests.sh Signed-off-by: Jiahao XU <[email protected]> Co-authored-by: Félix Saparelli <[email protected]>
* Make binaries with `required-features` optional so that crates like `sccache` would work as expected. * Fix `infer_bin_dir_template`: concat with `data.bin_path` introduced in rust-lang#417 * Always `chmod +x $bin` on unix in case the archive (e.g. `sccache`) did not set the executable bit of fmt == Bin. * CI: Install `sccache` but do not run it since it requires cargo env to be ready Signed-off-by: Jiahao XU <[email protected]>
No description provided.