Fix install with new bazel 0.5.3 #6736

fbudin69500 · 2017-08-01T15:42:12Z

This change is

jamiesnape · 2017-08-01T15:43:47Z

Reviewed 12 of 12 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

jwnimmer-tri · 2017-08-01T15:55:33Z

Reviewed 12 of 12 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

soonho-tri · 2017-08-01T16:20:56Z

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:

INFO: Found 105 targets and 265 test targets...
FAIL: //drake/common:resource_tool_installed_test (see /private/var/tmp/_bazel_soonhok/51e1ef8bb59598000e1968965dd33f03/execroot/drake/bazel-out/osx-dbg/testlogs/drake/common/resource_tool_installed_test/test.log).
INFO: From Testing //drake/common:resource_tool_installed_test:
==================== Test output for //drake/common:resource_tool_installed_test:
[Installing] libexec/drake/drake/common/resource_tool
Traceback (most recent call last):
  File "drake/common/install", line 81, in <module>
    main(sys.argv[1:])
  File "drake/common/install", line 75, in main
    install("drake/common/resource_tool", "libexec/drake/drake/common/resource_tool")
  File "drake/common/install", line 47, in install
    shutil.copy2(src, dst_full)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/shutil.py", line 130, in copy2
    copyfile(src, dst)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/shutil.py", line 82, in copyfile
    with open(src, 'rb') as fsrc:
IOError: [Errno 2] No such file or directory: 'drake/common/resource_tool'
E
======================================================================
ERROR: test_install_and_run (__main__.TestResourceTool)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/private/var/tmp/_bazel_soonhok/51e1ef8bb59598000e1968965dd33f03/bazel-sandbox/5978266533241409104/execroot/drake/bazel-out/osx-dbg/bin/drake/common/resource_tool_installed_test.runfiles/drake/drake/common/test/resource_tool_installed_test.py", line 15, in test_install_and_run
    os.path.abspath("tmp"),
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)
CalledProcessError: Command '['drake/common/install', '/private/var/tmp/_bazel_soonhok/51e1ef8bb59598000e1968965dd33f03/bazel-sandbox/5978266533241409104/execroot/drake/bazel-out/osx-dbg/bin/drake/common/resource_tool_installed_test.runfiles/drake/tmp']' returned non-zero exit status 1

----------------------------------------------------------------------
Ran 1 test in 0.050s

FAILED (errors=1)
================================================================================
INFO: Elapsed time: 0.605s, Critical Path: 0.29s
//drake/common:resource_tool_installed_test                              FAILED in 0.3s
  /private/var/tmp/_bazel_soonhok/51e1ef8bb59598000e1968965dd33f03/execroot/drake/bazel-out/osx-dbg/testlogs/drake/common/resource_tool_installed_test/test.log

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

soonho-tri · 2017-08-01T16:22:28Z

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

soonho-tri · 2017-08-01T16:26:02Z

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

drake-jenkins-bot · 2017-08-01T16:43:06Z

Jenkins build passed

fbudin69500 · 2017-08-01T16:51:25Z

I started Mac builds on Jenkins to test this PR:
https://drake-jenkins.csail.mit.edu/job/mac-sierra-unprovisioned-clang-bazel-experimental/
https://drake-jenkins.csail.mit.edu/job/mac-elcapitan-unprovisioned-clang-bazel-experimental/

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

jwnimmer-tri · 2017-08-01T17:08:17Z

It seems like the change to spellings of load is to work around a possible Bazel bug. Can you please be sure to file that upstream?

Review status: all files reviewed at latest revision, 1 unresolved discussion.

a discussion (no related file):
My read of the commit message is that the bug is only with install.bzl (and maybe check_licenses?). This PR changes all load statements in files that have install.bzl rules. If that's what we want then so be it, but then all BUILD files in tree should switch to that style, for consistency -- not just ones that mentioned install.bzl. Alternatively, only the relevant load statements could be changed, so that we have per-bzl consistency.

Comments from Reviewable

fbudin69500 · 2017-08-01T22:04:24Z

The problem has been reported here:
https://groups.google.com/forum/#!topic/bazel-discuss/7ZCPvG3pXns

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…

My read of the commit message is that the bug is only with install.bzl (and maybe check_licenses?). This PR changes all load statements in files that have install.bzl rules. If that's what we want then so be it, but then all BUILD files in tree should switch to that style, for consistency -- not just ones that mentioned install.bzl. Alternatively, only the relevant load statements could be changed, so that we have per-bzl consistency.

The load statements of files that are used in @drakeand outside should have their workspace-qualified path, otherwise they are loaded twice and seen as different files by bazel.

Comments from Reviewable

jwnimmer-tri · 2017-08-01T22:08:49Z

a discussion (no related file):

Previously, fbudin69500 (Francois Budin) wrote…

The load statements of files that are used in @drakeand outside should have their workspace-qualified path, otherwise they are loaded twice and seen as different files by bazel.

Let me be more specific, by way of example. In drake/bindings/BUILD this PR changes the spelling of the load path as @drake//tools:mosek.bzl, but leaves drake/solvers/BUILD spelling unchanged. Why should this PR leave the two spellings inconsistent? Same for lint.bzl, etc. etc.

