Skip to content

PR CI is not building rust-analyzer #115031

Open
@RalfJung

Description

@RalfJung
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

Activity

added
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Aug 20, 2023
lnicola

lnicola commented on Aug 20, 2023

@lnicola
Member

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

RalfJung commented on Aug 20, 2023

@RalfJung
MemberAuthor

I think it runs ./x.py check

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

RalfJung commented on Aug 20, 2023

@RalfJung
MemberAuthor

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

RalfJung

RalfJung commented on Aug 20, 2023

@RalfJung
MemberAuthor

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

Veykril commented on Aug 20, 2023

@Veykril
Member

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.

added
A-CIArea: Our Github Actions CI
and removed
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Aug 22, 2023
RalfJung

RalfJung commented on Aug 22, 2023

@RalfJung
MemberAuthor

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

onur-ozkan commented on Aug 22, 2023

@onur-ozkan
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.

added a commit that references this issue on Aug 27, 2023

Rollup merge of rust-lang#115111 - ozkanonur:check-rust-analyzer-if-e…

ab1123b
onur-ozkan

onur-ozkan commented on Aug 27, 2023

@onur-ozkan
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

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-CIArea: Our Github Actions CI

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @lnicola@RalfJung@Veykril@onur-ozkan@rustbot

        Issue actions

          PR CI is not building rust-analyzer · Issue #115031 · rust-lang/rust