Skip to content

Move CI to rust-cache Action#8427

Merged
bors[bot] merged 3 commits into
rust-lang:masterfrom
Swatinem:ci-caching
May 3, 2021
Merged

Move CI to rust-cache Action#8427
bors[bot] merged 3 commits into
rust-lang:masterfrom
Swatinem:ci-caching

Conversation

@Swatinem
Copy link
Copy Markdown
Contributor

@Swatinem Swatinem commented Apr 8, 2021

This is humbling. I actually took inspiration from RAs pre-cache xtask when developing my action ;-)

Closes #7731

Comment thread xtask/src/pre_cache.rs
pub(crate) fn run(self) -> Result<()> {
let slow_tests_cookie = Path::new("./target/.slow_tests_cookie");
if !slow_tests_cookie.exists() {
panic!("slow tests were skipped on CI!")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worth keeping this check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

@lnicola lnicola Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a check to make sure we set those two variables. We could test them in the workflow, but it badly needs a refactoring (I know it's possible to remove the duplication, but I don't know the syntax).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I guess we can just remove this. If, at some point we notice that slow tests are not being run, we'll learn that the check was helpful in the end :)

Comment thread .github/workflows/ci.yaml
@Swatinem
Copy link
Copy Markdown
Contributor Author

Swatinem commented Apr 8, 2021

Pinned the action to Swatinem/rust-cache@ce325b6
Not really sure what to do about the sanity check.

The Rust Cross check time is cut in half, since previously it didn’t cache the dependencies, which it now does. Otherwise its a wash, as expected.

@matklad
Copy link
Copy Markdown
Contributor

matklad commented Apr 19, 2021

Assigning @lnicola to make a judgement call here :-)

@matklad
Copy link
Copy Markdown
Contributor

matklad commented May 3, 2021

Ok, let's just land it -- we can always go back if we want!

bors r+

@lnicola
Copy link
Copy Markdown
Member

lnicola commented May 3, 2021

Yeah, sorry for letting this slip. LGTM.

@bors
Copy link
Copy Markdown
Contributor

bors Bot commented May 3, 2021

@lnicola
Copy link
Copy Markdown
Member

lnicola commented Sep 16, 2021

@Swatinem I don't think we've had any caching issues after the PR, so I guess it's working :-).

@Swatinem Swatinem deleted the ci-caching branch September 28, 2021 13:13
@Swatinem
Copy link
Copy Markdown
Contributor Author

Thats very good to hear. Also thanks @matklad for mentioning the action, the number of likes has exploded since your blog post :-D

@matklad
Copy link
Copy Markdown
Contributor

matklad commented Jan 4, 2022

@Swatinem FYI: https://old.reddit.com/r/rust/comments/rvqxkf/does_sccache_really_help/hr7kla6/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: investigate rust-cache

3 participants