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

Automate management of broken tools #45861

Closed
nrc opened this issue Nov 8, 2017 · 9 comments
Closed

Automate management of broken tools #45861

nrc opened this issue Nov 8, 2017 · 9 comments
Assignees
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Nov 8, 2017

Rather than using the tools manifest for opting-in to breaking a tool.

The rough plan: on master (but not beta or stable) the CI would ignore failing to build or test some tools (RLS, Clippy, etc). However, some bot would note when a tool started to fail and email interested parties (and/or display the info in a dashboard).

This should lower the friction for contributors working in the Rust repo when they make tool-breaking changes. However, we might miss some 'true positive' failure in the tools (e.g., #45820).

@nrc nrc added A-build T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 8, 2017
@kennytm kennytm added C-feature-request Category: A feature request, i.e: not implemented / a PR. I-nominated labels Nov 8, 2017
@kennytm
Copy link
Member

kennytm commented Nov 8, 2017

We could do it like this on the Travis CI side:

  1. Change the purpose of the cargotest job and aux job.

    • The old cargotest job becomes the new aux job. It will run:
      • cargotest
      • pretty tests
    • The old aux job becomes the tools job. It will run test against external tools:
      • cargo, rls, rustfmt, miri, clippy
  2. Don't run the tools job in auto branch. (Promote something to tier-2 to take its place)

    • Alternatively, still run the job but mark it as allow_failure. That's rather pointless though.
  3. Run the tools job whenever the branch is master/beta/stable and not a PR.

    • We could also run this in a PR, so reviewer can inspect the ❌ and determine whether it is legit or spurious, to reduce the chance a "true positive" is missed. This will double the resource spent on testing PRs, and won't reflect the actual behavior when merged to master.
  4. Monitor the Travis badge.

Rough sketch of the change:

diff --git a/.travis.yml b/.travis.yml
index db34f14044..a28954761b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -161,16 +161,16 @@ matrix:
       if: branch = auto
     - env: IMAGE=i686-gnu-nopt
       if: branch = auto
-    # - env: IMAGE=wasm32 issue 42646
-    #   if: branch = auto
+    - env: IMAGE=wasm32
+      if: branch = auto
     - env: IMAGE=x86_64-gnu
       if: branch = auto
     - env: IMAGE=x86_64-gnu-full-bootstrap
       if: branch = auto
+    - env: IMAGE=x86_64-gnu-tools
+      if: branch IN (master, beta, stable) # (<- uncomment to disable PR test) AND type != pull_request
     - env: IMAGE=x86_64-gnu-aux
       if: branch = auto
-    - env: IMAGE=x86_64-gnu-cargotest
-      if: branch = auto
     - env: IMAGE=x86_64-gnu-debug
       if: branch = auto
     - env: IMAGE=x86_64-gnu-nopt

Not sure what will be the corresponding change on the AppVeyor side.

@kennytm
Copy link
Member

kennytm commented Nov 13, 2017

Ideas after discussion with @nrc,

  1. We will create a repository to store the current test status. The Travis+AppVeyor job will push a commit whenever the test result changes. The dev-tools team will create a dashboard to read from this repository.

  2. The commit message will @-mention any party responsible for the tool.

@nrc nrc removed the I-nominated label Nov 14, 2017
@nrc
Copy link
Member Author

nrc commented Nov 14, 2017

We discussed this at the dev-tools meeting today. We mostly discussed the implementation details. One interesting issue that came up: we want to avoid notification except on changing from testing to not testing. That requires some state in the 'bot' doing the monitoring. One way to avoid security concerns is to use a GitHub repo (I've done this for rustaceans.org and findwork). We might then be able to rely on GitHub notifications for doing the email notification when a tool breaks.

@kennytm kennytm self-assigned this Nov 14, 2017
@kennytm kennytm added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Nov 17, 2017
@kennytm
Copy link
Member

kennytm commented Nov 17, 2017

Tracker.

Changes to the rust-lang/rust CI system

(Note: we need to disable tools jobs from auto before enabling @-mentions to prevent spurious mentions due to failing builds.)

Other changes

  • Build a dashboard using the content of the new repository?

kennytm added a commit to kennytm/rust that referenced this issue Nov 21, 2017
…miri).

If build failed for these tools, they will be automatically skipped from
distribution, and will not fail the whole build.

Test failures are *not* ignored, nor build failure of other tools (e.g.
cargo). Therefore it should have no observable effect to the current CI
system.

This is step 1/8 of automatic management of broken tools rust-lang#45861.
bors added a commit that referenced this issue Nov 28, 2017
[auto-toolstate][1/8] Always ignore build failure of failable tools (rls, rustfmt, clippy)

If build failed for these tools, they will be automatically skipped from distribution, and will not fail the whole build.

Test failures are *not* ignored, nor build failure of other tools (e.g. cargo). Therefore it should have no observable effect to the current CI system.

This is step 1/8 of automatic management of broken tools #45861. The purpose is concentrate all failure detection about tools into a single CI job for easy management, while keeping the ability to distribute these tools in the nightlies.

r? @Mark-Simulacrum
bors added a commit that referenced this issue Dec 3, 2017
…-fast, r=alexcrichton

[auto-toolstate][2+3/8] Move external tools tests into its own job with --no-fail-fast

This PR performs these  things:

1. The `aux` job now performs "cargotest" and "pretty" tests. The clippy/rustfmt/rls/miri tests are moved into its own job.
2. These tests are run with `--no-fail-fast`, so that we can get the maximum number of failures of all tools from a single CI run.
3. The test results are stored into a JSON file, ready to be uploaded in the future.

This is step 2 and 3/8 of automatic management of broken tools #45861.
bors added a commit that referenced this issue Dec 8, 2017
…and-remove-toolstate-toml, r=<try>

[WIP] [auto-toolstate][4–8/8] Upload the toolstate result to an external git repository, and removes BuildExpectation

This PR consists of 3 commits.

1. (Steps 4–6) The `toolstate.json` output previously collected is now pushed to the https://github.com/******/rust-toolstate repository.
2. (Step 7) Revert commit ab018c7, thus removing all traces of `BuildExpectation` and `toolstate.toml`.
3. (Step 8) Adjust CONTRIBUTION.md for the new procedure.

These are the last steps of #45861. After this PR, the toolstate will be automatically computed and published to https://******.github.io/rust-toolstate/. There is no need to manage toolstate.toml again.

Closes #45861.
bors added a commit that referenced this issue Dec 19, 2017
…and-remove-toolstate-toml, r=<try>

[WIP] [auto-toolstate][4–8/8] Upload the toolstate result to an external git repository, and removes BuildExpectation

This PR consists of 3 commits.

1. (Steps 4–6) The `toolstate.json` output previously collected is now pushed to the https://github.com/rust-lang-nursery/rust-toolstate repository.
2. (Step 7) Revert commit ab018c7, thus removing all traces of `BuildExpectation` and `toolstate.toml`.
3. (Step 8) Adjust CONTRIBUTION.md for the new procedure.

These are the last steps of #45861. After this PR, the toolstate will be automatically computed and published to https://rust-lang-nursery.github.io/rust-toolstate/. There is no need to manage toolstate.toml again.

Closes #45861.
bors added a commit that referenced this issue Dec 26, 2017
…and-remove-toolstate-toml, r=alexcrichton

[auto-toolstate] Upload the toolstate result to an external git repository, and removes BuildExpectation

This PR consists of 3 commits.

1. (Steps 4–6) The `toolstate.json` output previously collected is now pushed to the https://github.com/rust-lang-nursery/rust-toolstate repository.
2. (Step 7) Revert commit ab018c7, thus removing all traces of `BuildExpectation` and `toolstate.toml`.
3. (Step 8) Adjust CONTRIBUTION.md for the new procedure.

These are the last steps of #45861. After this PR, the toolstate will be automatically computed and published to https://rust-lang-nursery.github.io/rust-toolstate/. There is no need to manage toolstate.toml again.

Closes #45861.
@nrc
Copy link
Member Author

nrc commented Jan 4, 2018

There doesn't seem to be anywhere else to track it, so #47146 is another example of a PR breaking rustfmt, and causing a real bug, which should have been noticed by the author if tool bustage was not handled automatically.

@kennytm kennytm reopened this Jan 4, 2018
@kennytm
Copy link
Member

kennytm commented Jan 4, 2018

Reopened as tracking issue.

(The breakage from #47146 is noted as soon as the PR is merged: rust-lang-nursery/rust-toolstate@1b316cc)

@kennytm
Copy link
Member

kennytm commented Feb 21, 2018

Follow-up changes:

@rust-lang rust-lang deleted a comment from kennytm-githubbot Feb 21, 2018
bors added a commit that referenced this issue Feb 28, 2018
…crum

Auto-toolstate management follow-up.

Tracking comment: #45861 (comment)

* Fixed rust-lang-nursery/rust-toolstate#1, a proper link to the PR will be included.
* Fixed rust-lang-nursery/rust-toolstate#2, a comment will be posted to the PR if the toolstate changed
* Toolstate regression will be rejected at the last week of the 6-week cycle (currently entirely date-based).
* Implemented https://internals.rust-lang.org/t/the-current-submodule-setup-is-not-tenable/6593, moved doc tests of Nomicon, Reference, Rust-by-Example and The Book to the "tools" job and thus allowed to fail like other external tools.
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 1, 2018
…Mark-Simulacrum

Auto-toolstate management follow-up.

Tracking comment: rust-lang#45861 (comment)

* Fixed rust-lang-nursery/rust-toolstate#1, a proper link to the PR will be included.
* Fixed rust-lang-nursery/rust-toolstate#2, a comment will be posted to the PR if the toolstate changed
* Toolstate regression will be rejected at the last week of the 6-week cycle (currently entirely date-based).
* Implemented https://internals.rust-lang.org/t/the-current-submodule-setup-is-not-tenable/6593, moved doc tests of Nomicon, Reference, Rust-by-Example and The Book to the "tools" job and thus allowed to fail like other external tools.
@jonas-schievink jonas-schievink added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-build labels Apr 21, 2019
@steveklabnik
Copy link
Member

Triage: only one or two boxes unchecked, but I'm not 100% sure what the current status is.

@Mark-Simulacrum
Copy link
Member

Pretty sure this is basically all done or not needed today, though there is ongoing work in this area (subtrees making toolstate entirely unnecessary, as well as #65000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. 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

5 participants