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

brew.sh: don't require /usr/local/Cellar creation. #780

Merged
merged 1 commit into from Aug 24, 2016

Conversation

MikeMcQuaid
Copy link
Member

  • 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 successfully run brew tests with your changes locally?

If you're using e.g. a /usr/local/homebrew prefix then don't require the /usr/local/Cellar to be manually created to avoid e.g. /usr/local/homebrew/Cellar being used. Let's do all we can to let
people use this Cellar location as it means they can put their repository wherever they like and still use all our bottles.

CC @Homebrew/maintainers for thoughts as it's a pretty big change and @chdiza as this was part of your neat /usr/local/homebrew installation hack that I'm enjoying.

@BrewTestBot BrewTestBot added the in progress Maintainers are working on this label Aug 22, 2016
@MikeMcQuaid
Copy link
Member Author

Note: this is probably something we may need a smarter migration to handle that this but thought I'd open the easy PR to get some thoughts.

@chdiza
Copy link
Contributor

chdiza commented Aug 22, 2016

As long as this doesn't prevent someone from making (say) /usr/local/homebrew be both PREFIX and REPOSITORY, it seems fine.

@apjanke
Copy link
Contributor

apjanke commented Aug 22, 2016

I'm a little surprised that the Cellar could be located underneath HOMEBREW_REPOSITORY instead of HOMEBREW_PREFIX in the first place. Built packages may depend on the state of stuff in the prefix. If nothing else, on the set of installed Homebrew-managed packages. But since we're not fully sanitizing the environment and PATH yet, they might pick up other unbrewed things located under the prefix. So I'd think that the Cellar would have to be prefix-specific, not per-repo.

As long as this doesn't prevent someone from making (say) /usr/local/homebrew be both PREFIX and REPOSITORY, it seems fine.

When they're the same value, the Cellar directory should have been created as part of setting up the prefix, so it'll still work. (And it's a degenerate case, so both branches of the HOMEBREW_CELLAR initialization logic would come up with the same value.)

@MikeMcQuaid
Copy link
Member Author

As long as this doesn't prevent someone from making (say) /usr/local/homebrew be both PREFIX and REPOSITORY, it seems fine.

@chdiza It would not.

@MikeMcQuaid
Copy link
Member Author

Note: this is probably something we may need a smarter migration to handle that this but thought I'd open the easy PR to get some thoughts.

I've changed the approach here so this will only affect people who don't have $HOMEBREW_REPOSITORY/Cellar already existing so it's tweaking the defaults for new Cellars in /usr/local and /usr/local/bin/brew rather than moving any existing Cellar.

then
HOMEBREW_CELLAR="$HOMEBREW_PREFIX/Cellar"
elif [[ "$HOMEBREW_PREFIX" == "/usr/local" &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a single = for string comparison in shell code for consistency with existing code.

@UniqMartin
Copy link
Contributor

How about instead of further complicating the logic, we simply change the default (if no Cellar directory exists yet) for all users to a Cellar in HOMEBREW_PREFIX instead of HOMEBREW_REPOSITORY? The following code would accomplish this:

# Where we store built products; a Cellar in HOMEBREW_PREFIX (often /usr/local)
# unless there's already a Cellar in HOMEBREW_REPOSITORY.
if [[ -d "$HOMEBREW_REPOSITORY/Cellar" ]]
then
  HOMEBREW_CELLAR="$HOMEBREW_REPOSITORY/Cellar"
else
  HOMEBREW_CELLAR="$HOMEBREW_PREFIX/Cellar"
fi

The only users for which this might cause trouble are users that have HOMEBREW_PREFIX different from HOMEBREW_REPOSITORY and that (unexpectedly) have a Cellar directory in both. (We could address this potential problem by checking for the existence of both in a brew doctor check if we think this is a likely enough scenario.)

@MikeMcQuaid
Copy link
Member Author

The only users for which this might cause trouble are users that have HOMEBREW_PREFIX different from HOMEBREW_REPOSITORY and that (unexpectedly) have a Cellar directory in both. (We could address this potential problem by checking for the existence of both in a brew doctor check if we think this is a likely enough scenario.)

@UniqMartin That makes sense to me and seems suitably non-disruptive 👍

If you're using e.g. a `/usr/local/homebrew` prefix then don't require
the `/usr/local/Cellar` to be manually created to avoid e.g.
`/usr/local/homebrew/Cellar` being used. Let's do all we can to let
people use this `Cellar` location as it means they can put their
repository wherever they like and still use all our bottles.
@MikeMcQuaid MikeMcQuaid merged commit 66f2625 into Homebrew:master Aug 24, 2016
@MikeMcQuaid MikeMcQuaid deleted the usr-local-cellar branch August 24, 2016 13:41
@BrewTestBot BrewTestBot removed the in progress Maintainers are working on this label Aug 24, 2016
@UniqMartin
Copy link
Contributor

🎉

@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants