1846041 - Searchfox doesn't index third-party Rust code (under third_party/) because of rust-analyzer's behavior around Cargo.toml exclusions

Status: RESOLVED FIXED

(Webtools :: Searchfox, defect)

Description

[Jim Blandy :jimb]

Searchfox seems to not understand types defined in third_party/rust, making it less useful for anything that talks to such code.

For example, in wgpu_server_adapter_pack_info, I don't get any hover highlights or useful context menu for the fields of wgt::AdapterInfo like device_type. The context menu on AdapterInfo itself doesn't seem to be able to find its definition in third_party/rust/wgpu-types, only other uses elsewhere.

And third_party/rust/wgpu-core/src/instance.rs doesn't seem to have been indexed at all.

In the Searchfox matrix chat room, Emilio said:

yeah, we need to look into why rust-analyzer is not analyzing third-party crates and see if we can do something easy for it to do so

so I gather that this is not deliberate behavior.

If there is some reason that we're excluding third_party/rust from Searchfox, could we get an exception for third_party/rust/wgpu-*, at least?

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jim Blandy :jimb from comment #0)

If there is some reason that we're excluding third_party/rust from Searchfox, could we get an exception for third_party/rust/wgpu-*, at least?

The current rust-analyzer invocation is currently very straightforward

	cd $(topsrcdir)/ && \
          CARGO=$(MOZ_FETCHES_DIR)/rustc/bin/cargo \
          RUSTC=$(MOZ_FETCHES_DIR)/rustc/bin/rustc \
          $(MOZ_FETCHES_DIR)/rustc/bin/rust-analyzer scip . && \
          zip -r5D '$(ABS_DIST)/$(PKG_PATH)$(MOZSEARCH_SCIP_INDEX_BASENAME).zip' \
          index.scip

It seems like this means the exclusion is coming from our root Cargo.toml explicitly excluding the directory:

# Excluded crates may be built as dependencies, but won't be considered members
# of the workspace and their dev-dependencies won't be included.
exclude = [
  # Exclude third-party code vendored into mozilla-central.
  "servo",
  "third_party/rust",

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Note that :emilio landed an enhancement in https://github.com/rust-lang/rust-analyzer/pull/15633 that maybe could help? There is still a to-do to leverage it from https://github.com/mozsearch/mozsearch/pull/652 that motivated the enhancement.

We ran out of contractor budget so there's no one to look at this unless someone volunteers (or is able to convince their manager to find contractor budget if they can't provide time for someone to look at it).

Comment 4

[Nicolas Guichard]

Attachment: rust-analyzer Pull Request

Comment 5

[Nicolas Guichard]

The issue isn't that third_party/rust is listed in exclude. The issue is that third_party crates which use the Cargo replacement mechanism are still marked as external libraries by rust-analyzer which simply skips them when building its StaticIndex. For instance third_party/rust/bindgen/lib.rs is correctly indexed because it is added with a relative path in Cargo.toml#149 instead of relying on the replacement rules of .cargo/config.toml.
See the attached PR for more details.

While trying this out on mozilla-central I noticed that the time crate was causing trouble, filed https://github.com/rust-lang/rust-analyzer/issues/17811 about this.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Fantastic, thank you!

Comment 7

[Nicolas Guichard]

Attachment: Bug 1846041 - Fix Rust Nightly's new elided_named_lifetimes warning. r=glandium!

This warning was introduced by https://github.com/rust-lang/rust/pull/129207.

Comment 8

[Nicolas Guichard]

Attachment: Bug 1846041 - Adjust mozglue build script for rustc 1.83. r=glandium!

Comment 9

[Nicolas Guichard]

Attachment: Bug 1846041 - Allow unreachable_patterns for TryInto with platform-dependant types. r=glandium!

Comment 10

[Nicolas Guichard]

Attachment: Bug 1846041 - Extend max_build_output_timeout for Searchfox builds. r=glandium!

Starting with toolchain 1.82 beta, rust-analyzer now indexes all
vendored crates, and sometimes hits the default 40 minutes timeout.

Comment 11

[Nicolas Guichard]

Attachment: Bug 1846041 - Switch Searchfox builds to Rust 1.82-beta.5. r=glandium!

Support for indexing vendored Rust crates was introduced in
rust-analyzer 2024-08-12 and is currently only available in the
1.82 beta and nightly toolchains.

We can revert this once 1.82 hits stable.

Comment 12

[Phabricator Automation]

Comment on attachment 9423525 [details]
Bug 1846041 - Adjust mozglue build script for rustc 1.83. r=glandium!

Revision D221463 was moved to bug 1917744. Setting attachment 9423525 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9423524 [details]
Bug 1846041 - Fix Rust Nightly's new elided_named_lifetimes warning. r=glandium!

Revision D221462 was moved to bug 1917746. Setting attachment 9423524 [details] to obsolete.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d19ec13f3c2
Extend max_build_output_timeout for Searchfox builds. r=glandium
https://hg.mozilla.org/integration/autoland/rev/1e2bba8c4f63
Switch Searchfox builds to Rust 1.82-beta.5. r=glandium

Comment 15

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5d19ec13f3c2
https://hg.mozilla.org/mozilla-central/rev/1e2bba8c4f63

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Just to put numbers on what we saw in terms of indexing job runtimes (which was expected and explicitly fine), runtimes before and after landing (anecdotal, no math done):

• linux x64 debug: 85 minutes to 109 minutes
• win 2012 x64 debug: 88 minutes to 113 minutes
• osx cross debug 86 minutes to 113 minutes
• osx aarch64 cross debug 84 minutes to 107 minutes
• android 5.0 aarch64 debug: 107 minutes to 159 minutes
• ios debug 71 minutes to 96 minutes

This does raise the question of whether we need to split the rust part of the job out so it can be parallelized or increase the capacity of the builder to avoid causing indexing latency problems for searchfox. Since we already wait 5 hours (300 minutes) for coverage data, we still have quite a large amount of head room, which is to say we don't need to split the job.

I think the only question is then whether the longer runtime increases the probability of "infrastructure failures" like when taskcluster was on EC2 and using spot instances and the instances could potentially be killed. Right now I do some "retry" and "exception" failures with the exceptions explicitly marked as bug 1922641 where it seems like the ~AZ has run out of resources which could also explain the retries being requested after 12 minutes and only being started after 42 minutes. The retry deadlines still leave us with enough headroom, and I think the meta issues on worker retries is probably something best left to taskcluster ops who can file a bug against searchfox if there's a need to slice and dice the the jobs.