Skip to content

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Oct 5, 2025

Part of #147370.
MCP: rust-lang/compiler-team#923

This PR prepares rust-analyzer crates with in-rust-tree cargo featues where needed, and and updates bootstrap to run the main rust-analyzer tests in rust-lang/rust CI, not just the proc-macro-srv crate tests.

This supersedes the earlier attempt at #136779. I was honestly expecting more failures in this PR, but looking back at the previous attempt, that makes sense because we no longer run i686-mingw (32-bit windows-gnu) which had a bunch of these failures. In the earlier attempt I also disabled the i686-mingw-related failures for i686-msvc since I didn't feel like digging into 32-bit msvc at the time. Try results from this PR shows that it's most likely limited to 32-bit windows-gnu specifically.

rust-analyzer test remarks

  • I actually had to remove the CARGO_WORKSPACE_DIR expect-test-hack in order for expect-test to be able to find the test expectation HTML files (for syntax_highlighting tests in ide). When I added the hack, ironically, it made expect-test unable to find the expectation files. I think this was because previously the path was of the proc-macro-srv crate specifically, now we point to the root r-a workspace?
  • The cfg-related differences on aarch64-apple-darwin might've been fixed? I can't tell, but we don't seem to be observing the differences now.
  • I'm not sure why config::{generate_config_documentation, generate_package_json_config} no longer fails. Perhaps they were fixed to no longer try to write to source directory?

Review remarks

  • Commit 1 updates r-a crates that are involved in tests needing artifacts from rustc_private compiler crates to use the in-rust-tree cargo feature. I briefly tried to use a plain --cfg=in_rust_tree, but quickly realized it was very hacky, and needed invasive bootstrap changes. The cargo feature approach seems most "natural"/well-supported to both bootstrap and cargo.
  • Commit 2 updates bootstrap to not only run the proc-macro-srv tests, but the whole r-a tests.

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
try-job: aarch64-msvc-1

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Oct 5, 2025
@jieyouxu
Copy link
Member Author

jieyouxu commented Oct 5, 2025

@bors try

rust-bors bot added a commit that referenced this pull request Oct 5, 2025
[DO NOT MERGE] Run main rust-analyzer tests

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
try-job: aarch64-msvc-1
@rust-bors

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment was marked as outdated.

@rust-log-analyzer

This comment was marked as outdated.

@jieyouxu
Copy link
Member Author

jieyouxu commented Oct 5, 2025

Huh.
@bors try

rust-bors bot added a commit that referenced this pull request Oct 5, 2025
[DO NOT MERGE] Run main rust-analyzer tests

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
try-job: aarch64-msvc-1
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Oct 5, 2025

☀️ Try build successful (CI)
Build commit: 867471f (867471f2085391d229c964771d52579ba984c324, parent: e2c96cc06bdbdbc6f59c7551194d6a742260d6ff)

@Kivooeo
Copy link
Member

Kivooeo commented Oct 5, 2025

I'd consider this a win?

@jieyouxu

This comment was marked as resolved.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Oct 5, 2025
… r=Kobzol

bootstrap: don't build book redirect pages during dry-run/test

Currently, `./x test bootstrap` does not automatically transitively checkout submodules needed to pass all involved test steps. Apparently one place where bootstrap's self-test can choke on locally is trying to build book redirect pages without the book submodules checked out.

This change is orthogonal to making bootstrap checking out required submodules for self-tests, and IMO is beneficial regardless since IMO we should not be building these redirect pages during test/dry-run _anyway_.

This was blocking me trying to rebless bootstrap self-tests for rust-lang#147372. cf. [#t-infra/bootstrap > Bootstrap self-tests @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20self-tests/near/543157194).

r? `@Kobzol` (or bootstrap)
rust-timer added a commit that referenced this pull request Oct 5, 2025
Rollup merge of #147374 - jieyouxu:bootstrap-redirect-pages, r=Kobzol

bootstrap: don't build book redirect pages during dry-run/test

Currently, `./x test bootstrap` does not automatically transitively checkout submodules needed to pass all involved test steps. Apparently one place where bootstrap's self-test can choke on locally is trying to build book redirect pages without the book submodules checked out.

This change is orthogonal to making bootstrap checking out required submodules for self-tests, and IMO is beneficial regardless since IMO we should not be building these redirect pages during test/dry-run _anyway_.

This was blocking me trying to rebless bootstrap self-tests for #147372. cf. [#t-infra/bootstrap > Bootstrap self-tests @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20self-tests/near/543157194).

r? `@Kobzol` (or bootstrap)
@jieyouxu jieyouxu force-pushed the rust-analyzer-main-tests branch from bcc2aae to c72221d Compare October 6, 2025 08:44
@jieyouxu jieyouxu force-pushed the rust-analyzer-main-tests branch from c72221d to 0af3c68 Compare October 6, 2025 08:45
Comment on lines 474 to 480
// FIXME: teach rust-analyzer to use `RUST_ANALYZER_TEST_DIR` for its test output root
// directory. In the rust-lang/rust CI, we separate checkout directory vs build directory,
// where the checkout directory is read-only whereas build directory (including test output
// directories) is writable.
let dir = testdir(builder, target);
t!(fs::create_dir_all(&dir));
cargo.env("RUST_ANALYZER_TEST_DIR", dir);
Copy link
Member Author

@jieyouxu jieyouxu Oct 6, 2025

Choose a reason for hiding this comment

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

Remark: I added this mostly for review discussion, remove before merge if r-a wants to use another approach or don't need this.

Copy link
Member Author

@jieyouxu jieyouxu Oct 6, 2025

Choose a reason for hiding this comment

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

Actually, let me remove this, this was an intermediate thing. Tracking this as a follow-up in #147370.

@jieyouxu
Copy link
Member Author

jieyouxu commented Oct 6, 2025

r? @Kobzol (for bootstrap side)
r? @Veykril (for r-a side)

FYI @rust-lang/rust-analyzer

@jieyouxu jieyouxu marked this pull request as ready for review October 6, 2025 09:02
@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 6, 2025
@jieyouxu jieyouxu changed the title [DO NOT MERGE] Run main rust-analyzer tests Run main rust-analyzer tests in rust-lang/rust CI Oct 6, 2025
@jieyouxu jieyouxu force-pushed the rust-analyzer-main-tests branch from 0af3c68 to bbfdd24 Compare October 6, 2025 09:04
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

The changes look fine, but I'm not sure if this doesn't require an MCP. We should also check the CI durations.

View changes since this review

@jieyouxu
Copy link
Member Author

jieyouxu commented Oct 6, 2025

The changes look fine, but I'm not sure if this doesn't require an MCP.

I will open one if just for the visibility, even if in the end we find that it's not technically needed.

@jieyouxu jieyouxu force-pushed the rust-analyzer-main-tests branch from bbfdd24 to bb43aca Compare October 6, 2025 09:23
@jieyouxu jieyouxu added the needs-mcp This change is large enough that it needs a major change proposal before starting work. label Oct 6, 2025
@jieyouxu jieyouxu added S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. and removed needs-mcp This change is large enough that it needs a major change proposal before starting work. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants