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 registry - respect renamed dependencies #32430

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

Conversation

usbalbin
Copy link

@usbalbin usbalbin commented Nov 5, 2024

rust allows renaming dependencies such as when depending on multiple versions of the same package. This is not supported by gitea as discovered in #31500 . This PR tries to address that.

Not yet tested

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 5, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 5, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Nov 5, 2024
@usbalbin
Copy link
Author

usbalbin commented Nov 5, 2024

This is my first PR to gitea and first ever two lines of go code so please keep that in mind :)

From what I can tell there is some sort of testing machinery in tests/integration/api_packages_cargo_test.go . Should I update that to test this?

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but I'm not certain if that is doing everything it should.
It's probably best if @KN4CK3R looks at it as well.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 5, 2024
@usbalbin
Copy link
Author

usbalbin commented Nov 5, 2024

The links i used:
API :
publish

{
    [...]
    "deps": [
        {
            // Name of the dependency.
            // If the dependency is renamed from the original package name,
            // this is the original name. The new package name is stored in
            // the `explicit_name_in_toml` field.
            "name": "rand",
            [...]
            // If the dependency is renamed, this is a string of the new
            // package name. If not specified or null, this dependency is not
            // renamed.
            "explicit_name_in_toml": null,
        }
    ],
    [...]
}

Registry index
JSON schema

[...]
"deps": [
        {
            // Name of the dependency.
            // If the dependency is renamed from the original package name,
            // this is the new name. The original package name is stored in
            // the `package` field.
            "name": "rand",
            [...]
            // If the dependency is renamed, this is a string of the actual
            // package name. If not specified or null, this dependency is not
            // renamed.
            "package": null,
    }
    [...]
}

@@ -137,14 +137,15 @@ func parsePackage(r io.Reader) (*Package, error) {
dependencies := make([]*Dependency, 0, len(meta.Deps))
for _, dep := range meta.Deps {
dependencies = append(dependencies, &Dependency{
Name: dep.Name,
Name: dep.ExplicitNameInToml,
Copy link
Author

Choose a reason for hiding this comment

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

This should probably default to dep.Name if dep.ExplicitNameInToml is not set

@lunny
Copy link
Member

lunny commented Nov 5, 2024

It's better to have a test for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/go Pull requests that update Go code size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants