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

Overwrite bazel if /usr/bin/bazel already exists. #36927

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

ailzhang
Copy link
Contributor

No description provided.

@@ -259,7 +259,7 @@ if [[ "${BUILD_ENVIRONMENT}" == *xla* ]]; then
# XLA build requires Bazel
# We use bazelisk to avoid updating Bazel version manually.
sudo npm install -g @bazel/bazelisk
sudo ln -s "$(command -v bazelisk)" /usr/bin/bazel
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not call get_bazel function from .jenkins/pytorch/common.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO bazelisk is better than our get_bazel function, which installs a fixed version of bazel. bazelisk is able to read a bazelversion file in tensorflow and pull in the right version automatically.
It saves us from manually updating bazel version in pytorch CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should similarly add a .bazelversion file in pytorch and start using bazelisk in CI.
It has the benefit that:

  • Easier to manage and update
  • User would know which bazelversion to work with.

@gchanan
Copy link
Contributor

gchanan commented Apr 20, 2020

I'll merge this and we can work out a better solution later.

@gchanan gchanan merged commit 9ba0a89 into pytorch:release/1.5 Apr 20, 2020
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.

None yet

4 participants