Skip to content

Don't require/recommend ownership of /usr/local.#896

Merged
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
MikeMcQuaid:no-chown-usr-local
Sep 9, 2016
Merged

Don't require/recommend ownership of /usr/local.#896
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
MikeMcQuaid:no-chown-usr-local

Conversation

@MikeMcQuaid
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid commented Sep 8, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

Apple reset this on every OS X major (and some minor) updates and it always proves a painful and unnecessary step. Instead just check the directories we actually care about are writable.

This may mean if these directories do not already exist (although they are now created by the installed) that brew link will fail and require manual intervention but this seems to be superior for both new and the majority of existing users.

@BrewTestBot BrewTestBot added the in progress Maintainers are working on this label Sep 8, 2016
@DomT4
Copy link
Copy Markdown
Contributor

DomT4 commented Sep 8, 2016

Are we suggesting anything for the Caskroom, which lives in HOMEBREW_PREFIX by default now IIRC? Is there any situation where opt wouldn't be writable & that might need fixing?

Other than those two questions, seems reasonable 👍.

@MikeMcQuaid
Copy link
Copy Markdown
Member Author

MikeMcQuaid commented Sep 8, 2016

@DomT4 Caskroom is in HOMEBREW_REPOSITORY actually (which feels a little weird but is nice to avoid another root directory). Good call about opt, will check.

@tdsmith tdsmith mentioned this pull request Sep 8, 2016
Merged
@zmwangx
Copy link
Copy Markdown
Contributor

zmwangx commented Sep 9, 2016 •
edited
Loading

👍 on the change. Not directly related to this PR, but what about adding a warning on what chown -R actually does? Something like

Note that chown -R changes the ownership of all files and subdirectories, which could be slow and lead to unintended side effects if you have files and subdirectories that are not managed by Homebrew.

The reason is that chown -R could be very slow and doesn't print anything, so the user could decide that it's stuck and abort halfway, screwing up everything (unless running chown -R for a second time and letting it finish). Also, if there are software not managed by Homebrew installed to /usr/local, some could stop working correctly if you change the owner.

@MikeMcQuaid
Copy link
Copy Markdown
Member Author

MikeMcQuaid commented Sep 9, 2016

@zmwangx In my experience the longer you make these type of messages the less likely they are to be read. As we don't run them for users and they are recommendations I'm 🆒 with just trusting users .

@zmwangx
Copy link
Copy Markdown
Contributor

zmwangx commented Sep 9, 2016

In my experience the longer you make these type of messages the less likely they are to be read.

True 😓

@MikeMcQuaid
Don't require/recommend ownership of /usr/local.
492391f 
Apple reset this on every OS X major (and some minor) updates and it
always proves a painful and unnecessary step. Instead just check the
directories we actually care about are writable.

This may mean if these directories do not already exist (although they
are now created by the installed) that `brew link` will fail and require
manual intervention but this seems to be superior for both new and the
majority of existing users.
@MikeMcQuaid MikeMcQuaid merged commit b5a6b4e into Homebrew:master Sep 9, 2016
@MikeMcQuaid MikeMcQuaid deleted the no-chown-usr-local branch September 9, 2016 08:29
@BrewTestBot BrewTestBot removed the in progress Maintainers are working on this label Sep 9, 2016
@ilovezfs
Copy link
Copy Markdown
Contributor

ilovezfs commented Sep 9, 2016

@MikeMcQuaid looks like this broke CI.

https://bot.brew.sh/job/Homebrew%20Core%20Pull%20Requests/7460/version=yosemite/console

for everything ...

@ilovezfs ilovezfs mentioned this pull request Sep 9, 2016
Closed
@MikeMcQuaid MikeMcQuaid mentioned this pull request Sep 9, 2016
Merged
5 tasks
@MikeMcQuaid
Copy link
Copy Markdown
Member Author

MikeMcQuaid commented Sep 9, 2016

Fix in #899.

norm added a commit to norm/suited that referenced this pull request Apr 24, 2017
@norm
Put Homebrew checkouts separate to /usr/local
42d45ac 
Previously Homebrew was installed into /usr/local directly. Now it
installs into /usr/local/Homebrew by default. This does the same.

See Homebrew/install#60 and
Homebrew/brew#896 for more discussion on why
Homebrew changed.
@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MikeMcQuaid @DomT4 @zmwangx @ilovezfs @BrewTestBot