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

Add include and exclude information to metadata #13331

Open
GuillaumeGomez opened this issue Jan 21, 2024 · 2 comments
Open

Add include and exclude information to metadata #13331

GuillaumeGomez opened this issue Jan 21, 2024 · 2 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-metadata Command-package S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@GuillaumeGomez
Copy link
Member

Problem

I'm currently working on rust-lang/rust-clippy#11677. However, to finish this lint, I need to have the include and exclude information in the metadata (to be used with cargo_metadata).

Proposed Solution

I think that to prevent having to recompute include and exclude when using it from metadata and duplicate what cargo does, it should contain the equivalent of cargo package --list. An initial implementation was done in

Notes

No response

@GuillaumeGomez GuillaumeGomez added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Jan 21, 2024
@epage
Copy link
Contributor

epage commented Jan 22, 2024

FYI the title and problem are still focused on one specific solution, rather than the problem at hand.

Going to copy oer from #13330 @weihanglo's comment which has some good context when considering solutions:

While it might be useful to have include and exclude in cargo metadata output, it doesn't seem to be a proper way to fix rust-lang/rust-clippy#11677. The solution might end up reimplement the logic Cargo find and package files, which includes .gitignore, shell glob syntax, symlinks handling, git dirty status, and other details (see #11405). cargo package --list might be more reliable, but not 100% accurate (see #8407 and #11666).

I would suggest starting the discussion from an issue, and jump back to implementation when the design is settled.

For the most part, cargo package --list should be fine for your use case since this involves the source code looking up paths the same way. For other use cases,
we'd likely need to output more than cargo package --list but the mapping cargo makes from source to destination (file moves, resolving of symlinks, whether the Cargo.lock is generated or pulled from the root, etc). We'd need to be confident enough in this new data structure to meet our compatibility guarantees.

Besides those limitations in cargo package --list, it can be slow. I currently use it in cargo release to identify which packages are affected by which commits (making me a candidate for that extended use case). I worry that if we include it within cargo metadata, we'll negatively impact all other users.

We could have clippy run cargo package --list only if that lint is enabled but then that will forever keep this as a niche lint.

@GuillaumeGomez
Copy link
Member Author

I was pretty sure that I was missing on the overall complexity on cargo side hence why I implemented it with just the fields values and why I was planning to retrieve the information from clippy by emulating how cargo was computing it. ^^'

@epage epage added S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-metadata Command-package S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

No branches or pull requests

2 participants