Add precompiled drake-visualizer to Bazel build#6259
Add precompiled drake-visualizer to Bazel build#6259jamiesnape merged 2 commits intoRobotLocomotion:masterfrom jamiesnape:bazel-drake-visualizer
Conversation
|
+@mwoehlke-kitware for feature review. |
|
@mwoehlke-kitware Could you review this? Very high priority. |
|
Reviewed 7 of 7 files at r1. tools/director.bzl, line 15 at r1 (raw file):
BTW, since we're using this now in multiple places, would it make sense to move it to a utility function? tools/director.bzl, line 28 at r1 (raw file):
Oh, goody... is this (unlike VTK) something I can turn off if I don't want it? Comments from Reviewable |
|
I'll take platform review and test it locally, though we should probably get one more volunteer to test locally. Comments from Reviewable |
|
I'll do test this on my local macs today. |
|
Reviewed 7 of 7 files at r1. WORKSPACE, line 262 at r1 (raw file):
BTW Probably this stanza (and its bzl file) should be removed in this PR? Or is that happening in a follow-up? In whichever PR removes this stanza, the drake/automotive/BUILD, line 439 at r1 (raw file):
The tools/BUILD, line 28 at r1 (raw file):
Probably the tools/director.bzl, line 55 at r1 (raw file):
Are you sure we need The only thing I can see that might need tools/drake_visualizer_apple.sh, line 4 at r1 (raw file):
FYI So this is probably fine, but is repeating some tools/drake_visualizer_linux.sh, line 3 at r1 (raw file):
FYI If you want to be a bit safer with LD_LIBRARY_PATH modification, apparently the better syntax goes like torch/distro#228. I'm not sure that it matters when we're in a sandbox already, though. Comments from Reviewable |
You may need to wait for an update of the drake-visualizer binary for Mac. |
|
Review status: all files reviewed at latest revision, 8 unresolved discussions. tools/director.bzl, line 55 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I think we need it for Mac. I will double check. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 8 unresolved discussions. WORKSPACE, line 262 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I was going to follow up, but I should probably do that here. tools/director.bzl, line 28 at r1 (raw file): Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Not at present. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 8 unresolved discussions. tools/drake_visualizer_linux.sh, line 3 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Indeed; I'd be in favor of that also: Comments from Reviewable |
|
a discussion (no related file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Is Comments from Reviewable |
|
a discussion (no related file): Previously, jamiesnape (Jamie Snape) wrote…
The first fix was to Comments from Reviewable |
Sure. Please ping me here when it's ready. Review status: all files reviewed at latest revision, 9 unresolved discussions. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I probably need to fix the Python path too. Comments from Reviewable |
|
a discussion (no related file): Previously, jamiesnape (Jamie Snape) wrote…
Indeed Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions. WORKSPACE, line 262 at r1 (raw file): Previously, jamiesnape (Jamie Snape) wrote…
Do you want the file Comments from Reviewable |
|
WORKSPACE, line 262 at r1 (raw file): Previously, jamiesnape (Jamie Snape) wrote…
I think yes; git remembers. Comments from Reviewable |
|
Waiting on update of |
|
Review status: 2 of 10 files reviewed at latest revision, 8 unresolved discussions. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Done. WORKSPACE, line 262 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Done. 🐘 drake/automotive/BUILD, line 439 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Done. tools/BUILD, line 28 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Done. tools/director.bzl, line 55 at r1 (raw file): Previously, jamiesnape (Jamie Snape) wrote…
Apparently not. Gone here and in VTK. tools/drake_visualizer_linux.sh, line 3 at r1 (raw file): Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Done. Also for Comments from Reviewable |
|
Reviewed 8 of 8 files at r2. Comments from Reviewable |
|
tools/drake_visualizer_linux.sh, line 4 at r2 (raw file):
The Comments from Reviewable |
|
a discussion (no related file): Valgrind identifies the culprit as: Hello to our old friend, the static destruction order fiasco (probably). I don't think that should block this PR, but is annoying and worth a follow-up issue. Comments from Reviewable |
|
Review status: 9 of 10 files reviewed at latest revision, 3 unresolved discussions. tools/drake_visualizer_linux.sh, line 4 at r2 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Done. Comments from Reviewable |
|
Tested on Xenial. We should probably get ack's from Trusty and OS X testers as well. Reviewed 1 of 1 files at r3. Comments from Reviewable |
|
When testing, also run |
|
Review status: all files reviewed at latest revision, 2 unresolved discussions. Comments from Reviewable |
|
Double Review status: all files reviewed at latest revision, 2 unresolved discussions. Comments from Reviewable |
|
@soonho-tri The new Mac packages are uploaded. Do a |
|
Reviewed 1 of 1 files at r4. Comments from Reviewable |
|
Re-confirmed to work for me on Xenial (and the shutdown error is no longer happening to me, for whatever reason). Review status: all files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file):
Also, the symbolic link Comments from Reviewable |
|
a discussion (no related file): Previously, soonho-tri (Soonho Kong) wrote…
Does Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file):
No.
In the end, I also have: Comments from Reviewable |
|
I guess we need to split the instructions into Bazel and CMake at the very least. For now, uninstall |
|
Review status: all files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file): Previously, soonho-tri (Soonho Kong) wrote…
Running Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 1 unresolved discussion. a discussion (no related file): Previously, soonho-tri (Soonho Kong) wrote…
Comments from Reviewable |
|
We will update CI on Monday morning and then merge this. Once RobotLocomotion/director#508 goes in, life will get easier as everything can use Qt 5 and VTK 8. |
|
BTW, I think it's a good practice not to destructive-update the $ brew update
error: Failed to merge in the changes.
Patch failed at 0001 Add vtk@8.0 formula
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".A user has to untap and retap to resolve the issue (or go to the repository and do |
|
CI has been updated. |
Toward #3129. This will need a Mac CI update immediately before it is merged. It also may need to be coordinated with RobotLocomotion/director#508.
This change is