Optimize CI caching and path filtering #2340

Madoshakalaka · 2022-01-07T08:46:23Z

Per-job Caching

each job now has its own cache keys.
Previously all jobs have the same cache. This is mad inefficient.
Cache from the nightly toolchain doesn't help at all in the same job with the stable toolchain for example.

Also added missing npm caching for website-related workflows.

stale cache invalidation

cargo caches aggressively and leaves a lot of outdated dependencies in the cache.
cargo-sweep is introduced to detect and clean unvisited cache during each job.
Also added a manually-dispatchable weekly cron job to build the cargo-sweep binary and commit to the ci/ directory.

Path Filtering Workflows

Touching website/** now won't trigger irrelevant workspace tests.

~~The same goes the other way around, touching other stuff won't trigger website workflows.~~

edit: touching other stuff should trigger website tests because the snippets depend on them.

~~Combined Website building and publishing~~

Previously two workflows were set up sequentially to do the building and publishing. The artifact download and upload can take a minute and a huge chunch of complex steps are dedicated to
setting variables and communicating between the two workflows. They are now combined with a job:needs key to preserve the sequential nature.

Remove check-examples job in the pull-request workflow

It is covered by a previous clippy --all-targets job and only increases build time

Extract cargo fmt job as workflow

It doesn't need any caching and can be completed concurrently as fast as possible instead of blocking the pull-request workflow.

Renaming

pull-request.yml is renamed to main-checks.yml

un-exclude `tool/website-test`

I can't figure out the reasoning for excluding the package in the workspace. It would complicate caching setup so I included it and the tests pass smoothly locally. I'll see how it goes in the CI.

Fixes ~~#0000~~ complaints about slow CI speed

CI is tricky to get right, I expect this PR to go through some painful followup edits :D

# Conflicts: # .github/workflows/publish-examples.yml # .github/workflows/pull-request.yml # website/package-lock.json

move website test as a prerequisite for publish

Madoshakalaka · 2022-01-07T09:02:42Z

tools/website-test/build.rs

-            writeln!(dst, "#[doc = include_str!(\"{}\")]", file.display())?;
+
+            writeln!(dst, "#[doc = include_str!(r\"{}\")]", file.display())?;


this previously prevented these tests to be run on windows

Madoshakalaka · 2022-01-07T09:04:18Z

website/docs/concepts/components/children.mdx

+:::note Extra Braces Required
+`sidebar = {{html_nested!{...}}}` appears to require one extra set of braces.
+This is unintuitive, and we will fix the behavior in a later release.
+:::


add notes on the website for #2267

This help check the correctness of the combined website workflow too

Madoshakalaka · 2022-01-07T09:12:41Z

@voidpumpkin Why did the benchmark start for my PR anyways, when the PR doesn't have the label "performance", does this fail to filter?
https://github.com/yewstack/yew/actions/runs/1666840597/workflow#L3-L7

Madoshakalaka · 2022-01-07T09:18:27Z

I think everything works as expected, except the weird benchmark workflow behavior which @voidpumpkin is going to redo anyways. I'll sleep on this PR and see if anything needs to be changed tomorrow 🛏️

siku2

Very annoying to review like this so let's just run it and fix issues as they pop up.

siku2 · 2022-01-07T09:22:43Z

Oh I absolutely hate the idea of keeping the cargo-sweep binary in the repository. There has to be a way around that?

voidpumpkin

Looks goof for me but I will wait for @siku2 to review as he know more whats up with our ci.

website/docs/concepts/components/children.mdx

website/versioned_docs/version-0.19.0/concepts/components/children.mdx

Madoshakalaka · 2022-01-07T09:27:50Z

Oh I absolutely hate the idea of keeping the cargo-sweep binary in the repository. There has to be a way around that?

cargo-sweep doesn't release binaries, the release build is only 5Mb. hmm an alternative is to publish our own releases of ci-tools I guess? If that pollutes the repository too much I guess then we can have a separate ci-tools repository under yewstack

.github/workflows/build-cargo-sweep.yml

siku2

Actually noticed some bigger problems, so let's not merge this just yet :)

Generally very happy with the changes though!

.github/workflows/build-and-publish-website.yml

.github/workflows/build-cargo-sweep.yml

voidpumpkin · 2022-01-07T09:36:10Z

@voidpumpkin Why did the benchmark start for my PR anyways, when the PR doesn't have the label "performance", does this fail to filter? https://github.com/yewstack/yew/actions/runs/1666840597/workflow#L3-L7

Yes it fails to filter.
But it's not a problem because merging is not blocked even if its still running.

siku2 · 2022-01-07T09:37:37Z

cargo-sweep doesn't release binaries, the release build is only 5Mb. hmm an alternative is to have publish our own releases of ci-tools I guess? If that pollutes the repository too much I guess then we can have a separate ci-tools repository under yewstack

How about using the cache for this too? Makes it slightly more complicated to perform automatic updates but it's much less of a hassle than to set up a separate repository for it.

fixes yewstack#2351

Madoshakalaka · 2022-01-10T03:36:12Z

Should be ready now

ranile · 2022-01-10T17:21:17Z

Is cargo check for examples on PRs removed?
Also, we can we publish examples preview like we do for website?

Madoshakalaka · 2022-01-11T04:19:39Z

Is cargo check for examples on PRs removed?

Yes, there is a cargo clippy --all-targets at the workspace root and doesn't that cover what cargo check does?

Also, we can we publish examples preview like we do for website?

Good point, separate PR?

voidpumpkin · 2022-01-11T15:05:41Z

Is cargo check for examples on PRs removed?

Yes, there is a cargo clippy --all-targets at the workspace root and doesn't that cover what cargo check does?

Also, we can we publish examples preview like we do for website?

Good point, separate PR?

I agree it should be separate PR

voidpumpkin

I would approve, but @Madoshakalaka could you rebase?

voidpumpkin · 2022-01-11T15:08:10Z

.github/workflows/fmt.yml

+on:
+  pull_request:
+    paths:
+      - "**/*.rs"


