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

CI: attempt to fix nightly builds in next branch #1541

Merged
merged 1 commit into from
Oct 10, 2021
Merged

CI: attempt to fix nightly builds in next branch #1541

merged 1 commit into from
Oct 10, 2021

Conversation

BastiDood
Copy link
Contributor

The Problem

Currently, the next branch's nightly builds fail due to its inability to compile the dashmap crate. According to the GitHub Actions logs, it turns out that this is due to xacrimon/dashmap#133, where it was discovered that serde unexpectedly introduced a semver-breaking change in a patch release. A patch was quickly issued for [email protected].

Now, this alone should fix the issue. Though, I would later find out while investigating the logs that dashmap had been pinned to 4.0.0. Looking at the Cargo.toml, Cargo should have pulled in the latest patch of dashmap (see the dependency declaration below).

serenity/Cargo.toml

Lines 155 to 158 in 7e0b1d3

[dependencies.dashmap]
version = "4"
features = ["serde"]
optional = true

Indeed, this is the case except for nightly builds. I went through the Git blames and found that #1222 made it so that the nightly builds only pulled in the "minimal version" of dependencies. Details of this nightly feature may be found at rust-lang/cargo#4100. Attached below is the workflow configuration in question:

run: cargo clean; cargo update -Z minimal-versions; cargo check

In effect, the invocation to cargo update -Z minimal-versions must have pinned dashmap to the buggy 4.0.0, hence the compilation failures.

Possible Courses of Action

  1. Manually bump up the dependency declarations. This is the approach I took with this PR, which also happens to be the most conservative.
  2. Explicitly use patch-only semver ranges for all dependencies via tilde requirements.
  3. Revert the invocation to cargo update -Z minimal-versions so that CI will always pull in the latest semver-compatible dependencies.

@arqunis arqunis added dependencies Related to Serenity dependencies. fix A solution to an existing bug. ci Related to the continuous integration. labels Oct 10, 2021
@BastiDood
Copy link
Contributor Author

Nice! It seems that the nightly builds now compile. The other checks seem to fail due to an error with the ring crate (which is a transitive dependency). Perhaps this can be investigated on another day. For now, the nightly builds have finally been fixed! 🎉

@arqunis arqunis merged commit 40f3247 into serenity-rs:next Oct 10, 2021
@BastiDood BastiDood deleted the deps/fix-nightly-ci branch October 10, 2021 16:49
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 29, 2021
arqunis pushed a commit to arqunis/serenity that referenced this pull request Nov 15, 2021
arqunis pushed a commit to arqunis/serenity that referenced this pull request Nov 15, 2021
arqunis pushed a commit to arqunis/serenity that referenced this pull request Dec 22, 2021
arqunis pushed a commit to arqunis/serenity that referenced this pull request Dec 23, 2021
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jan 23, 2022
arqunis pushed a commit to arqunis/serenity that referenced this pull request Mar 1, 2022
arqunis pushed a commit to arqunis/serenity that referenced this pull request Mar 15, 2022
arqunis pushed a commit to arqunis/serenity that referenced this pull request Apr 2, 2022
arqunis pushed a commit to arqunis/serenity that referenced this pull request Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to the continuous integration. dependencies Related to Serenity dependencies. fix A solution to an existing bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants