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

python: stop passing no-user flag on installation #10655

Merged
merged 1 commit into from Feb 22, 2021

Conversation

dtrodrigues
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 written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

--no-user is not a supported end-user flag in pip, and is explicitly non-documented: pypa/pip#8977 (comment)
pypa/pip@87bb4f9

It was added as part of building python packages from source in case a user's configuration specified --user installs due to the fact that you cannot do a --user install within a venv:

$ pip install --user foo
ERROR: Can not perform a '--user' install. User site-packages are not visible in this virtualenv.

Recent versions of pip fall back to a user installation if site-packages is not writeable. Additionally, when building a formula, only the global and site pip configurations are looked at -- the user configuration is empty within ..../.brew_home/, so this change in Homebrew doesn't impact users who have user installs as a local configuration.

@BrewTestBot
Copy link
Member

Review period will end on 2021-02-22 at 18:07:50 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 19, 2021
Copy link
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

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

I agree. If --no-user is "not a supported end-user flag" we shouldn't use it.

Separately, is it still possible for a user's global pip configuration file to affect builds? Should/can we handle this? I'm not just thinking about the --user or --no-user issue. Are there other settings that could be set in one of these that would affect the build? It's possible that we manually override everything that needs to be controlled, I'm just not familiar enough to judge.

@dtrodrigues
Copy link
Member Author

Yes - a user's global pip configuration does affect builds, but not their local configuration (since $HOME is replaced by .brew_home) or their environment variables (filtered out).

Running on macOS, during a build from source (for example for the jc python formula), the pip configuration is looked for in

global:
  /Library/Application Support/pip/pip.conf, exists: False
site:
  /opt/homebrew/Cellar/jc/1.14.3/libexec/pip.conf, exists: False
user:
  /private/tmp/jc-20210219-35155-hhi3f/jc-1.14.3/.brew_home/.pip/pip.conf, exists: False
  /private/tmp/jc-20210219-35155-hhi3f/jc-1.14.3/.brew_home/.config/pip/pip.conf, exists: False

On Linux, it is

global:
  /etc/xdg/pip/pip.conf, exists: False
  /etc/pip.conf, exists: False
site:
  /home/linuxbrew/.linuxbrew/Cellar/jc/1.14.3/libexec/pip.conf, exists: False
user:
  /tmp/jc-20210220-30481-ijb3gc/jc-1.14.3/.brew_home/.pip/pip.conf, exists: False
  /tmp/jc-20210220-30481-ijb3gc/jc-1.14.3/.brew_home/.config/pip/pip.conf, exists: False

There are some settings which can be beneficial when passed into the Homebrew python building of a formula from source (see Homebrew/discussions#863). The only way to manipulate those is by changing the global config file, which typically requires administrative privileges.

The cases in which we want to allow a user to influence their formula builds and how much is an open question and a tradeoff between reproducibility and customization.

FWIW, the --isolated flag for pip doesn't do anything for us since it only ignores user configurations, and those are already ignored -- it doesn't isolate from site or global configurations: pypa/pip#3828

@Rylan12
Copy link
Member

Rylan12 commented Feb 20, 2021

Gotcha, thanks

@MikeMcQuaid
Copy link
Member

Makes sense 👍🏻

@BrewTestBot
Copy link
Member

Review period ended.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 22, 2021
@dtrodrigues dtrodrigues merged commit b1bde5f into Homebrew:master Feb 22, 2021
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Mar 25, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants