Switch from Travis to GHA by JohnTitor · Pull Request #1073 · rust-lang/rustc-dev-guide

JohnTitor · 2021-03-02T16:42:50Z

Fixes #940

Some scripts are taken from https://github.com/rust-lang/std-dev-guide and https://github.com/rust-lang/simpleinfra. Some actions log can be found here: https://github.com/JohnTitor/rustc-dev-guide/actions

TODO: We should also switch required status from Travis to GHA.

JohnTitor · 2021-03-02T16:44:42Z

.github/workflows/ci.yml

+          cargo install mdbook --version ${{ env.MDBOOK_VERSION }}
+          cargo install mdbook-linkcheck --version ${{ env.MDBOOK_LINKCHECK_VERSION }}
+          cargo install mdbook-toc --version ${{ env.MDBOOK_TOC_VERSION }}


I haven't come up with a good way to use --version ^x.y.z while using actions/cache.

This seems fine to me.

I guess that means we'll have to manually update these tools more often?

Is it worth it to introduce some automation to update those dependencies?

Well, I guess we might be able to get dependabot setup, but I'm not sure if it's worth doing it here.

AFAIK dependabot only updates Cargo.toml or Cargo.lock so it doesn't help us. But an automation job or something else would be great as follow-up work.

I guess that means we'll have to manually update these tools more often?

Yes, we can still use --version ^x.y.z specifying but it doesn't update the mdbook and their tools version due to the cache key.

.github/workflows/ci.yml

jyn514 · 2021-03-02T16:57:42Z

.github/workflows/ci.yml

+          git switch -c ci
+          git fetch origin master
+          git rebase origin/master
+          git log --oneline | head -n 10


Why does this rebase over master? Doesn't github do that automatically? What happens on merge conflicts?

I just copy-pasted from the travis script, this is no longer required now?

(IIRC we introduced it to avoid broken link failures that are already fixed on master.)

I know that rust-lang/rust runs CI after rebasing against master; I don't think they do that explicitly and github handles it automatically. Not sure though.

That sounds surprising to me, but a quick search for 'rebase' in rust's src/ci found no result. 🤷‍♂️

IIRC this was introduced when we did huge refactorings on librustc, which causes a ton of link failures. Now the broken rate is low, so we could remove rebasing, I think.

ci/linkcheck.sh

jyn514 · 2021-03-02T18:05:36Z

Personally I think this is good as is and we can fix any bugs as they arise :) maybe @spastorino or @LeSeulArtichaut want to take a look though?

camelid · 2021-03-02T18:16:25Z

.github/workflows/ci.yml

+  schedule:
+    # Run at 18:00 UTC every day
+    - cron: '0 18 * * *'


Is this the link-check cronjob?

From what I understand, all the steps in the file are executed when master or a PR is pushed to, and at a regular interval (cron job). Is that correct?

I think you're partially correct, but there are several instances of #1073 (comment).

Is this the link-check cronjob?

Yeah, it's the same as what we have on travis.

Is there a reason to not have this in a separate yml file inside of workflows?

Is there a reason to not have this in a separate yml file inside of workflows?

I don't think it's worth having duplicate workflows with multiple files.

camelid · 2021-03-02T18:17:16Z

.github/workflows/ci.yml

+          cargo install mdbook --version ${{ env.MDBOOK_VERSION }}
+          cargo install mdbook-linkcheck --version ${{ env.MDBOOK_LINKCHECK_VERSION }}
+          cargo install mdbook-toc --version ${{ env.MDBOOK_TOC_VERSION }}


I guess that means we'll have to manually update these tools more often?

camelid · 2021-03-02T18:18:00Z

.github/workflows/ci.yml

+          key: ${{ runner.os }}-${{ hashFiles('./book/linkcheck') }}
+
+      - name: Check line lengths
+        if: github.event_name != 'push'


Why doesn't this run on push?

I'm not sure we should block the deployment by the line length failure. It will be caught by the cron job anyway.

Will the line-length check run for PRs? I was confused about whether push was just for pushes to master, or if it included PR pushes as well.

Will the line-length check run for PRs? I was confused about whether push was just for pushes to master, or if it included PR pushes as well.

push here means just for master as we restrict it here: https://github.com/JohnTitor/rustc-dev-guide/blob/4c70da4512e1019a4deb0de9160791965b73a6a8/.github/workflows/ci.yml#L4-L6

For instance, I pushed this branch but the workflow wasn't triggered.
https://github.com/JohnTitor/rustc-dev-guide/tree/doesnt-trigger-event

ci/linkcheck.sh

.github/workflows/ci.yml

LeSeulArtichaut · 2021-03-02T18:38:44Z

.github/workflows/ci.yml

+          cargo install mdbook --version ${{ env.MDBOOK_VERSION }}
+          cargo install mdbook-linkcheck --version ${{ env.MDBOOK_LINKCHECK_VERSION }}
+          cargo install mdbook-toc --version ${{ env.MDBOOK_TOC_VERSION }}


Is it worth it to introduce some automation to update those dependencies?

LeSeulArtichaut · 2021-03-08T22:33:52Z

Mhh, btw we won't be able to merge this without disabling the required CI pass from Travis.

