Skip to content

fix(internal/librarian/rust): update version bump to handle renamed dependencies#5437

Open
jameslynnwu wants to merge 4 commits intogoogleapis:mainfrom
jameslynnwu:fix-librarian-issue-5365-20260416
Open

fix(internal/librarian/rust): update version bump to handle renamed dependencies#5437
jameslynnwu wants to merge 4 commits intogoogleapis:mainfrom
jameslynnwu:fix-librarian-issue-5365-20260416

Conversation

@jameslynnwu
Copy link
Copy Markdown
Contributor

The version bump logic for Rust libraries is updated to correctly identify and update dependencies that have been renamed in the workspace manifest using the package field.

Previously, the logic only checked if the line started with the crate name, missing cases where the crate was aliased (like wkt for google-cloud-wkt). The new approach uses regular expressions to check for both direct names and the package field, while also ensuring we do not match commented-out lines.

Fixes #5365

…ependencies

The version bump logic for Rust libraries is updated to correctly identify
and update dependencies that have been renamed in the workspace manifest
using the package field.

Previously, the logic only checked if the line started with the crate name,
missing cases where the crate was aliased (like wkt for google-cloud-wkt).
The new approach uses regular expressions to check for both direct names
and the package field, while also ensuring we do not match commented-out
lines.

Fixes googleapis#5365
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Rust manifest update logic to support renamed dependencies by using regular expressions to identify crate and package names. It also includes new test cases for these scenarios. A review comment pointed out that the packageRegex could fail if the keyword starts a line or if it appears within an inline comment, and provided a more robust regex suggestion to fix these issues.

Comment thread internal/librarian/rust/update_manifest.go Outdated
@jameslynnwu jameslynnwu marked this pull request as ready for review April 17, 2026 20:18
@jameslynnwu jameslynnwu requested a review from a team as a code owner April 17, 2026 20:18
crateName: "test-crate",
want: "[workspace.dependencies]\nother-crate = { version = \"1.0.0\", path = \"src/other-crate\" }\n",
},
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: with regex I add a test for "not-want"

}
lines := strings.Split(string(contents), "\n")
updated := false
crateRegex := regexp.MustCompile(fmt.Sprintf(`^\s*%s\s*=`, regexp.QuoteMeta(crateName)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use string matching with strings.HasPrefix instead of regex, since it is more transparent and easier to understand that regex

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this what you were looking for? Or did you also want to convert the isPackage check to prefix matching?

	lines := strings.Split(string(contents), "\n")
	updated := false
	packageRegex := regexp.MustCompile(fmt.Sprintf(`^\s*[^#]*\bpackage\s*=\s*"%s"`, regexp.QuoteMeta(crateName)))
	for i, line := range lines {
		trimmed := strings.TrimSpace(line)

		// Check if line starts with: crateName = ...
		isCrate := false
		if strings.HasPrefix(trimmed, crateName) {
			after := strings.TrimSpace(trimmed[len(crateName):])
			if strings.HasPrefix(after, "=") {
				isCrate = true
			}
		}

		// Check if line matches package regex
		isPackage := packageRegex.MatchString(line)
if (isCrate || isPackage) && versionRegex.MatchString(line) {

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.

rust: bump versions can miss wkt

3 participants