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

feature: try to add nix-darwin support #141

Merged
merged 12 commits into from
Jan 31, 2023
Merged

Conversation

n8henrie
Copy link
Collaborator

Merges work by @montchr, @cmhamill, and @rtimush and rebases on main.

- fixes ryantm#60
- fixes ryantm#120
- closes ryantm#107
@ryantm
Copy link
Owner

ryantm commented Jan 30, 2023

Yay passing tests! I don't have a system to try this on. If you do, please chime in if it works for you.

@ryantm ryantm changed the title Try to add nix-darwin support to agenix feature: try to add nix-darwin support Jan 30, 2023
ryantm
ryantm previously approved these changes Jan 30, 2023
Copy link
Owner

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Code overall looks good to me and I'm basically ready to merge. I tested the integration test fails if I expect a different decrypted file contents.

It would be nice to get one more nix-darwin user to sanity check this before merging, but if we don't get one by the end of the week lets :shipit:

modules/age.nix Show resolved Hide resolved
@rtimush
Copy link

rtimush commented Jan 30, 2023

I tested this PR and it all worked for me 👍
The only minor thing was that this output was a bit too verbose during darwin-rebuild switch:
image

@n8henrie
Copy link
Collaborator Author

I'll capture that output, good point.

Will do a little more testing this morning.

@ryantm
Copy link
Owner

ryantm commented Jan 30, 2023

@n8henrie
Copy link
Collaborator Author

Few minor changes incoming.

@n8henrie
Copy link
Collaborator Author

n8henrie commented Jan 30, 2023

@ryantm What would you think about using apfs instead of hfs here? APFS is the Apple-preferred filesystem these days, should be compatible with MacOS 10.13+ (released ~2017).

EDIT: For example the nix store is APFS on my M1 Mac (is it hfs on older macs)?

@ryantm
Copy link
Owner

ryantm commented Jan 30, 2023

@n8henrie I know almost nothing about MacOS file systems. I haven't used a Mac for over 16 years. Maybe you can ping the other people that were trying to get nix-darwin support?

@n8henrie
Copy link
Collaborator Author

Looks like using mount_apfs won't let us use the mask flag (-m=0751) that we're currently using on hfs. I'm not sure how important this is, so I just left it as hfs for now.

@n8henrie
Copy link
Collaborator Author

n8henrie commented Jan 30, 2023

Integration tests are now failing, I think due to removal of the activation scripts.

EDIT: Nope, passing now and without the activation scripts.

@n8henrie
Copy link
Collaborator Author

Gah, older macos doesn't like the diskutil, which had a more readable size for the ramdisk. Fix incoming.

@n8henrie
Copy link
Collaborator Author

Ok, I think this is ready to merge (would be great if 1 more nix-darwin user can pull and test). Tests passing, working on reboot with my system.

@ryantm
Copy link
Owner

ryantm commented Jan 31, 2023

@rtimush up for another try?

@rtimush
Copy link

rtimush commented Jan 31, 2023

@rtimush up for another try?

Tried and everything worked fine for me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants