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

Run cachix workflow for x86_64-darwin #4710

Closed

Conversation

mtoohey31
Copy link
Contributor

This merge request updates the Cachix workflow so it also runs on macOS. This will allow Nix users to avoid having to build Helix themselves on x86_64-darwin when they have configured the Helix Cachix as a binary cache.

I have not tested these changes. I don't have a Cachix cache set up myself, and I don't have permissions for the Helix one. I am as confident as I can be without having tested them that these changes are correct, since they're copied directly from cachix-action's test workflow, but someone else should definitely also take a careful look at them before this gets merged.

This partially addresses my comment on #1720.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

My only hesitancy is that we only have 5GB of storage and each build takes a decent amount of space (although recent nix-cargo-integration changes have improved this). I suppose we could try it though and see if it ends up being too expensive to have multiple builds for the same cache

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-packaging Area: Packaging and bundling S-waiting-on-review Status: Awaiting review from a maintainer. labels Nov 11, 2022
@mtoohey31
Copy link
Contributor Author

Ah, I didn't realize there was that limitation. Is there any chance we could cut down on what's being cached without loosing most of the benefits? In the output from this run I see rust-docs, rustc, rust-std, and a couple other things contributing quite a lot of size. Is caching those necessary? It looks like cachix-action supports includes/excludes.

I was hoping to get aarch64-linux cached too, but I guess that's probably pushing it if this is already borderline.

@the-mikedavis
Copy link
Member

Oh hmm yeah, trimming those out would save a good amount of space. I would imagine that those are served by the nixpkgs cache so I'm surprised to see them uploaded to the cache (unless they're created by nix-cargo-integration). Excluding them from the cache would be good 👍

@mtoohey31
Copy link
Contributor Author

Ok, I've pushed another commit that attempts to do that, but I'm much less confident about that working than the prior commit. Is there any way we can test it without merging it into main and breaking things?

@the-mikedavis
Copy link
Member

It would be cool to do something like https://github.com/helix-editor/helix/pull/3757/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R9-R11 for this cachix file but I think it wouldn't work well because secrets (like the cachix publish key) are not given to forks.

You can set up a 5GB cache with your github account through cachix (needs a sign-up but is otherwise free) and then merge this to your fork. I would do that to test this out but the Helix cachix is tied to my account and the 5GB is account-wide 😅

@mtoohey31
Copy link
Contributor Author

Ok, I'll try and give that a go sometime soon, I'm going to mark this as a draft in the meantime though.

@mtoohey31 mtoohey31 marked this pull request as draft November 12, 2022 21:33
@mtoohey31
Copy link
Contributor Author

It turns out my assumption here: #4710 (comment) was incorrect. The rust toolchain is created by nix-cargo-integration, since it's based on the rust-toolchain.toml file, meaning it's not cached in the nixpkgs cache. So excluding the toolchain from the cache isn't a good option.

I'm going to close this for now, because I haven't had any other good ideas on how to reduce the size of the things being cached.

@mtoohey31 mtoohey31 closed this Jan 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-packaging Area: Packaging and bundling C-enhancement Category: Improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants