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.

Sign up for GitHub

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

Jump to bottom

chore(ci): Avoid writing to cache in workflows triggered by the merge queue #2341

Merged
merged 19 commits into from
Aug 16, 2023
Merged

chore(ci): Avoid writing to cache in workflows triggered by the merge queue #2341

merged 19 commits into from
Aug 16, 2023

Conversation

TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Aug 16, 2023 •
edited
Loading

Description

Problem*

Resolves #2274

Summary*

This PR wraps our usage of the actions/cache action with an action which choses between this and actions/cache/restore based on a boolean input. We can then perform a read-only cache restore if the workflow was triggered by the merge_group trigger.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench requested a review from phated August 16, 2023 12:55
@TomAFrench TomAFrench marked this pull request as ready for review August 16, 2023 13:59
phated
phated reviewed Aug 16, 2023
View reviewed changes
.github/workflows/test.yml Outdated Show resolved Hide resolved
phated
phated suggested changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

I don't think env vars are expanded in the inputs section without the ${{ }} syntax

.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/wasm.yml Outdated Show resolved Hide resolved
.github/workflows/wasm.yml Outdated Show resolved Hide resolved
.github/workflows/wasm.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@TomAFrench TomAFrench changed the title chore(ci): use custom caching action to prevent cache writes in merge queue chore(ci): Avoid writing to cache in workflows triggered by the merge queue Aug 16, 2023
@TomAFrench TomAFrench requested a review from phated August 16, 2023 18:47
phated
phated suggested changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

One nit, and since this is closing the "revisit caching" issue, can you add nix caching to the build-wasm job inside wasm.yml? That is the longest step in the CI process because it doesn't have a cache for the cargo deps.

.github/workflows/publish.yml Outdated Show resolved Hide resolved
@TomAFrench
Copy link
Member Author

can you add nix caching to the build-wasm job inside wasm.yml?

I was going to handle this in a follow-up PR to maintain the scope of this one but ok.

@TomAFrench TomAFrench requested a review from phated August 16, 2023 21:12
phated
phated approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

Thanks for this! One thing I'm still noticing is that we build wasm-bindgen-cli from source each time which takes a majority of the 7min CI. We can fix that later though

@TomAFrench TomAFrench added this pull request to the merge queue Aug 16, 2023
Merged via the queue into master with commit b6c36aa Aug 16, 2023
15 checks passed
@TomAFrench TomAFrench deleted the merge-queue-readonly-caches branch August 16, 2023 22:03
TomAFrench added a commit that referenced this pull request Aug 16, 2023
@TomAFrench
Merge branch 'master' into assert-eq-builtin
22a99c2 
* master:
  chore: update noir-source-resolver from `1.1.2` to `^1.1.3` (#2349)
  chore(ci): Avoid writing to cache in workflows triggered by the merge queue (#2341)
  chore(noir): Release 0.10.3 (#2344)
  feat(lsp): Add `Execute` code lens for `main` functions (#2330)
  feat(lsp): Add `Compile` code lens for `main` function and contracts (#2309)
  feat: Allow calling higher-order functions with closures (#2335)
TomAFrench added a commit that referenced this pull request Aug 17, 2023
@TomAFrench
Merge branch 'master' into abi-wasm
c970b4f 
* master: (25 commits)
  chore: update noir-source-resolver from `1.1.2` to `^1.1.3` (#2349)
  chore(ci): Avoid writing to cache in workflows triggered by the merge queue (#2341)
  chore(noir): Release 0.10.3 (#2344)
  feat(lsp): Add `Execute` code lens for `main` functions (#2330)
  feat(lsp): Add `Compile` code lens for `main` function and contracts (#2309)
  feat: Allow calling higher-order functions with closures (#2335)
  fix: Display warning if last expression of block is unused (#2314)
  chore(noir): Release 0.10.2 (#2343)
  fix: Prevent dead instruction elimination of brillig functions which may contain side-effects (#2340)
  chore: Separate integration tests for contracts and programs (#2339)
  chore: move orphaned integration tests to new directory (#2331)
  chore(noir): Release 0.10.1 (#2328)
  feat(ssa): Switch mem2reg pass to be per function rather than per block (#2243)
  feat(ssa): Perform dead instruction elimination on intrinsic functions (#2276)
  feat: Add full call stacks to runtime errors (#2310)
  chore(ci): fix mismatched input name to publish workflow (#2327)
  chore: add README for integration test structure (#2277)
  feat: Improved error message for unexpected return type (#2302)
  feat(stdlib): Implement `str` `as_bytes` and `into_bytes` function (#2298)
  chore(ci): automatically convert changelog entries to sentence case (#2325)
  ...
TomAFrench added a commit that referenced this pull request Aug 22, 2023
@TomAFrench
Merge branch 'master' into remove-keys-from-artifact
69b0058 
* master: (46 commits)
  chore: Remove `serde` from `noirc_frontend` (#2390)
  chore: allow parenthesizing in two type locations  (#2388)
  chore(ci): automatically delete cache entries associated with closed PRs (#2342)
  feat: Perform more checks for compile-time arithmetic (#2380)
  chore: Remove `noirc_abi::FunctionSignature` and define in terms of HIR (#2372)
  feat: Update to `acvm` 0.22.0 (#2363)
  chore: Update committed ACIR artifacts (#2376)
  feat(ssa): Merge slices in if statements with witness conditions (#2347)
  chore: Separate frontend `Visibility` and `Distinctness` from the ABI (#2369)
  feat: add syntax for specifying function type environments (#2357)
  chore: Remove unused `Directive::Log` (#2366)
  chore: clippy fixes (#2365)
  chore: Extract bytecode from artifact files for backend integration test inputs (#2356)
  feat: Add trait definition representation in DefCollector and HIR (#2338)
  chore: Remove unused `Intrinsic::Println` (#2358)
  fix: Remove duplicte `T` in `expected T, found T` error on tuple assignment (#2360)
  chore(brillig): Fix brillig radix instruction return vector size (#2350)
  fix: Show types in error message in same order as in source code (#2353)
  chore: update noir-source-resolver from `1.1.2` to `^1.1.3` (#2349)
  chore(ci): Avoid writing to cache in workflows triggered by the merge queue (#2341)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phated phated phated approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI should avoid creating cache entries for merge queue branches
2 participants
@TomAFrench @phated