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

feat(brew): use sudo if Homebrew owned by another user on Linux #904

Merged
merged 1 commit into from
Sep 22, 2024
Merged

feat(brew): use sudo if Homebrew owned by another user on Linux #904

merged 1 commit into from
Sep 22, 2024

Conversation

tomaszn
Copy link
Contributor

@tomaszn tomaszn commented Sep 11, 2024

NOTE: This is my first contact with Rust, and the implementation is surely suboptimal.

What does this PR do

Currently, on Linux, if the Homebrew installation directory is owned by a different user, the step fails.

Homebrew directory, which on Linux is /home/linuxbrew/.linuxbrew/, is typically owned by the user who installed Homebrew, or a dedicated user account.

This change ensures that Homebrew updates can proceed smoothly even when its directory ownership does not match the current user's UID. It accomplishes this by running the update command with sudo, changing the UID to the proper owner account. Appropriate sudo configuration is assumed for this to work properly, for example:

%wheel  ALL=(linuxbrew) CWD=/tmp NOPASSWD: /home/linuxbrew/.linuxbrew/Homebrew/bin/brew

If this feature is acceptable, I am happy to work on the implementation further. Probably, the existing Sudo class would need extending to support providing the UID, to be reused when constructing the command. Any constructive criticism is highly welcome.

Standards checklist

  • The PR title is descriptive.
  • I have read CONTRIBUTING.md
  • Optional: I have tested the code myself

@SteveLauC
Copy link
Member

Hi, have you checked this answer of question How to use Homebrew on a Multi-user MacOS Sierra Setup, looks like the officially suggested way to share Homebrew with multiple users is to install the core globally and then install a clone locally, so that sudo can be avoided🤔

@tomaszn
Copy link
Contributor Author

tomaszn commented Sep 16, 2024

Hi, thanks for joining me here, and pointing out an alternative approach.

Unfortunately, it uses nonstandard "prefix" directories, which causes recompilation of the packages in the best case, and failure to install in the worst case. There is a comment about such issues: https://stackoverflow.com/questions/41840479/how-to-use-homebrew-on-a-multi-user-macos-sierra-setup/55021458#comment133690270_55021458

This is also mentioned in the FAQ: https://docs.brew.sh/FAQ#why-should-i-install-homebrew-in-the-default-location

When it comes to using sudo, there is an FAQ entry: https://docs.brew.sh/FAQ#why-does-homebrew-say-sudo-is-bad

It implicitly refers to running Homebrew as root, not to any sudo usage. The last sentence suggests creating a dedicated role account and using sudo to switch. Or maybe my interpretation is wrong...

@SteveLauC
Copy link
Member

Hi, thanks for the detailed explanation! So IIUC, sharing the same insatllaction of Homebrew with multiple users is basically unsupported and not suggested?

@tomaszn
Copy link
Contributor Author

tomaszn commented Sep 16, 2024

Yes, that's what I understand.
I set up Homebrew on Linux with a separate "linuxbrew" account, and with this sudo configuration snippet, brew seems to work properly for several accounts.

@SteveLauC SteveLauC changed the title feat(brew): use sudo if Homebrew owned by another user feat(brew): use sudo if Homebrew owned by another user on Linux Sep 22, 2024
@SteveLauC
Copy link
Member

Get it, the idea makes sense to me.

@SteveLauC
Copy link
Member

If this feature is acceptable, I am happy to work on the implementation further. Probably, the existing Sudo class would need extending to support providing the UID, to be reused when constructing the command.

The current impl looks generally good, so I think there is currently no need to do this. But we can do this when we have more steps that need this feature.


Have you tried if Topgrade's --dry-run option works with this new impl?

@tomaszn
Copy link
Contributor Author

tomaszn commented Sep 22, 2024

Thank you for reviewing the changes.
I've implemented all the recommended updates. Please take a look at your convenience.

The dry run appears to be working successfully:

── 13:53:19 - Brew (sudo as user 'linuxbrew') ──────────────────────────────────────
Dry running: /usr/bin/sudo --set-home '--user=linuxbrew' /home/linuxbrew/.linuxbrew/Homebrew/bin/brew update in /tmp

On Linux, run "brew update" with sudo if the Homebrew installation directory
is owned by a different user. This is typically the user who installed
Homebrew, but can also be a dedicated user account. This change ensures that
Homebrew updates can proceed smoothly even when its directory ownership does
not match the current user's UID. Proper sudo configuration is assumed for
this to work properly.
Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the patch!

@SteveLauC SteveLauC merged commit 27245cb into topgrade-rs:main Sep 22, 2024
11 checks passed
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.

2 participants