Is this waiting on something else?

JohnTitor · 2021-03-09T22:25:31Z

Is this waiting on something else?

There should not be any blocker on this PR, I think. But yeah, we have to tweak the required status.
@Mark-Simulacrum may I ask you?

Mark-Simulacrum · 2021-03-10T15:55:49Z

I'll need to figure out how to merge - it looks like GitHub is not seeing the CI/ci workflow yet, maybe it needs to run on a non-PR first. I think maybe the easiest thing is to just override the protections, merge into master, and then go back and try and configure things.

LeSeulArtichaut · 2021-03-10T16:05:05Z

👍 overriding the protections seems good to me

Mark-Simulacrum · 2021-03-10T17:33:46Z

Updated the requirements.

Update books ## nomicon 1 commits in adca786547d08fe676b2fc7a6f08c2ed5280ca38..6fe476943afd53a9a6e91f38a6ea7bb48811d8ff 2021-02-16 16:34:20 +0900 to 2021-03-10 07:28:57 +0900 - Merge pull request rust-lang/nomicon#257 from skade/opaque-types-fix ## reference 3 commits in 3b6fe80c205d2a2b5dc8a276192bbce9eeb9e9cf..e32a2f928f8b78d534bca2b9e7736413314dc556 2021-02-22 22:09:17 -0800 to 2021-03-08 23:24:30 -0800 - Clarify that ::foo paths are not necessarily based off of the "crate root" (rust-lang/reference#974) - Comment typo (rust-lang/reference#977) - Fix misspelled word discrimnant (rust-lang/reference#976) ## book 2 commits in 0f87daf683ae3de3cb725faecb11b7e7e89f0e5a..fc2f690fc16592abbead2360cfc0a42f5df78052 2021-03-01 08:54:04 -0500 to 2021-03-05 14:03:22 -0500 - Fix wrapping - fix: redundant double introduction of shadowing (rust-lang/book#2633) ## rust-by-example 1 commits in 3e0d98790c9126517fa1c604dc3678f396e92a27..eead22c6c030fa4f3a167d1798658c341199e2ae 2021-02-25 08:23:10 -0300 to 2021-03-04 16:26:43 -0300 - Fix grammar "terminates" -> "terminate" (rust-lang/rust-by-example#1423) ## rustc-dev-guide 15 commits in c431f8c..67ebd4b 2021-02-28 16:35:20 -0500 to 2021-03-11 13:36:25 -0800 - Remove extra the (rust-lang/rustc-dev-guide#1088) - Fix double-word typos (rust-lang/rustc-dev-guide#1084) - I-nominated are nominated for discussion (rust-lang/rustc-dev-guide#1080) - Complete unfinished statement - Check `BASE_SHA` only if it's a PR (rust-lang/rustc-dev-guide#1083) - Update lins - Apply suggestions from code review - Add stub about the THIR - Switch from Travis to GHA (rust-lang/rustc-dev-guide#1073) - Adjust a bit better P- label text - Fix typos (rust-lang/rustc-dev-guide#1079) - Update cmake version in prerequisites.md (rust-lang/rustc-dev-guide#1077) - Fix typo: suceed -> succeed - Add article on using WPA to profile rustc memory usage on Windows (rust-lang/rustc-dev-guide#1074) - Use more accurate estimate of generated LLVM IR with llvm-lines ## embedded-book 1 commits in a96d096cffe5fa2c84af1b4b61e1492f839bb2e1..f61685755fad7d3b88b4645adfbf461d500563a2 2021-02-17 08:08:52 +0000 to 2021-03-08 01:06:44 +0000 - Swap to GHA (rust-embedded/book#285)

Switch from Travis to GHA

374c29b

JohnTitor commented Mar 2, 2021

View reviewed changes

JohnTitor mentioned this pull request Mar 2, 2021

Add custom_domain key for github-actions/static-websites rust-lang/simpleinfra#61

Closed

jyn514 reviewed Mar 2, 2021

View reviewed changes

Address review comments

4c70da4

JohnTitor force-pushed the switch-to-gha branch from ddca529 to 4c70da4 Compare March 2, 2021 17:36

camelid reviewed Mar 2, 2021

View reviewed changes

LeSeulArtichaut reviewed Mar 2, 2021

View reviewed changes

Address the review comments, part 2

1a56a3b

Mark-Simulacrum approved these changes Mar 10, 2021

View reviewed changes

Mark-Simulacrum merged commit 674f73d into rust-lang:master Mar 10, 2021

JohnTitor deleted the switch-to-gha branch March 10, 2021 16:53

ehuss mentioned this pull request Mar 16, 2021

Update books rust-lang/rust#83206

Merged

Conversation

JohnTitor commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camelid Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jyn514 commented Mar 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeSeulArtichaut commented Mar 8, 2021

Uh oh!

JohnTitor commented Mar 9, 2021

Uh oh!

Mark-Simulacrum commented Mar 10, 2021

Uh oh!

LeSeulArtichaut commented Mar 10, 2021

Uh oh!

Mark-Simulacrum commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JohnTitor commented Mar 2, 2021 •

edited

Loading

camelid Mar 2, 2021 •

edited

Loading