Skip to content

bazel: reorder PATH on macOS#21369

Merged
phlax merged 1 commit intoenvoyproxy:mainfrom
sergiitk:bazelrc-reorder-macos-path
May 19, 2022
Merged

bazel: reorder PATH on macOS#21369
phlax merged 1 commit intoenvoyproxy:mainfrom
sergiitk:bazelrc-reorder-macos-path

Conversation

@sergiitk
Copy link
Copy Markdown
Contributor

@sergiitk sergiitk commented May 19, 2022

Reorder PATH macOS so that package managers binaries take precedence
over system binaries.

Signed-off-by: Sergii Tkachenko sergiitk@google.com

Commit Message: bazel: reorder PATH on macOS
Additional Description: Reorder PATH macOS so that package managers binaries take precedence over system binaries.

  1. /opt/homebrew/bin: Homebrew
  2. /opt/local/bin: commonly used by macports
  3. /usr/local/bin:/usr/bin:/bin: user tools -> common tools -> fundamental system tools

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #20750]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

P.S. This bit me hard, I couldn't understand why bazel run was using different python version than bazel build genrule cmd.

ref #17467
cc @keith

Reorder PATH macOS so that package managers binaries take precedence
over system binaries.

Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
@sergiitk sergiitk changed the title bazel: reorder PATH macOS bazel: reorder PATH on macOS May 19, 2022
@sergiitk
Copy link
Copy Markdown
Contributor Author

Note: this also solved a problem with external/base_pip3_sphinx/BUILD.bazel complaining about missing @base_pip3_importlib_metadata rule.

@phlax
Copy link
Copy Markdown
Member

phlax commented May 19, 2022

cc @RyanTheOptimist

@sergiitk
Copy link
Copy Markdown
Contributor Author

Until this is merged, the sphinx problem can be worked around by creating user.bazelrc with the following:

build:macos --action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin

Make sure the first path with python3 binary points to python3.10. You can verify this with a target like so:

genrule(
    name = "which_python3",
    outs = ["which_python3.txt"],
    cmd = "which python3"
)

Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @sergiitk

@phlax phlax merged commit 525df54 into envoyproxy:main May 19, 2022
@sergiitk sergiitk deleted the bazelrc-reorder-macos-path branch May 19, 2022 16:12
@sergiitk
Copy link
Copy Markdown
Contributor Author

Happy to help, @phlax!

@keith
Copy link
Copy Markdown
Member

keith commented May 19, 2022

The general purpose here was actually to prefer the system one to the package manager one, since the system one is probably more likely not be broken (we've seen people do crazy things to their homebrew directories). But ideally in that case the order would be used everywhere so you wouldn't be swapping back and forth between them still.

@phlax
Copy link
Copy Markdown
Member

phlax commented May 19, 2022

ah apologies - i should have waited - do you want to revert @keith

im wondering if we can prefer just the python dirs - ideally this will be fixed with the hermetic toolchains - but tbh it would be great if some mac users could test that out for local builds before we land it - windows is already breaking

@keith
Copy link
Copy Markdown
Member

keith commented May 19, 2022

I think this should be fine in the common case, I imagine folks will be more likely to have differing versions in homebrew than they would installed via xcode, just since xcode updates less frequently, but we can roll with this until we hit issues, or ideally replace it with your toolchains change

@sergiitk
Copy link
Copy Markdown
Contributor Author

sergiitk commented May 19, 2022

@keith I understand your concern. However, we already have the case where python 3.10 is implicitly required (my build was broken because trycast expected __required_fields__, which was added in 3.9), but the default one provided by the system is 3.8. That's where package managers come in.

we've seen people do crazy things to their homebrew directories

True, but we can't control crazy things people do. We already do recommend using brew to install dependencies though - so it's on user to have their setup in order. That said, we probably should add python 3.10 to that brew install list.

@phlax
Copy link
Copy Markdown
Member

phlax commented May 19, 2022

@sergiitk we are just chatting about this very issue on slack (#envoy-users) channel - please do jump on - another related issue is different HOMEBREW install paths

@keith
Copy link
Copy Markdown
Member

keith commented May 19, 2022

True, but we can't control crazy things people do. We already do recommend using brew to install dependencies though - so it's on user to have their setup in order.

Yea I wasn't suggesting we try to not recommend using brew ever, but it's less likely that they could do something unexpected with /usr/bin/python3 than with the homebrew version, which just translates to issues. But either way until we can move to actual toolchains I think this is fine

ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Reorder PATH macOS so that package managers binaries take precedence
over system binaries.

Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

generate compile_commands.json fail with python 3.10.3

3 participants