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: keep HOMEBREW_PREFIX/bin in PATH with HOMEBREW_ENV_FILTERING #3191

Closed
wants to merge 1 commit into from

Conversation

maxim-belkin
Copy link
Contributor

  • 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?
    Yes, here
  • Have you written new tests for your changes? Here's an example.
    Nope. Please suggest tests.
  • Have you successfully run brew tests with your changes locally?
    Nope


Fixes #2875.

Basically, with HOMEBREW_ENV_FILTERING enabled, Homebrew filters out everything, including its own bin directory from PATH. This PR brings peace of mind to those who do HOMEBREW_ENV_FILTERING stuff occasionally.

$ export HOMEBREW_ENV_FILTERING=1
$ brew install subversion --with-python
$ ls -1 /usr/local/Cellar/subversion/1.9.7/lib/perl5/site_perl/
5.26.0
$ unset HOMEBREW_ENV_FILTERING
$ brew reinstall subversion
$ ls -1 /usr/local/Cellar/subversion/1.9.7/lib/perl5/site_perl/
5.26.0

@MikeMcQuaid
Copy link
Member

Sorry but this ends up defeating much of the point of HOMEBREW_ENV_FILTERING by randomly adding things to the PATH based on user configuration (i.e. their chosen packages). 😭

@maxim-belkin
Copy link
Contributor Author

Are you suggesting #2875 is not an issue and should be, in fact, closed?

@maxim-belkin maxim-belkin deleted the fix-env-filtering branch September 22, 2017 18:12
@MikeMcQuaid
Copy link
Member

I think it's an issue but the solution is using HOMEBREW_PATH inside relevant requirements.

@maxim-belkin
Copy link
Contributor Author

How can Homebrew even think about Homebrew-ed perl if user does not specify anything on the command line?

@maxim-belkin
Copy link
Contributor Author

maxim-belkin commented Sep 22, 2017

Just a reminder, in #2875 the "problem" is that

brew install subversion --with-python

uses System's perl, not Homebrew-ed one.

There is nothing on the command line that would suggest that using system's Perl is wrong

@MikeMcQuaid
Copy link
Member

I'm not sure I understand the question. Requirements can read the user's path with HOMEBREW_PATH and which through that to find what they want us to use but we don't want to expose that globally to e.g. the build process.

@maxim-belkin
Copy link
Contributor Author

Em... Could you please elaborate on:

Requirements can read the user's path with HOMEBREW_PATH and which through that to find what they want us to use

Can't we assume that those who use Homebrew have HOMEBREW_PREFIX/bin in their PATH?

@maxim-belkin
Copy link
Contributor Author

In subversion formula,

depends_on :perl => ["5.6", :recommended]

is satisfied by system's Perl even if Homebrew-ed Perl is installed and is better/newer.

So... because we do not tell Homebrew to use its own Perl and PATH is set in such a way that only system's Perl can be found, how can Homebrew even think about using Homebrew-ed Perl?

@MikeMcQuaid
Copy link
Member

Can't we assume that those who use Homebrew have HOMEBREW_PREFIX/bin in their PATH?

Yes. Please check out the implementation of HOMEBREW_ENV_FILTERING. We reset the PATH and keep around HOMEBREW_PATH with the original user's value. The prior is used by the build process, the latter can be used by which in Requirements to add the full Cellar path to the PATH.

@maxim-belkin
Copy link
Contributor Author

def which and def which_all in requirement.rb already consults ORIGINAL_PATHS.

@MikeMcQuaid
Copy link
Member

Yes, which is why an additional fix would be required here...

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 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.

HOMEBREW_ENV_FILTERING ignores locally-satisfied requirements (?)
2 participants