-
-
Notifications
You must be signed in to change notification settings - Fork 430
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
Add Homebrew Bundle module #262
Conversation
f48698e
to
92ccc0d
Compare
Alrighty, this seems like it's in a good state for initial review. I'll add some review comments with thoughts/questions I have. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my review comments for specific thoughts/questions.
I expect we'll also want test for this module. However, I'm not sure what they should be since it's my understanding that activation scripts aren't run when running tests, and most of the things I'd want to test are that things were successfully installed etc.
If there are suggestions for what should be tested. I'm happy write the test up.
modules/programs/brew-bundle.nix
Outdated
''; | ||
}; | ||
|
||
whalebrews = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should note that I actually haven't tested Homebrew Bundles whalebrew
integration, since I'm not currently using Docker on my macOS machine. I've based the implementation of this option on the Homebrew Bundle, and whalebrew
documentation.
8cfcb91
to
785130a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not used homebrew on my system for quite some time so I don't really have much feedback here. I think it's best that the people that will actually use it give their suggestions, looks good to me in general if everybody is happy with it.
This is a great answer to my comment, thank you. I realized much of this (especially the tying to |
Co-authored-by: Scott Day <[email protected]>
@LnL7, just want to flag up front that I don't feel confident I understand exactly what you were getting at. Apologies if my responses below aren't on target. As far as I can tell, Homebrew has no plans to use
There could be some value in having access to the lockfile for the last successful invocation of From
That poses a couple challenges:
This is kind of a bummer since, if
It does seem to me that setting For the former case, I do think that having these variables set leads to a more intuitive experience, and that the illustrative examples are representative of a broader class of use-cases that I expect to be fairly common. I also agree that with these variables set globally, it may be confusing/cumbersome for folks to use a Brewfile provided with a project (e.g., they'd have to run Given my understanding of your concerns, here are what seem to me like the best ways to move forward:
I'd prefer the latter, but would settle for the former in order to get this merged. |
@LnL7, I've gone ahead and implemented the latter of the two paths forward I listed with the options disabled by default (to make things quicker if you're happy with it). Of course, if you'd really prefer the other option, or I completely misunderstood your concerns, just let me know, and I'll make the required changes. |
I would love for this to get merged. I've recently needed to install a homebrew package and I would love a nix/declarative way to do it such as this. |
If you use niv or flakes to pin dependencies, you can point it to the version from this fork until it gets merged. |
I don't use flakes yet, that's on my TODO list :) |
In case it's helpful, my Nix configs repo is a flake and it has and output |
Mostly just wanted to make sure I got the idea across. I leave the decision up to the people that will use this. |
I'm happy with this implementation - I think this looks ready to merge. |
I agree, I've forked this and changed my flake input to my fork. Everything's working as expected. One thing I think could improve is if homebrew isn't installed it should install it automatically. |
We had a discussion about this and ultimately decided that's not a good idea for reasons cited here: #262 (comment). If memory serves, this will print a warning and do nothing if |
Yeah, during activation a (non-fatal) error message gets printed if Hombrew isn't installed: nix-darwin/modules/homebrew.nix Lines 206 to 211 in 2a0b9a9
While I do agree it would be nice for this module to install Homebrew, this module is already pushing the boundries of the usual Nix zen, and given that I expect that installation of Homebrew would need to happen during activation, that would push things even further. Regardless, I think we should move forward with merging this initial version of the module, and leave potential additions like adding Homebrew installation to future discussions/PRs. |
Okay cool yeah, I must've missed that comment. |
👋 Wanted to provide feedback that it is working as intended, thank you! 😄 For now I am setting my inputs as follows:
One less thing to manage via shell scripts 🙇 It is not deterministic due to the lack of ability to pin versions but I think it is a good compromise for MacOS users. Edit: I found out a (good?) quirk where I can include different files with different sets of |
This is expected behavior and is not actually unique to this module! Nix merges lists and attribute sets if they're defined in more than one location. This is helpful to design modular configurations :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There hasn't really been an new feedback so merging.
Nix Darwin already has in `master` Homebrew (LnL7/nix-darwin#262), therefore there is no need to include malobs's dotfiles as flake input.
Are there any developments on this front? I would really like nix to transparently use homebrew instead of me, as a user, installing it beforehand. |
I'm not aware of any work on this front. I'd be happy to give feedback on a PR that implemented Homebrew installation (though it's not my call on whether it gets merged). |
Overview and motivation
This module adds the ability for
nix-darwin
to manage packages/apps installed by Homebrew Bundle. This givesnix-darwin
users the ability to not only manage packages/apps installed via Homebrew, but also apps from the Mac App Store viamas
, and Docker containers viawhalebrew
.While I wish I could manage installing all my packages/apps directly through Nix via
nix-darwin
there are a sizable number of cases where that currently just isn't feasible at present. I expect many other users are in the same boat. I think this module, or something like it, provides a good compromise, by allowing users to move more of their macOS system configuration into Nix while leveraging Homebrew Bundle under the hood.Here's an example of this module in use in my current config:
https://github.com/malob/nixpkgs/blob/master/darwin/homebrew.nix
It's been working quite well in my testing so far, both on my system, and my GitHub workflow.
Suggestions for changes/improvements are of course welcome.
Implementation details
The options,
homebrew.taps
,homebrew.brews
,homebrew.casks
,homebrew.masApps
,homebrew.whalebrews
,are used to generate a Brewfile in the Nix store which defines which packages/apps Homebrew Bundle will install/upgrade. (The
homebrew.extraConfig
option is also provided to allow users to include more complex Brewfile entries.)The activation script runs
brew bundle
using the generated Brewfile, with theHOMEBREW_NO_AUTO_UPDATE
environment variable set to ensure idempotence. Thehomebrew.autoUpdate
option is provided for users who want to allow Homebrew to auto-update during activation.The
homebrew.cleanup
option is provided to allow the user to enable Homebrew Bundle uninstall packages/apps that are currently installed but aren't present in the generated Brewfile. I made the default"none"
so that new users of this module would be less likely to uninstall packages/apps accidentally.More detailed documentation is provided in the module.
(Edit * 3: Updated to reflect changes since creating PR)