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

Exempt Miri from "no tools breakage the week before beta cutoff"? #62266

Closed
RalfJung opened this issue Jul 1, 2019 · 7 comments
Closed

Exempt Miri from "no tools breakage the week before beta cutoff"? #62266

RalfJung opened this issue Jul 1, 2019 · 7 comments
Labels
A-miri Area: The miri tool T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 1, 2019

Would it make sense to exempt Miri from the rule that tools must not regress the week before the beta is branched? Miri is anyway not distributed with the beta, it's a nightly-only tool. And this "no breakage" rule makes landing PRs during that timeframe very painful.

Cc @rust-lang/infra @oli-obk

@jonas-schievink jonas-schievink added A-miri Area: The miri tool T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 1, 2019
@Centril
Copy link
Contributor

Centril commented Jul 5, 2019

This is also painful for the person doing rollup ops most of the time (me) so I think this is a great idea. =P

@pietroalbini
Copy link
Member

Discussed this at the infra meeting, we're ok merging a patch for this.

@rustbot modify labels: -I-nominated

@RalfJung
Copy link
Member Author

I tried to implement this, but I am confused. I am looking at src/ci/docker/x86_64-gnu-tools/checktools.sh, which does:

check_dispatch() {
    if [ "$1" = submodule_changed ]; then
        # ignore $2 (branch id)
        verify_status $3 $4
    elif [ "$2" = beta ]; then
        echo "Requiring test passing for $3..."
        if check_tool_failed "$3"; then
            exit 4
        fi
    fi
}

status_check() {
    check_dispatch $1 beta book src/doc/book
    check_dispatch $1 beta nomicon src/doc/nomicon
    check_dispatch $1 beta reference src/doc/reference
    check_dispatch $1 beta rust-by-example src/doc/rust-by-example
    check_dispatch $1 beta edition-guide src/doc/edition-guide
    check_dispatch $1 beta rls src/tools/rls
    check_dispatch $1 beta rustfmt src/tools/rustfmt
    check_dispatch $1 beta clippy-driver src/tools/clippy
    # these tools are not required for beta to successfully branch
    check_dispatch $1 nightly miri src/tools/miri
    check_dispatch $1 nightly embedded-book src/doc/embedded-book
    check_dispatch $1 nightly rustc-guide src/doc/rustc-guide
}

So this already looks like if $1 is beta_required, the last 3 listed tools will not be checked. But then why do we see PRs getting rejected when they break Miri during the beta week?

Cc @pietroalbini @kennytm

@RalfJung
Copy link
Member Author

Oh, wow, okay the code is much more spaghetti than I thought.

The beta_required there is not for doing the check the week before the beta cutoff, it is for doing the check when we are on the beta (or stable) branch.

@kennytm
Copy link
Member

kennytm commented Jul 18, 2019

Someday we should to rewrite all these 🍝 bash script using Python (or Rust) 🤷.

I think #62759 (comment) should have exempted miri.

@RalfJung
Copy link
Member Author

Someday we should to rewrite all these spaghetti bash script using Python (or Rust) 🤷.

Yeah, the usual problem with CI/infrastructure scripts. ;)

Not having things spread across 2 or 3 files (in 2 totally different directories!) would also help.

@RalfJung
Copy link
Member Author

This was fixed by #62759.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-miri Area: The miri tool T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants