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
Copy link

okkero commented Dec 2, 2024

What's keeping this right now? Can I help?

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

github-actions bot commented Feb 7, 2025

Hi there,

This PR appears to have been inactive for a while. Please let us know if you are still working on it, or if we can close it for now.

Thanks, the Renovate team

@okkero
Copy link

okkero commented Feb 10, 2025

How can I contribute to this? @rarkins @mkniewallner

@rarkins
Copy link
Collaborator

rarkins commented Feb 10, 2025

@okkero the best way would be for you to sync this branch into your own fork, then fix conflicts + coverage

@okkero
Copy link

okkero commented Feb 10, 2025

@rarkins Thanks. I might be able to take a stab at it later this week.

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:inactive-pr PR has become inactive and we want to prompt the submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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