blue-build / modules

BlueBuild standard modules used for building your Atomic Images
Apache License 2.0
24 stars 0 forks source link

feat: Add Brew module #239

Closed mecattaf closed 4 months ago

mecattaf commented 4 months ago

Summary

This pull request adds a new Brew module to the BlueBuild project. The Brew module installs Homebrew (Brew) on the system, installs specified Brew packages, and configures systemd services to maintain the packages.

Features

Configuration Options

Example Configuration

Here is an example configuration for the Brew module in recipe.yml:


---

type: brew

# Installs Brew (Homebrew) to /home/linuxbrew/.linuxbrew via external download

install_brew: true # Optional - Default: true

# List of Brew packages to be installed

packages:

  - ollama # Required

# Interval between Brew updates

update_interval: '6h'  # Optional - Default: '6h'

# Interval between Brew package upgrades

upgrade_interval: '8h' # Optional - Default: '8h'

# Time delay after boot before first Brew update

wait_after_boot_update: '10min' # Optional - Default: '10min'

# Time delay after boot before first Brew upgrade

wait_after_boot_upgrade: '30min' # Optional - Default: '30min'

# If true, disables automatic activation of `brew-update.timer`.

disable_update: false # Optional - Default: false

# If true, disables automatic activation of `brew-upgrade.timer`.

disable_upgrade: false # Optional - Default: false

Motivation

Homebrew provides a vast collection of CLI applications that might not be available in Fedora repositories. This module leverages Homebrew to provide easy access to these applications.

Additional Notes

This module follows the recommendations of the Brew maintainers, specifically targeting a single user (UID 1000) to run Brew commands. The module sets up systemd timers to ensure Brew packages remain updated and upgraded over time.

Context

Context for this module's implementation was based on previous discussions and guidance provided by @m2Giles and other BlueBuild maintainers regarding the installation and management of Brew.

Related Issue: #230

Interpretation of Bluefin Brew PR Files ### Systemd Services and Timers - **brew-setup.service**: - Sets up Brew after installation. - **brew-update.timer**: - Runs periodically to update Brew. - **brew-upgrade.timer**: - Runs periodically to upgrade Brew packages. - **brew-update.service**: - Service unit executed by the `brew-update.timer`. - **brew-upgrade.service**: - Service unit executed by the `brew-upgrade.timer`. ### Helper Scripts - **brew.sh**: - Shell script to download and install Brew. ### Configuration for User Shells - **brew.fish**: - Fish shell configuration for Brew. - **brew-bash-completion.sh**: - Bash shell completion configuration for Brew. ### tmpfiles.d Configuration - **homebrew.conf**: - Configuration file to set up necessary directories for Brew. ### Mounts Configuration - **var-home-linuxbrew.mount**: - Mounts for overlay filesystem to support Brew.

Looking forward to your feedback!

m2Giles commented 4 months ago

The var-home-linuxbrew.mount is vestigial. Recommend using tmpfiles.d instead for creating the copy to point.

Ultimately on first boot you want the contents of /usr/share/homebrew to end up at /var/home/linuxbrew.

If you go the mount route, you will need an overlayfs since /usr is read only. Eventually everything will be pulled up to the upper dir so you are not really saving space. It's also being stored in /usr compressed as well.

I'm also unsure why you would have a boolean for installing or not. Should be if they want this or not. Every service file is dependent on brew being present.

xynydev commented 4 months ago

The var-home-linuxbrew.mount is vestigial. Recommend using tmpfiles.d instead for creating the copy to point.

Ultimately on first boot you want the contents of /usr/share/homebrew to end up at /var/home/linuxbrew.

If you go the mount route, you will need an overlayfs since /usr is read only. Eventually everything will be pulled up to the upper dir so you are not really saving space. It's also being stored in /usr compressed as well.

+1, I had the exact same thoughts, but didn't know how to comment yet, without seeing the service units that haven't been pushed to the PR yet.

Has this module been tested yet? I don't expect brew install to work with the current way things work.

fiftydinar commented 4 months ago

Thanks for the contribution!

I currently don't have anything to add, since xynydev & gmpinder said everything that I see as flaws that have to be fixed.

m2giles also added a good point about using mount vs tmpfiles.d.

mecattaf commented 4 months ago

Thanks for all the feedback. I think the updates I just pushed should cover everything.

fiftydinar commented 4 months ago

After all of this fixing, this looks good to me!

The only thing that remains is to test this in real-life, to check if everything is working correctly.

fiftydinar commented 4 months ago

Brew install fails, because execution script detects that it's ran as root:

0.091 ================================================= Start 'brew' Module =================================================
0.126 Downloading and installing Brew...
0.153 Warning: Running in non-interactive mode because `stdin` is not a TTY.
0.174 ==> Checking for `sudo` access (which may request your password)...
0.175 Don't run this as root!

I tested build with type: brew added.

Build has this PR commit as the latest: 91eaad0 (#239)

Failed build: https://github.com/fiftydinar/gidro-os/actions/runs/9272606213/job/25510790662#step:2:2331

fiftydinar commented 4 months ago

I think these ones are responsible to trick brew that build is not running as root:

https://github.com/ublue-os/bluefin/blob/main/build_files/base/brew.sh#L5C1-L6C18

https://github.com/ublue-os/bluefin/blob/main/scripts/sudoif.sh

mecattaf commented 4 months ago

In brew.sh

Mind trying now?

fiftydinar commented 4 months ago

@mecattaf Installation passes successfully now in logs (type: brew used only).

Logs are very noisy, but maybe necessary? https://github.com/fiftydinar/gidro-os/actions/runs/9273300849/job/25513073884?pr=26#step:2:2146

I will test the image in a VM.

mecattaf commented 4 months ago

I will defer to more experienced contributors to keep improving the PR, but as I understand TODO:

Thanks

fiftydinar commented 4 months ago

I will defer to more experienced contributors to keep improving the PR, but as I understand TODO:

* [ ]  Remove verbose logging from brew.sh

* [ ]  Resolve installing brew packages at build time

* [ ]  Decide approach re: nofile limit and brew analytics

Thanks

That's right.

I think that this can be added in PR post 1st, titled like this:

To do

fiftydinar commented 4 months ago

EDIT: Solved

NOTE to add in README: After excluding type: brew from the module after it's installed in the system, some remaining folders will remain in the running system:

Created by brew

Created by tmpfiles.d

This would potentially solve the problem, but it must be executed manually by the user or set as automatic systemd service afterwards by image-maintaner:

/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/uninstall.sh)"
if [[ -d "/var/lib/homebrew" ]]; then
  sudo rm -r /var/lib/homebrew
fi
if [[ -d "/var/cache/homebrew" ]]; then
  sudo rm -r /var/cache/homebrew
fi
if [[ -d "/var/home/linuxbrew" ]]; then
  sudo rm -r /var/home/linuxbrew
fi
xynydev commented 4 months ago

This is starting to look pretty good. I'd like the nofile-limits explanation be expanded before merge, though. One of y'all would probably qualify better to do that.

fiftydinar commented 4 months ago

@xynydev Can you check the documentation now for nofile-limits?

It maybe can be better reworded, but I think it explains things a bit better.

xynydev commented 4 months ago

It looks great! I'll have one final read of the docs before approving.

xynydev commented 4 months ago

@fiftydinar since you've made change reqs we need your manual approval to merge.

xynydev commented 4 months ago

Ah, @gmpinder requested changes too. I don't think he's awake yet so I guess I'll just force-merge.