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.

Sign up for GitHub

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

Jump to bottom

PR CI is not building rust-analyzer #115031

Open
RalfJung opened this issue Aug 20, 2023 · 8 comments
Open

PR CI is not building rust-analyzer #115031

RalfJung opened this issue Aug 20, 2023 · 8 comments
Labels
A-CI Area: Our Github Actions CI

Comments

@RalfJung
Copy link
Member

In #114993 I ran down the totally wrong path for a while when I saw an RA build failure in bors -- PR CI was fine, so I was sure this must have been something odd about the bors builder. Turns out however we just don't check if RA builds, not even if src/tools/rust-analyzer is changed by the PR.

I thought the mingw-check builder does at least a check-build of everything, but that does not seem to be the case?
@rust-lang/rust-analyzer @rust-lang/infra
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 20, 2023
@lnicola
Copy link
Member

lnicola commented Aug 20, 2023

I think this is intentional and actually saves quite a bit of CI time. Would it be better if we asked PR authors to not make unneeded changes (like compile fixes) to RA in this repo?

@RalfJung
Copy link
Member Author

I think it runs ./x.py check

rust/src/ci/docker/host-x86_64/mingw-check/Dockerfile

Line 43 in 4f4dae0

python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \

but that doesn't include RA. It includes clippy and Miri. RA should probably be added?

@RalfJung
Copy link
Member Author

A check-build of RA takes around 15s on my system. I don't think it will be prohibitively slow on CI.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 20, 2023 •
edited
Loading

For other tools like clippy and Miri, things work as follows:

  • We always run a check-build on the mingw-check builder. This takes less than a minute per tool so it's fine.
  • If they change, the tools builder gets enabled, and it runs their full test suite. This takes a while which is why it doesn't happen for all PRs.

At least the first bullet we should also do for RA, IMO.

@Veykril
Copy link
Member

Veykril commented Aug 20, 2023

I agree, we should have r-a be built on rust-lang/rust CI. The main point was not running r-a's (complete) test suite I believe when people investigated CI times, which is fine.

@Noratrieb Noratrieb added A-CI Area: Our Github Actions CI and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 22, 2023
@RalfJung
Copy link
Member Author

If they change, the tools builder gets enabled, and it runs their full test suite. This takes a while which is why it doesn't happen for all PRs.

Actually, turns out since 9dfec5d we always run the tools builder.

But that's not the point here, I think this is asking for a bootstrap change: ./x.py check should also build RA. Cc @rust-lang/bootstrap

@onur-ozkan
Copy link
Member

But that's not the point here, I think this is asking for a bootstrap change: ./x.py check should also build RA. Cc https://github.com/orgs/rust-lang/teams/bootstrap

We can add all the enabled tools from tools in the config.toml file to x check.

@onur-ozkan onur-ozkan mentioned this issue Aug 22, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 27, 2023
@matthiaskrgr
Rollup merge of rust-lang#115111 - ozkanonur:check-rust-analyzer-if-e…
ab1123b 
…nabled, r=Mark-Simulacrum

compile rust-anaylzer with `x check` if it's enabled

By default, `x check` doesn't compile the rust-analyzer. But when it's enabled in the config's tools section, there's no reason not to do it. This change allows `x check` to compile rust-analyzer if it's enabled in config's tools section.

Helps to rust-lang#115031
@onur-ozkan
Copy link
Member

With #115111, if enabled in the configuration, rust-analyzer will be involved in the compilation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-CI Area: Our Github Actions CI
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@lnicola @RalfJung @Veykril @onur-ozkan @rustbot @Noratrieb