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

bazelrc: Add platform-specific --action_env=PATH #10305

Merged

Conversation

EricCousineau-TRI
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI commented Dec 21, 2018

Resolves #10304

\cc @liangfok


This change is Reviewable

Copy link
Contributor Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

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

+@jamiesnape for review, please.

Reviewable status: all discussions resolved, platform LGTM missing

Copy link
Contributor

@jamiesnape jamiesnape left a comment

Choose a reason for hiding this comment

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

:lgtm: assuming CI is vaguely happy.

Reviewed 3 of 3 files at r1, 3 of 3 files at r2.
Reviewable status: all discussions resolved, platform LGTM missing


tools/macos.bazelrc, line 11 at r1 (raw file):

build:python3 --action_env=DRAKE_PYTHON_BIN_PATH=/usr/local/bin/python3

# Configure ${PATH for actions (#10304).

BTW PATH or $PATH or ${PATH}.


tools/ubuntu-bionic.bazelrc, line 9 at r1 (raw file):

build:python3 --action_env=DRAKE_PYTHON_BIN_PATH=/usr/bin/python3

# Configure ${PATH for actions (#10304).

BTW PATH or $PATH or ${PATH}.


tools/ubuntu-xenial.bazelrc, line 9 at r1 (raw file):

build:python3 --action_env=DRAKE_PYTHON_BIN_PATH=/usr/bin/python3

# Configure ${PATH for actions (#10304).

BTW PATH or $PATH or ${PATH}.

@jamiesnape
Copy link
Contributor

jamiesnape commented Dec 21, 2018

Maybe you need build --action_env (I think it may get ignored otherwise)?

Copy link
Contributor

@jamiesnape jamiesnape left a comment

Choose a reason for hiding this comment

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

Reviewable status: all discussions resolved, platform LGTM missing


tools/macos.bazelrc, line 12 at r2 (raw file):


# Configure ${PATH} for actions (#10304).
--action_env=PATH=/usr/local/bin:/usr/bin:/bin

BTW build --action_env=PATH=/usr/local/bin:/usr/bin:/bin


tools/ubuntu-bionic.bazelrc, line 10 at r2 (raw file):


# Configure ${PATH} for actions (#10304).
--action_env=PATH=/usr/bin:/bin

BTW build --action_env=PATH=/usr/bin:/bin


tools/ubuntu-xenial.bazelrc, line 10 at r2 (raw file):


# Configure ${PATH} for actions (#10304).
--action_env=PATH=/usr/bin:/bin

BTW build --action_env=PATH=/usr/bin:/bin

Copy link
Contributor Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

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

+@jwnimmer-tri for platform review, please.

Reviewable status: all discussions resolved, platform LGTM missing


tools/macos.bazelrc, line 12 at r2 (raw file):

Previously, jamiesnape (Jamie Snape) wrote…

BTW build --action_env=PATH=/usr/local/bin:/usr/bin:/bin

Done. D'oh!

Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r3.
Reviewable status: 2 unresolved discussions, platform LGTM from [jwnimmer-tri]


tools/macos.bazelrc, line 11 at r3 (raw file):

build:python3 --action_env=DRAKE_PYTHON_BIN_PATH=/usr/local/bin/python3

# Configure ${PATH} for actions (#10304).

nit I'm not sure that that the issue number is buying us much here? That issue discussion is just one random specific symptom. This is really just "Bazel nulls the path by default, let's set one that we like".

Ditto for the other files.


tools/macos.bazelrc, line 11 at r3 (raw file):

build:python3 --action_env=DRAKE_PYTHON_BIN_PATH=/usr/local/bin/python3

# Configure ${PATH} for actions (#10304).

nit Is is important that these constants match execute.bzl? If so, both files should have a comment saying as much.

Ditto for the other files.

Copy link
Contributor

@jamiesnape jamiesnape left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 unresolved discussions, platform LGTM from [jwnimmer-tri]


tools/macos.bazelrc, line 11 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit Is is important that these constants match execute.bzl? If so, both files should have a comment saying as much.

Ditto for the other files.

BTW The rationale for the values is the same. I think that part of execute is probably now redundant, so we could/should switch back to the built-in bzl functions. Possibly worthy of an issue to do so, but I can not had a chance to confirm that suspicion yet.

@jamiesnape
Copy link
Contributor

Needs buildifier to be run.

@EricCousineau-TRI EricCousineau-TRI merged commit 972825e into RobotLocomotion:master Jan 2, 2019
@jamiesnape jamiesnape removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python PATH is Broken on Mac High Sierra
3 participants