Skip to content

Remove /opt/homebrew mentions from PATH #66

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.

Sign up for GitHub

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

Merged
zsimic merged 1 commit into from
Aug 13, 2025
Merged

Remove /opt/homebrew mentions from PATH #66

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/portable_python/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,16 @@ def compile(self):
runez.decompress(path, self.m_src_build, simplify=True)

env_vars = self._get_env_vars()
if not PPG.config.get_value("allow-homebrew"):
# Remove any mention of /opt/homebrew from PATH (reduce chances of dynamic links to homebrew)
path_env_var = env_vars.get("PATH")
if path_env_var:
Copy link
Collaborator

@reallistic reallistic Jul 2, 2025

Choose a reason for hiding this comment

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

This is gated by the presence of configured env_vars but mutates the os.environment vars. Shouldn't this:

a. be done regardless of configured env vars
b. take into account the configured paths instead of wholesale overriding them

Copy link
Contributor Author

@zsimic zsimic Jul 2, 2025

Choose a reason for hiding this comment

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

The mechanism used takes into account existing env vars, configured env vars, and programmatically yielded env vars. The idea was to do similarly to how most build scripts are written, typically like "CFLAGS=${CFLAGS} -Lmy-flag".

One can always do this to just set an env var to something (without following this "additive" behavior that p-p has):

export PATH=...
portable-python ...

But you're right I could do this at startup actually 🤔

b) is already the case (p-p follows the same "additive" approach for all env vars)

a) -> yup, I could indeed "groom" PATH at startup instead of here

Copy link
Contributor Author

@zsimic zsimic Jul 2, 2025

Choose a reason for hiding this comment

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

The only diff in doing it here vs at startup, is that here any modification of PATH gets restored... whereas if done on startup, it would stick throughout the whole p-p run.

This place here is called by the compile() function, so we'll get like:

  • openssl.compile() -> that would groom PATH then restore it
  • cpython.compile() -> same thing
  • etc

Copy link
Contributor Author

@zsimic zsimic Jul 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Actually I think it's best if we do it like this here, it does allow for an "advanced" case like for example this config:

macos:
  env:
    MACOSX_DEPLOYMENT_TARGET: 13  # Ventura
  openssl:
    allow-homebrew: true
    env: ... # extra env var additions for openssl only

Now, homebrew is allowed on macos AND for openssl only (every other component will have /opt/homebrew cleaned up from PATH by default).

One could also decide to allow homebrew globally, with like at top-level:

allow-homebrew: true  # Leave PATH alone everywhere

_paths = os.environ.get("PATH", "").split(":")
_revised = [p for p in _paths if not p.startswith("/opt/homebrew")]
if _revised != _paths:
LOG.info("Removed /opt/homebrew mentions from PATH")
env_vars["PATH"] = runez.joined(_revised, delimiter=":")
Copy link
Collaborator

@jag250 jag250 Jul 1, 2025

Choose a reason for hiding this comment

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

Would the PATH variable stay changed once this is done running? (aka cause side effects) (also I have zero context here and haven't read any other code)

Copy link
Contributor Author

@zsimic zsimic Jul 2, 2025

Choose a reason for hiding this comment

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

Env vars are in effect just for this compile() function run.


prev_env_vars = {}
for var_name, value in env_vars.items():
LOG.info("env %s=%s", var_name, runez.short(value, size=2048))
Expand Down