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

Cargo: support versioning-strategy: "increase-if-necessary" #4009

Open
Tamschi opened this issue Jun 30, 2021 · 9 comments
Open

Cargo: support versioning-strategy: "increase-if-necessary" #4009

Tamschi opened this issue Jun 30, 2021 · 9 comments
Labels
L: rust:cargo Rust crates via cargo T: feature-request Requests for new features

Comments

@Tamschi
Copy link

Tamschi commented Jun 30, 2021

Rust/Cargo dependencies are generally resolved to newest supported and Cargo defaults to caret requirements, as mentioned here: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html

This means that updating the manifest/Cargo.toml requirement from e.g. "0.1.2" to "0.1.3" or from "1.2.5" to "1.3.0" is generally unnecessary to resolve the latest version, but can instead lead to unwanted dependency duplication if a dependency has a more strict requirement on that package.

For this reason, it would be very helpful to be able to skip these manifest updates where possible.
A lockfile/Cargo.lock update should still happen in these cases if applicable, but note that not forcing the increased version to (also) be used can lead to that file not being updated either.


Alternatives that I checked:

"lockfile-only" does not work for me, as this would skip major semver bumps, which I generally would like to include in order to avoid duplicates downstream.

@Tamschi Tamschi added the T: feature-request Requests for new features label Jun 30, 2021
Tamschi added a commit to Tamschi/taml that referenced this issue Jul 22, 2021
@davepacheco
Copy link

This would be very helpful!

From what I can tell, Dependabot for Cargo only supports app-like behavior (update Cargo.toml with "increase" strategy, also update Cargo.lock) and library-like behavior (update Cargo.toml using "increase-if-necessary" strategy, but Cargo.lock must not exist). We have a library that has a Cargo.lock file. (This is a little unusual but it lets us see what we're testing in CI and avoid silent breakage for developers when dependencies break us.) We want Dependabot to use "increase-if-necessary" for Cargo.toml and update Cargo.lock always. It doesn't seem like there's any way to do this. This feels surprising, since all the pieces seem to be there.

@davepacheco
Copy link

@Tamschi Looking at this commit, did you find that if you used an explicit caret requirement in Cargo.toml, then Dependabot skipped updating Cargo.toml for compatible changes? If so, this seems like a Dependabot bug, since Cargo treats "X.Y.Z" exactly the same as "^X.Y.Z", so Dependabot shouldn't treat them differently.

@Tamschi
Copy link
Author

Tamschi commented Oct 21, 2021

@Tamschi Looking at this commit, did you find that if you used an explicit caret requirement in Cargo.toml, then Dependabot skipped updating Cargo.toml for compatible changes? If so, this seems like a Dependabot bug, since Cargo treats "X.Y.Z" exactly the same as "^X.Y.Z", so Dependabot shouldn't treat them differently.

No, it turned out I was too optimistic there.

I'm currently switching my Dependabot configuration over to ignoring anything but major version dependency updates entirely as a stopgap measure (updating my projects as I touch them), but this of course means I won't automatically see if a minor version update causes an incompatibility.

I'm mostly using the same workflow as you (lib + lock), for the same reasons.

Tamschi added a commit to Tamschi/taml that referenced this issue Oct 21, 2021
… up the minimum requirements", as this doesn't work

This reverts commit 9ae1f4c.

# Conflicts:
#	Cargo.toml

See dependabot/dependabot-core#4009 .
Tamschi added a commit to Tamschi/taml that referenced this issue Oct 21, 2021
… up the minimum requirements", as this doesn't work

This reverts commit 9ae1f4c.

# Conflicts:
#	Cargo.toml

See dependabot/dependabot-core#4009 .
@jeffwidman jeffwidman added the L: rust:cargo Rust crates via cargo label Aug 25, 2022
elasticdog added a commit to EarthmanMuons/spellout that referenced this issue Jul 21, 2023
At the moment, the cargo ecosystem does not support the strategy
directly, so I'm going to see if dependabot will allow multiple
definitions for the same ecosystem in order to work around the problem.

See:
- https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#versioning-strategy
- dependabot/dependabot-core#4009
github-merge-queue bot pushed a commit to EarthmanMuons/spellout that referenced this issue Jul 21, 2023
At the moment, the cargo ecosystem does not support the strategy
directly, so I'm going to see if dependabot will allow multiple
definitions for the same ecosystem in order to work around the problem.

See:
- https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#versioning-strategy
- dependabot/dependabot-core#4009
elasticdog added a commit to EarthmanMuons/spellout that referenced this issue Jul 24, 2023
github-merge-queue bot pushed a commit to EarthmanMuons/spellout that referenced this issue Jul 24, 2023
@Pascualex
Copy link

Is this on the roadmap? It's great that Dependabot supports bumping the Cargo.lock, but that's like half of the magic.

@clechasseur
Copy link

I'm a little confused. After entering #8689 and realizing it was a duplicate, I read this documentation page that states that for cargo, the only two supported versioning strategies were auto and lockfile-only. To see if I could help, I forked this repo and checked if I could work on this.

It turns out that this seems to be already supported by the cargo gem:

if update_strategy == :bump_versions_if_necessary
update_version_requirement_if_needed(req)
else
update_version_requirement(req)
end

I was even able to prove it by running the dry-run script:

[dependabot-core-dev] ~ $ bin/dry-run.rb --requirements-update-strategy bump_versions_if_necessary cargo clechasseur/test-cargo-dependabot
=> fetching dependency files
=> dumping fetched dependency files: ./dry-run/clechasseur/test-cargo-dependabot/
=> parsing dependency files
=> updating 3 dependencies: serde, strum, syn

=== serde (1.0.172)
 => checking for updates 1/3
🌍 --> GET https://crates.io/api/v1/crates/serde
🌍 <-- 200 https://crates.io/api/v1/crates/serde
 => latest available version is 1.0.195
 => latest allowed version is 1.0.195
 => requirements to unlock: own
 => requirements update strategy: bump_versions_if_necessary
🌍 --> GET https://crates.io/api/v1/crates/serde
🌍 <-- 200 https://crates.io/api/v1/crates/serde
🌍 --> GET https://github.com/serde-rs/serde.git/info/refs?service=git-upload-pack
🌍 <-- 200 https://github.com/serde-rs/serde.git/info/refs?service=git-upload-pack
🌍 --> GET https://github.com/serde-rs/serde.git/info/refs?service=git-upload-pack
🌍 <-- 200 https://github.com/serde-rs/serde.git/info/refs?service=git-upload-pack
 => bump serde from 1.0.172 to 1.0.195

    ± Cargo.lock
    ~~~
<snip>
    ~~~
    29 insertions (+), 4 deletions (-)

My test repo's Cargo.lock has an outdated version of serde which works with the current requirement, so Cargo.toml was not updated.

However, if I try to use versioning-strategy: "increase-if-necessary" in my dependabot.yml, I get an error:

https://github.com/clechasseur/test-cargo-dependabot/pull/3/checks?check_run_id=20246936113

Your .github/dependabot.yml contained invalid details
Dependabot encountered the following error when parsing your .github/dependabot.yml:

The property '#/updates/0/versioning-strategy' value "increase-if-necessary" did not match one of the following values: lockfile-only, auto
Please update the config file to conform with Dependabot's specification.

And sure enough, the PR created by dependabot does update Cargo.toml:

https://github.com/clechasseur/test-cargo-dependabot/pull/3/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542

Try as I might, I could not find the place where versioning-strategy is actually validated in the dependabot-core code. I assume it's actually in a different repo, but I tried to look and couldn't find anything.

Is it possible to update the validation and let us specify increase-if-necessary, since the cargo gem supports it? If you point me in the right direction, I can help with a PR if required.

@cwfitzgerald
Copy link

+1 for this! This looks like exactly what we need!

@jonhoo
Copy link

jonhoo commented Mar 2, 2024

Worth pointing out that now that the guidance is for lockfiles to also be checked in for libraries, getting this change in becomes increasingly important. PRs like jonhoo/obs-do#6 are unfortunate as they also end up bumping the minimum supported Rust version by virtue of bumping the minimum dependency version unnecessarily.

@torokati44
Copy link

+1 for this from us (over at @ruffle-rs) as well.

@Kixunil
Copy link

Kixunil commented Sep 9, 2024

Not only should this be supported but it should be the default. Uselessly updating Cargo.toml is just plain wrong and it tends to be a reflection that the one who did the bump doesn't understand cargo properly (which is actually the case for the bot :)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: rust:cargo Rust crates via cargo T: feature-request Requests for new features
Projects
None yet
Development

No branches or pull requests

9 participants