-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix install with new bazel 0.5.3 #6736
Fix install with new bazel 0.5.3 #6736
Conversation
Reviewed 12 of 12 files at r1, 1 of 1 files at r2. Comments from Reviewable |
Reviewed 12 of 12 files at r1, 1 of 1 files at r2. Comments from Reviewable |
I have the following problem on my local mac. FYI, this is not something introduced by this PR. But I expected that it's resolved by this PR but it's not the case:
Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
oh.. wait. I might run it with a wrong PR. Retesting it now.. Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
OK. I'm happy to report that it actually fixed my PR. Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
Jenkins build passed |
I started Mac builds on Jenkins to test this PR: Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
It seems like the change to spellings of Review status: all files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file): Comments from Reviewable |
The problem has been reported here: Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
The load statements of files that are used in Comments from Reviewable |
a discussion (no related file): Previously, fbudin69500 (Francois Budin) wrote…
Let me be more specific, by way of example. In Comments from Reviewable |
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
... or in other words, this PR seems to imply a convention of "If a BUILD file mentions install.bzl, then all of its load statements should be Easier conventions to follow would be: Comments from Reviewable |
Bazel 0.5.x changed the way providers work. Update install and related rules to use the "new" paradigm.
1c1c259
to
ace4a3b
Compare
Review status: 4 of 13 files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
That's correct. I updated the code to follow 1). Comments from Reviewable |
Reviewed 9 of 9 files at r3. drake/common/BUILD, line 5 at r3 (raw file):
This change is a straggler? Comments from Reviewable |
Apparently in newer versions of Bazel (starting with 0.5? As of 0.5.3 at least...), Bazel doesn't recognize that a provider is the same entity if one BUILD loads the .bzl file that declares the provider with a workspace-qualified name, and another omits the workspace qualification. Since these are the same .bzl, this feels suspiciously like a Bazel bug. Anyway, work around the issue by always using the workspace-qualified path. This should get `bazel build install` working again with Bazel 0.5.3.
Using the flag '-extra_checks' was generating the following error: BazelJavaBuilder threw exception: -extra_checks is no longer supported; use\ -XepDisableAllChecks to disable Error Prone
ace4a3b
to
487a73d
Compare
Reviewed 1 of 1 files at r4. Comments from Reviewable |
Review status: all files reviewed at latest revision, 1 unresolved discussion. drake/common/BUILD, line 5 at r3 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Yes, just updated that. Comments from Reviewable |
+@SeanCurtis-TRI Plateform review please Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
+@ggould-tri for final platform review. Reviewed 3 of 12 files at r1, 1 of 1 files at r2, 8 of 9 files at r3, 1 of 1 files at r4. Comments from Reviewable |
Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
For posterity, the Bazel upstream issue here was bazelbuild/bazel#3115. |
This change is