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

Avoid redundant bazel build #4458

Merged
merged 7 commits into from
Mar 23, 2019
Merged

Conversation

raulchen
Copy link
Contributor

What do these changes do?

Bazel will invalidate caches if the command-line options are different. This PR puts all needed action_envs in the .bazelrc file to avoid cache invalidation.

Related issue number

@raulchen raulchen requested review from pcmoritz and guoyuhong March 22, 2019 12:15
@@ -1,2 +1,4 @@
# build config
build --compilation_mode=opt
build --action_env=PATH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pcmoritz I think by passing PATH to bazel, #4334 is not needed any more. But I don't know how to verify that.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is still needed, otherwise we can't guarantee which python will be picked up if there are multiple ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, just noticed that the build.sh script takes a -p option.

@raulchen
Copy link
Contributor Author

Java CI time reduced from 32min to 17min

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13194/
Test FAILed.

Copy link
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

LGTM, but let's be careful to monitor that introducing PATH into the python build doesn't break things (i.e. different binaries might be picked up based on whether PATH is set or not).

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.

4 participants