Skip to content

fix(cargo): preserve version metadata #37548

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DanielleHuisman
Copy link
Contributor

Changes

Preserve version metadata in Cargo datasource, so the registry request doesn't fail. Instead, only strip version metadata from newValue in Cargo versioning.

I tried using versionOrig in the Cargo datasource, as suggested in the issue. This fixes the registry request issue, but instead it runs into a new issue when updating the Cargo lock file using cargo update. This command also required the version metadata.

Context

#37537

Documentation

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

How I've tested my work

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

Example on a real repository: DanielleHuisman/renovate-issue-37537#2

@rarkins rarkins requested a review from zharinov August 15, 2025 14:52
Copy link
Collaborator

@rarkins rarkins 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 not sure this is logically correct.

If you are adding logic so that currentVersion -> currentValue logic is changed, then it can create a problem when currentValue -> currentVersion is unchanged.

For example this PR sets 0.2.0+metadata version to be 0.2.0 value, but if you were to run something like matches('0.2.0+metadata', '0.2.0') then it would return false.

I think we need more discussion about this before we accept a change from the solution proposed in the corresponding Issue

@rarkins
Copy link
Collaborator

rarkins commented Aug 16, 2025

What exactly was the cargo update problem?

@rarkins rarkins marked this pull request as draft August 16, 2025 07:04
@DanielleHuisman
Copy link
Contributor Author

This is the error when using the versionOrig approach:

Command failed: cargo update --config net.git-fetch-with-cli=true --manifest-path Cargo.toml --package [email protected]+8.13.36 --precise 0.3.7
error: package ID specification `[email protected]+8.13.36` did not match any packages
help: there are similar package ID specifications:

  [email protected]+8.13.52

I see I misinterpreted this error yesterday. I thought the issue was the --precise 0.3.7 part, but the --package [email protected]+8.13.36 part is the actual issue.

Apparently, the first command in cargoUpdatePrecise already updates the package, which causes the second command to fail.

@rarkins
Copy link
Collaborator

rarkins commented Aug 16, 2025

Thanks. Can you either update this PR back to that approach or submit a separate draft PR so one of us can test it locally to see if that's fixable?

@DanielleHuisman
Copy link
Contributor Author

I tried commenting out the first Cargo command and that solves the issue for this particular situation. I'm unsure if that solution works for all situations. However, given the name of the function is cargoUpdatePrecise with a comment // If all dependencies have locked versions then update them precisely., I'm wondering why the first step is to perform an "imprecise" update.

@DanielleHuisman
Copy link
Contributor Author

Thanks. Can you either update this PR back to that approach or submit a separate draft PR so one of us can test it locally to see if that's fixable?

I reverted my commit and made a new one with the versionOrig approach. This should trigger the cargo update error.

@DanielleHuisman DanielleHuisman force-pushed the fix/cargo-version-metadata branch from 7137307 to 36d8ee1 Compare August 16, 2025 09:30
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.

3 participants