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

use the manifest path instead of just the member name when using prepare --bin #255

Merged
merged 3 commits into from
Mar 10, 2024

Conversation

ClementTsang
Copy link
Contributor

@ClementTsang ClementTsang commented Feb 14, 2024

Issue #, if available:

Closes: #232

Description of changes:

The current code assumes that when running prepare with --bin, that we can remove all members and just add one member that is equal to whatever we passed in --bin.

The problem with that is that if your packages aren't in the exact same root directory, then this will result in a recipe file that breaks once you try calling cook on it.

To fix this, this PR instead sets the sole member to the path of the package, which (I think) can be found using in cargo_metadata::Metadata, grabbing the manifest path, and setting it to be relative to the workspace root + removing Cargo.toml from the path. I chose to do it this way rather than just matching + grabbing whatever's in the members array as I don't think that would work due to wildcards being valid.

This also adds a test emulating various package locations in a workspace and seeing if the path is correctly set. This previously failed, but now passes with these changes. All other previously-existing tests seem to pass, so I don't believe this should break things. That said, if people were using the workaround mentioned in #232, then I believe this will be a breaking change, so that'll probably have to be communicated.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ClementTsang ClementTsang changed the title use the manifest path instead of just the member name when using --bin with prepare use the manifest path instead of just the member name when using prepare --bin Feb 14, 2024
Comment on lines 322 to 345
let new_member_path = pkg
.manifest_path
.strip_prefix(workspace_root)
.ok()
.and_then(|mp| mp.parent());
Copy link
Owner

Choose a reason for hiding this comment

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

Let's use https://docs.rs/pathdiff/latest/pathdiff/fn.diff_paths.html here. pathdiff is already in the dependency tree.

@LukeMathWalker
Copy link
Owner

Thank you for the excellent PR ❤️
I only have one comment related to path manipulation. Once that is solved we can merge.

@ClementTsang
Copy link
Contributor Author

Rebased to the newest main and cleaned up a bit along with the suggestion.

@LukeMathWalker LukeMathWalker merged commit f889c9f into LukeMathWalker:main Mar 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants