Alias Cargo dependency renames at the crates hub - #190
isaacparker0 wants to merge 3 commits into
Conversation
| version = version, | ||
| )) | ||
|
|
||
| for rename, fqs in workspace_dep_rename_versions_by_name.items(): |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c37161f71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef23e02074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hub_contents.append(""" | ||
| alias( | ||
| name = "{rename}-{version}", | ||
| actual = "{actual}", | ||
| )""".format(rename = rename, version = version, actual = _target_label(target_repo_name, target_package_path, real_name))) |
There was a problem hiding this comment.
Account for workspace-member aliases before emitting renames
When a workspace member named foo at version 1.0.0 depends on a different external package renamed to foo at the same version, this emits foo-1.0.0, and the later cargo_metadata["packages"] loop emits another alias with that exact name for the workspace member. Because workspace members are excluded from versions_by_name, the preceding collision guard does not catch this valid Cargo configuration, and the generated hub BUILD file fails to load due to the duplicate target.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
is this actually a valid cargo configuration?
| if rename in versions_by_name: | ||
| # A real crate already owns this name at the hub; don't let a | ||
| # same-named rename overwrite its alias. | ||
| continue |
There was a problem hiding this comment.
Preserve renames that match only transitive crate names
When a dependency is renamed to foo and an unrelated crate named foo exists only transitively in the lockfile, foo is present in versions_by_name but absent from workspace_dep_versions_by_name, since Cargo metadata was requested with --no-deps. This guard therefore suppresses every renamed alias even though no unversioned @hub//:foo target was generated for the transitive crate, so the declared-name label this change intends to provide remains missing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this seems potentially accurate?
some_alias = { version = "1", package = "real_crate_name" }only generated@crates//:real_crate_name. Also generate the alias so BUILD files can reference by declared name.