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

feat(manager/cargo): support git dependencies #32235

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mkniewallner
Copy link
Contributor

@mkniewallner mkniewallner commented Oct 30, 2024

Changes

Add support for git dependencies in cargo manager.

Context

Closes #26531.

Cargo specification: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories.

There is one change I am really not sure about.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Tested against https://github.com/mkniewallner/renovate-rust-git-dependencies.

@mkniewallner mkniewallner force-pushed the feat/support-git-dependencies-cargo branch 2 times, most recently from cddc619 to bf33681 Compare November 2, 2024 14:46
@mkniewallner mkniewallner force-pushed the feat/support-git-dependencies-cargo branch from bf33681 to ab69e05 Compare November 2, 2024 14:47
// Non-crate dependencies (like git ones) do not have locked versions.
// For crate dependencies, not having a locked version is not expected.
// In both situations, perform a regular workspace lockfile update.
if (nonCrateDep || crateDepWithoutLockedVersion) {
Copy link
Contributor Author

@mkniewallner mkniewallner Nov 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fully sure of the logic here, but without this change, we would display a warning that appears in the dependency dashboard when updating git dependencies (warning was added in #25983).

On the overall logic, technically cargo update --precise supports git dependencies, but I think the logic is different that the one for crates.

For instance on https://github.com/mkniewallner/renovate-rust-git-dependencies/blob/2eb824eb730a37b108f1d4eec951c027a031905a/Cargo.toml, when running cargo update without arguments, serde and transitive dependencies will get updated, but git dependencies will be left untouched:

 [[package]]
 name = "serde"
-version = "1.0.213"
+version = "1.0.214"
 
 [[package]]
 name = "serde_derive"
-version = "1.0.213"
+version = "1.0.214"
 
 [[package]]
 name = "syn"
-version = "2.0.85"
+version = "2.0.86"

And trying cargo update ruff_python_parser --precise 0.7.0 will lead to those changes in the lock file:

 [[package]]
 name = "ruff_python_parser"
 version = "0.0.0"
-source = "git+https://github.com/astral-sh/ruff?tag=0.6.1#499c0bd875c3f53c65f542a217b4d9a0962191c3"
+source = "git+https://github.com/astral-sh/ruff?tag=0.6.1#5e6de4e0c69660e8ca8608d1ac965216197756ce"

where the commit do resolve to 0.7.0, but while keeping 0.6.1 tag reference in the source + Cargo.toml, which feels wrong?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you split this back to Discussion - either existing or new - assuming that a design decision needs making? It's hard to follow threads in PR reviews

@mkniewallner mkniewallner marked this pull request as ready for review November 2, 2024 16:06
@okkero

This comment was marked as outdated.

@rarkins rarkins added the auto:inactive-pr PR has become inactive and we want to prompt the submitter label Feb 7, 2025

This comment was marked as outdated.

@okkero

This comment was marked as outdated.

@rarkins

This comment was marked as outdated.

@okkero

This comment was marked as outdated.

@mkniewallner
Copy link
Contributor Author

mkniewallner commented Feb 10, 2025

Hey, sorry for the delay, got quite busy lately. There was one kinda blocker that required some discussion in #32235 (comment). I'll check if it still applies (and also resolve conflicts).

@okkero

This comment was marked as spam.

@rarkins rarkins removed the auto:inactive-pr PR has become inactive and we want to prompt the submitter label Mar 20, 2025
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.

feat(manager/cargo): support for git dependencies
3 participants