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

fix: bin_aliases should come from packages #1031

Merged
merged 1 commit into from
May 10, 2024

Conversation

mistydemeo
Copy link
Contributor

It's possible for packages to have their own specific bin_aliases. We should always read from the package metadata version and skip the workspace version. Since we clone the workspace value into the package if the package has no version to compare to, that's safe.

cargo-dist/src/tasks.rs Outdated Show resolved Hide resolved
It's possible for packages to have their own specific bin_aliases. We should
always read from the package metadata version and skip the workspace version.
Since we clone the workspace value into the package if the package has no
version to compare to, that's safe.
@mistydemeo mistydemeo force-pushed the use_package_specific_bin_aliases branch from f1cb9f6 to e611026 Compare May 9, 2024 23:17
@@ -1729,7 +1729,7 @@ impl<'pkg_graph> DistGraphBuilder<'pkg_graph> {
warn!("skipping shell installer: not building any supported platforms (use --artifacts=global)");
return;
};
let bin_aliases = self.inner.bin_aliases.for_targets(&target_triples);
Copy link
Member

Choose a reason for hiding this comment

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

where does this change come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used to look up the workspace value from the DistGraph (self.inner), but now we're reading it from release.

@Gankra Gankra merged commit fa28a9c into main May 10, 2024
16 checks passed
@Gankra Gankra deleted the use_package_specific_bin_aliases branch May 10, 2024 13:07
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.

None yet

3 participants