Love this.
It inspired me to make actions that add labels that i have been adding manually. (They are needed for changelog generation)

…es and there can be #[doc(include("../some.md"))] in the code

# Conflicts: # Cargo.toml # website/docs/getting-started/build-a-sample-app.mdx

ranile · 2022-01-12T13:48:30Z

Please resolve the conflicts

# Conflicts: # .github/workflows/build-website.yml

voidpumpkin

oh wait why suspense extension became mdx in this pr?
(keeping in mind #2353 )

Madoshakalaka · 2022-01-18T09:28:44Z

oh wait why suspense extension became mdx in this pr? (keeping in mind #2353 )

kinda wanted to avoid contamination of PRs, I am well aware conflicts will need to be resolved for #2353 after this is merged and I have no problem with that :)

voidpumpkin · 2022-01-18T09:41:17Z

@hamza1311 i think we could merge this in

ranile

If anything goes wrong, it can be fixed later

ranile · 2022-01-18T09:43:42Z

Can't merge because of @siku2's review...

Madoshakalaka added 15 commits December 21, 2021 19:22

fix docs.rs logo url

748000d

fix formatting

f0bc0aa

bump docusaurus to beta14

ff29dcc

bring back the semicolon

b8a7196

cache npm for build-website workflow

39bb577

Add prebuilt cargo-sweep binary

ef19cc4

Optimize website CI

5f590a8

per-job caching, stale cache invalidation

5591ad8

Merge branch 'master' into ci-optimization

8e55e96

# Conflicts: # .github/workflows/publish-examples.yml # .github/workflows/pull-request.yml # website/package-lock.json

extract fmt job as workflow

c09cfca

move website test as a prerequisite for publish

fix typo in cargo-sweep executable path

9700bed

chmod +x

36755af

rename to main-checks.yml

a0700b6

--exclude has to be used with --workspace

c107369

revert accidental revert

e38f422

Madoshakalaka commented Jan 7, 2022

View reviewed changes

voidpumpkin added documentation meta Repository chores and tasks labels Jan 7, 2022

voidpumpkin requested a review from siku2 January 7, 2022 09:19

siku2 approved these changes Jan 7, 2022

View reviewed changes

voidpumpkin reviewed Jan 7, 2022

View reviewed changes

website/docs/concepts/components/children.mdx Outdated Show resolved Hide resolved

website/versioned_docs/version-0.19.0/concepts/components/children.mdx Outdated Show resolved Hide resolved

voidpumpkin reviewed Jan 7, 2022

View reviewed changes

.github/workflows/build-cargo-sweep.yml Outdated Show resolved Hide resolved

siku2 requested changes Jan 7, 2022

View reviewed changes

.github/workflows/build-and-publish-website.yml Outdated Show resolved Hide resolved

.github/workflows/build-cargo-sweep.yml Outdated Show resolved Hide resolved

replace actions-rs/install with wasm-pack-action

43c97f2

fixes yewstack#2351

Madoshakalaka marked this pull request as ready for review January 10, 2022 03:35

voidpumpkin requested changes Jan 11, 2022

View reviewed changes

Madoshakalaka added 3 commits January 11, 2022 20:47

shouldn't ignore .md because website snippet tests depends on .md fil…

bfea80a

…es and there can be #[doc(include("../some.md"))] in the code

Merge branch 'master' into ci-optimization

1b9a7b0

# Conflicts: # Cargo.toml # website/docs/getting-started/build-a-sample-app.mdx

Merge remote-tracking branch 'matt/ci-optimization' into ci-optimization

e60ba6b

Madoshakalaka requested a review from voidpumpkin January 12, 2022 03:50

Madoshakalaka added 2 commits January 11, 2022 21:17

restore accidentally deleted mdx files

c77cb78

include tools/website-test (for real this time)

7b7c166

voidpumpkin previously approved these changes Jan 12, 2022

View reviewed changes

voidpumpkin requested review from ranile and removed request for voidpumpkin January 12, 2022 08:25

Merge branch 'master' into ci-optimization

94a5619

# Conflicts: # .github/workflows/build-website.yml

Madoshakalaka dismissed voidpumpkin’s stale review via 94a5619 January 12, 2022 14:51

Madoshakalaka requested a review from voidpumpkin January 12, 2022 15:00

voidpumpkin approved these changes Jan 18, 2022

View reviewed changes

voidpumpkin requested changes Jan 18, 2022

View reviewed changes

voidpumpkin approved these changes Jan 18, 2022

View reviewed changes

ranile approved these changes Jan 18, 2022

View reviewed changes

siku2 merged commit 6669d18 into yewstack:master Jan 18, 2022

Madoshakalaka deleted the ci-optimization branch January 26, 2022 06:26

overlookmotel mentioned this pull request May 8, 2024

Speed up builds on CI with more caching oxc-project/oxc#3200

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize CI caching and path filtering #2340

Optimize CI caching and path filtering #2340

Madoshakalaka commented Jan 7, 2022 •

edited

Loading

Madoshakalaka Jan 7, 2022

Madoshakalaka Jan 7, 2022

Madoshakalaka commented Jan 7, 2022

Madoshakalaka commented Jan 7, 2022

siku2 left a comment

siku2 commented Jan 7, 2022

voidpumpkin left a comment

Madoshakalaka commented Jan 7, 2022 •

edited

Loading

siku2 left a comment

voidpumpkin commented Jan 7, 2022

siku2 commented Jan 7, 2022

Madoshakalaka commented Jan 10, 2022

ranile commented Jan 10, 2022

Madoshakalaka commented Jan 11, 2022

voidpumpkin commented Jan 11, 2022

voidpumpkin left a comment

voidpumpkin Jan 11, 2022

ranile commented Jan 12, 2022

voidpumpkin left a comment •

edited

Loading

Madoshakalaka commented Jan 18, 2022

voidpumpkin commented Jan 18, 2022

ranile left a comment

ranile commented Jan 18, 2022

		writeln!(dst, "#[doc = include_str!(\"{}\")]", file.display())?;

		writeln!(dst, "#[doc = include_str!(r\"{}\")]", file.display())?;

Optimize CI caching and path filtering #2340

Optimize CI caching and path filtering #2340

Conversation

Madoshakalaka commented Jan 7, 2022 • edited Loading

Per-job Caching

stale cache invalidation

Path Filtering Workflows

Remove check-examples job in the pull-request workflow

Extract cargo fmt job as workflow

Renaming

un-exclude tool/website-test

Madoshakalaka Jan 7, 2022

Choose a reason for hiding this comment

Madoshakalaka Jan 7, 2022

Choose a reason for hiding this comment

Madoshakalaka commented Jan 7, 2022

Madoshakalaka commented Jan 7, 2022

siku2 left a comment

Choose a reason for hiding this comment

siku2 commented Jan 7, 2022

voidpumpkin left a comment

Choose a reason for hiding this comment

Madoshakalaka commented Jan 7, 2022 • edited Loading

siku2 left a comment

Choose a reason for hiding this comment

voidpumpkin commented Jan 7, 2022

siku2 commented Jan 7, 2022

Madoshakalaka commented Jan 10, 2022

ranile commented Jan 10, 2022

Madoshakalaka commented Jan 11, 2022

voidpumpkin commented Jan 11, 2022

voidpumpkin left a comment

Choose a reason for hiding this comment

voidpumpkin Jan 11, 2022

Choose a reason for hiding this comment

ranile commented Jan 12, 2022

voidpumpkin left a comment • edited Loading

Choose a reason for hiding this comment

Madoshakalaka commented Jan 18, 2022

voidpumpkin commented Jan 18, 2022

ranile left a comment

Choose a reason for hiding this comment

ranile commented Jan 18, 2022

Madoshakalaka commented Jan 7, 2022 •

edited

Loading

un-exclude `tool/website-test`

Madoshakalaka commented Jan 7, 2022 •

edited

Loading

voidpumpkin left a comment •

edited

Loading