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

Update to bazel-0.18.0 and use try-import #22964

Merged
merged 7 commits into from
Dec 13, 2018

Conversation

perfinion
Copy link
Member

Bazel-0.18.0 adds a try-import option that will non-fatally try and
import a file. Use this for the configure options so that .bazelrc does
not need to change. ./configure rewriting .bazelrc makes using the git
repo annoying because the file is changed.

Also optionally import a /.bazelrc.user file that is gitignored so
user-specific options can go in there.

Fixes: #22762
Fixes: #22906
Signed-off-by: Jason Zaman jason@perfinion.com

@perfinion
Copy link
Member Author

Bazel-0.18.0 is due out on monday. Lets merge this in as soon as we can. Having ./configure edit the .bazelrc is causing a lot of annoyance.

@gunan
Copy link
Contributor

gunan commented Oct 14, 2018

Bumping the minimum required version to the cutting edge version has caused many issues before.
Due to this, I am against this change in minimum requirement until at least bazel 0.19 is out.

@perfinion
Copy link
Member Author

@gunan okay fair enough. In that case bazel-0.18.0 has temporarily added back reading tools/bazel.rc so I'll make a separate PR to move things back there then when 0.19.0 is out we do this change.

@yongtang
Copy link
Member

The issue #22449 might be related (needs bazel update > 0.17.1).

meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this pull request Oct 16, 2018
Disable TensorFlow from downstream project until tensorflow/tensorflow#22964 is submitted 

@philwo @mhlopko
philwo pushed a commit to bazelbuild/continuous-integration that referenced this pull request Oct 16, 2018
Disable TensorFlow from downstream project until tensorflow/tensorflow#22964 is submitted 

@philwo @mhlopko
@yongtang
Copy link
Member

@gunan @perfinion It looks like bazel 0.19.0 has been released:
bazelbuild/bazel#6116

@meteorcloudy
Copy link
Member

Since Bazel 0.19.0 has come out, can we try again to merge this change?
Currently, TensorFlow has been disabled from Bazel downstream projects due to the bazelrc file change, I wound like to bring it back before we cut 0.20.0.

@meteorcloudy
Copy link
Member

@perfinion Is this PR going to fix TF build with Bazel 0.19.0? If so, can you try to resolve the conflicts? Thanks!

@perfinion
Copy link
Member Author

@gunan okay to merge now? bazel 0.19 is out

@perfinion perfinion added the kokoro:force-run Tests on submitted change label Nov 8, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 8, 2018
@meteorcloudy
Copy link
Member

We need to upgrade Bazel to 0.18.1 on all CI machines before merging this.

@angerson
Copy link
Contributor

angerson commented Nov 10, 2018

@meteorcloudy Is that upgrade only required on TensorFlow's CI machines?
@av8ramit What do we need to do to upgrade Bazel on the CI machines?

@meteorcloudy
Copy link
Member

@angersson Yes, in order not to break anything, we should upgrade Bazel on all TF CI machines.
But, of course, after merging this change, anyone wants to build TensorFlow from HEAD needs at least 0.18.X.

@gunan
Copy link
Contributor

gunan commented Nov 12, 2018

@meteorcloudy, then this change needs to be done internally, to be able to make all the test infra changes atomicly.
Would you like to import this change and make those upgrades? @yifeif can provide the instructions to start by importing this change internally.

@meteorcloudy
Copy link
Member

Sounds good, I can help import this change and make those upgrades!

@tensorflowbutler
Copy link
Member

Nagging Assignee @case540: It has been 15 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

Bazel-0.18.0 adds a try-import option that will non-fatally try and
import a file. Use this for the configure options so that .bazelrc does
not need to change. ./configure rewriting .bazelrc makes using the git
repo annoying because the file is changed.

The allowed bazel range is now 0.18.0-0.20.0 inclusive. The env var
TF_IGNORE_MAX_BAZEL_VERSION can be set to skip the max bazel version
check.

Also optionally import a /.bazelrc.user file that is gitignored so
user-specific options can go in there.

Fixes: tensorflow#22762
Fixes: tensorflow#22906
Signed-off-by: Jason Zaman <jason@perfinion.com>
@perfinion
Copy link
Member Author

I updated this. min version is 0.18.0, max is 0.20.0. I added an env variable to skip the max bazel version check to make testing new versions easier.
User-specific changes should go in .bazelrc.user which is gitignored. Nothing should edit .bazelrc anymore since it is no longer gitignored.

Looks like the the CI builds are all failing, do they need to be upgraded?

@meteorcloudy
Copy link
Member

Yes, looks like Bazel is still 0.16.1 on Ubuntu. Can we upgrade to the latest Bazel on all CI machines?
@gunan @yifeif

@yifeif
Copy link
Contributor

yifeif commented Dec 5, 2018

@meteorcloudy I'll send you a change to upgrade ubuntu to the same bazel version internally. Ping me if you need help with pulling this PR in.

Copy link
Contributor

@gunan gunan left a comment

Choose a reason for hiding this comment

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

While minimum is 0.18, I still want our dockerfiles to have 0.20 because 0.18 and 19 have some bugs

@angerson angerson added the ready to pull PR ready for merge process label Dec 11, 2018
@meteorcloudy
Copy link
Member

@perfinion The Bazel version on CI machines has been upgraded now, can you rebase this PR again then we can rerun the presubmit for it. ;)

@tensorflow-copybara tensorflow-copybara merged commit 2349590 into tensorflow:master Dec 13, 2018
tensorflow-copybara pushed a commit that referenced this pull request Dec 13, 2018
PiperOrigin-RevId: 225413451
@perfinion perfinion deleted the bazelrc branch January 6, 2019 10:14
joeleba pushed a commit to joeleba/continuous-integration that referenced this pull request Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet