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

chore(nix): Fix dependency caching #1421

Merged
merged 5 commits into from
May 30, 2023
Merged

chore(nix): Fix dependency caching #1421

merged 5 commits into from
May 30, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented May 26, 2023

Description

Problem*

Resolves #1303

Summary*

This updates the flake to enable better caching. When building the dependencies, we were injecting the GIT_COMMIT and GIT_DIRTY variables, which made the dependencies rebuild from scratch on each commit. It also reworks the flake to have better delineation between native backend and wamer backend build targets (this will avoid needing the bb.wasm file during checks).

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.

.github/workflows/test.yml Outdated Show resolved Hide resolved
kevaundray
kevaundray previously approved these changes May 26, 2023
@kevaundray kevaundray enabled auto-merge May 26, 2023 22:29
@phated phated disabled auto-merge May 26, 2023 22:34
@phated
Copy link
Contributor Author

phated commented May 26, 2023

I should have switched this to a draft, as I'm still trying to look at the cache results

@phated phated marked this pull request as draft May 26, 2023 22:34
@phated phated force-pushed the phated/nix-caching branch from 23a156a to 3139ca1 Compare May 26, 2023 22:34
@kevaundray
Copy link
Contributor

I should have switched this to a draft, as I'm still trying to look at the cache results

You mean the performance difference?

@phated
Copy link
Contributor Author

phated commented May 26, 2023

You mean the performance difference?

Making sure I'm seeing the same results in CI as I saw locally. There's a subtle difference in how the nix store is restored in CI so I wanted to make sure this is working.

@phated
Copy link
Contributor Author

phated commented May 26, 2023

Looks like it is working the same. Running nix flake check on ubuntu finishes in 10 minutes with 8 minutes 36 seconds spent in the tests.

@phated phated marked this pull request as ready for review May 26, 2023 22:59
@phated phated added this pull request to the merge queue May 30, 2023
Merged via the queue into master with commit 9081ec9 May 30, 2023
@phated phated deleted the phated/nix-caching branch May 30, 2023 18:14
kevaundray added a commit that referenced this pull request Jun 1, 2023
kevaundray added a commit that referenced this pull request Jun 1, 2023
kevaundray added a commit that referenced this pull request Jun 1, 2023
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 should be caching cargo dependencies
2 participants