Comments from Reviewable

jwnimmer-tri · 2017-08-01T22:29:44Z

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…

Let me be more specific, by way of example. In drake/bindings/BUILD this PR changes the spelling of the load path as @drake//tools:mosek.bzl, but leaves drake/solvers/BUILD spelling unchanged. Why should this PR leave the two spellings inconsistent? Same for lint.bzl, etc. etc.

... 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 @drake prefixed". I think that is a difficult convention for developers to follow.

Easier conventions to follow would be:
(1) "install.bzl" must always be drake-prefixed (but not other bzl files); or
(2) all load statements in the whole project must always be drake-prefixed.

Comments from Reviewable

Bazel 0.5.x changed the way providers work. Update install and related rules to use the "new" paradigm.

fbudin69500 · 2017-08-02T14:56:12Z

Review status: 4 of 13 files reviewed at latest revision, 1 unresolved discussion.

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 @drake prefixed". I think that is a difficult convention for developers to follow.

Easier conventions to follow would be:
(1) "install.bzl" must always be drake-prefixed (but not other bzl files); or
(2) all load statements in the whole project must always be drake-prefixed.

That's correct. I updated the code to follow 1).

Comments from Reviewable

jwnimmer-tri · 2017-08-02T14:58:40Z

Reviewed 9 of 9 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

drake/common/BUILD, line 5 at r3 (raw file):

load(
    "@drake//tools:drake.bzl",

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

jwnimmer-tri · 2017-08-02T15:01:24Z

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

fbudin69500 · 2017-08-02T15:03:13Z

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…

This change is a straggler?

Yes, just updated that.

Comments from Reviewable

fbudin69500 · 2017-08-02T15:04:24Z

+@SeanCurtis-TRI Plateform review please

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

SeanCurtis-TRI · 2017-08-02T15:20:22Z

+@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.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

ggould-tri · 2017-08-02T17:26:57Z

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

jwnimmer-tri · 2023-09-20T16:08:11Z

For posterity, the Bazel upstream issue here was bazelbuild/bazel#3115.

fbudin69500 assigned jamiesnape and jwnimmer-tri Aug 1, 2017

stonier mentioned this pull request Aug 1, 2017

Shambhala on Jenkins failing RobotLocomotion/drake-external-examples#25

Closed

stonier mentioned this pull request Aug 2, 2017

Use the latest releases RobotLocomotion/drake-external-examples#26

Merged

Update install providers for Bazel changes

43c4d89

Bazel 0.5.x changed the way providers work. Update install and related rules to use the "new" paradigm.

fbudin69500 force-pushed the fix-install-with-new-bazel branch from 1c1c259 to ace4a3b Compare August 2, 2017 14:55

mwoehlke-kitware and others added 2 commits August 2, 2017 11:00

Replace deprecated BazelJavaBuilder flag '-extra_checks'

487a73d

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

fbudin69500 force-pushed the fix-install-with-new-bazel branch from ace4a3b to 487a73d Compare August 2, 2017 15:00

fbudin69500 assigned SeanCurtis-TRI Aug 2, 2017

SeanCurtis-TRI assigned ggould-tri Aug 2, 2017

jwnimmer-tri merged commit a0774df into RobotLocomotion:master Aug 2, 2017

jamiesnape removed their assignment Jun 22, 2021

jwnimmer-tri mentioned this pull request Sep 21, 2023

[tools] Consolidate rules_cc to a single point of control #20230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix install with new bazel 0.5.3 #6736

Fix install with new bazel 0.5.3 #6736

fbudin69500 commented Aug 1, 2017 •

edited by sherm1

Loading

jamiesnape commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

drake-jenkins-bot commented Aug 1, 2017

fbudin69500 commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

fbudin69500 commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

fbudin69500 commented Aug 2, 2017

jwnimmer-tri commented Aug 2, 2017

jwnimmer-tri commented Aug 2, 2017

fbudin69500 commented Aug 2, 2017

fbudin69500 commented Aug 2, 2017

SeanCurtis-TRI commented Aug 2, 2017

ggould-tri commented Aug 2, 2017

jwnimmer-tri commented Sep 20, 2023

Fix install with new bazel 0.5.3 #6736

Fix install with new bazel 0.5.3 #6736

Conversation

fbudin69500 commented Aug 1, 2017 • edited by sherm1 Loading

jamiesnape commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

soonho-tri commented Aug 1, 2017

drake-jenkins-bot commented Aug 1, 2017

fbudin69500 commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

fbudin69500 commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

jwnimmer-tri commented Aug 1, 2017

fbudin69500 commented Aug 2, 2017

jwnimmer-tri commented Aug 2, 2017

jwnimmer-tri commented Aug 2, 2017

fbudin69500 commented Aug 2, 2017

fbudin69500 commented Aug 2, 2017

SeanCurtis-TRI commented Aug 2, 2017

ggould-tri commented Aug 2, 2017

jwnimmer-tri commented Sep 20, 2023

fbudin69500 commented Aug 1, 2017 •

edited by sherm1

Loading