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

Include vendored crates in StaticIndex #17809

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

nicolas-guichard
Copy link
Contributor

StaticIndex::compute filters out modules from libraries. This makes an exceptions for vendored libraries, ie libraries actually defined inside the workspace being indexed.

This aims to solve https://bugzilla.mozilla.org/show_bug.cgi?id=1846041 In general StaticIndex is meant for code browsers, which likely want to index all visible source files.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2024
@lnicola
Copy link
Member

lnicola commented Aug 6, 2024

Why aren't we picking them up right now? Is it because of the Cargo.toml exclusion, or something else?

@nicolas-guichard
Copy link
Contributor Author

No this is unrelated to exclusions.
These packages have cargo_metadata::Package::source != None, which translates to PackageData::is_local = false here, which translates to PackageRoot::is_local = false here which leads to the FileSet not being added to ProjectFolders::local_filesets here, which leads to SourceRoot::is_library = true in SourceRootConfig::partition and modules were filtered out of StaticIndex when SourceRoot::is_library == true.

StaticIndex::compute filters out modules from libraries. This makes an
exceptions for vendored libraries, ie libraries actually defined inside
the workspace being indexed.

This aims to solve https://bugzilla.mozilla.org/show_bug.cgi?id=1846041
In general StaticIndex is meant for code browsers, which likely want to
index all visible source files.
@Veykril
Copy link
Member

Veykril commented Aug 7, 2024

Hmm, now I wonder is it somehow possible for us to detect (and mark) vendored dependencies? The approach in this PR works but feels a bit odd

@Veykril
Copy link
Member

Veykril commented Aug 7, 2024

I see, vendoring really is just overwriting the source to a directory. So we could figure this out by using cargo config actually to check if a source is a directory source to tell whether something is vendored
I'll merge this for now anyways as it gets the job done
@bors r+

@bors
Copy link
Contributor

bors commented Aug 7, 2024

📌 Commit 6dcc4e3 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 7, 2024

⌛ Testing commit 6dcc4e3 with merge 0c20faf...

@bors
Copy link
Contributor

bors commented Aug 7, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 0c20faf to master...

@bors bors merged commit 0c20faf into rust-lang:master Aug 7, 2024
11 checks passed
bors added a commit that referenced this pull request Aug 16, 2024
…rsky

Add scip/lsif flag to exclude vendored libaries

#17809 changed StaticIndex to include vendored libraries. This PR adds a flag to disable that behavior.

At work, our monorepo has too many rust targets to index all at once, so we split them up into several shards. Since all of our libraries are vendored, if rust-analyzer includes them, sharding no longer has much benefit, because every shard will have to index the entire transitive dependency graphs of all of its targets. We get around the issue presented in #17809 because some other shard will index the libraries directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants