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

Update to cfg-if 1.0 #417

Merged
merged 1 commit into from
Oct 15, 2020
Merged

Update to cfg-if 1.0 #417

merged 1 commit into from
Oct 15, 2020

Conversation

mbrubeck
Copy link
Contributor

No description provided.

@jyn514
Copy link
Member

jyn514 commented Oct 14, 2020

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 <jyn514@gmail.com>
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 }

@mbrubeck
Copy link
Contributor Author

mbrubeck commented Oct 14, 2020

+cfg-if = ">=0.1.2, ~1"

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):

$ cargo update -p cfg-if --precise 0.1.2
    Updating crates.io index
error: failed to select a version for the requirement `cfg-if = ">=0.1.2, ~1"`
candidate versions found which didn't match: 0.1.2

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 log crate maintainers decide if they want to maintain compatibility with 0.1.x.

@jyn514
Copy link
Member

jyn514 commented Oct 14, 2020

Oh, I see. It looks like cargo doesn't allow or which is unfortunate.

(Even if Cargo used “or” instead of “and” to combine the requirements, this would still be incorrect, because then it would allow versions like 2.0.0, which matches the first requirement but not the second.)

Sure, you could fix that with (>=0.1.2 AND < 1) OR ~1 if cargo had a syntax for it.

Copy link
Contributor

@KodrAus KodrAus left a 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)

@emilio
Copy link

emilio commented Dec 20, 2020

@KodrAus do you know when this will be released? Thanks!

@c410-f3r c410-f3r mentioned this pull request Jan 15, 2021
EFanZh pushed a commit to EFanZh/log that referenced this pull request Jul 23, 2023
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 <Jiahao_XU@outlook.com>
Co-authored-by: Félix Saparelli <felix@passcod.name>
EFanZh pushed a commit to EFanZh/log that referenced this pull request Jul 23, 2023
* 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 <Jiahao_XU@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants