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

Easier and bulletproofed USE_DRAKE=ON #520

Open
stonier opened this issue Jun 12, 2017 · 7 comments
Open

Easier and bulletproofed USE_DRAKE=ON #520

stonier opened this issue Jun 12, 2017 · 7 comments

Comments

@stonier
Copy link

stonier commented Jun 12, 2017

If USE_DRAKE:=ON, make sure Drake gets the right VTK library, fail fast if it doesn't.

  • call find_package(DRAKE)
  • on linux set HINTS to find VTK at drake's external path
  • on apple set HINTS to find TK at the homebrew install path
  • use these to call find_package(VTK REQUIRED HINTS ...) appropriately
  • check the VTK version found, if not 8.0, fail fast with a warning
  • This will mean making sure the VTK logic should shift to post-drake logic
@stonier
Copy link
Author

stonier commented Jun 12, 2017

Second problem is that any launching will fail because the PYTHONPATH will not be pointing to Drake's VTK, nor it's python bindings. The user will need to prefix to it as per drake_visualiser_linux.sh and drake_visualiser_apple.sh.

Not really something to hold up this script though - we can do something over in spartan.

@stonier stonier mentioned this issue Jun 12, 2017
@patmarion
Copy link
Member

patmarion commented Jun 12, 2017

This is specific to the drake cmake superbuild, right? The bazel build doesn't compile director, nor does it use a precompiled drake-visualizer that was built with USE_DRAKE=ON. Is that right?

This logic is easier handled in the drake cmake superbuild than director superbuild, since presumably the drake superbuild already has called find_package(VTK) and knows exactly which value of VTK_DIR to use. But I am happy to accept a PR to the director superbuild if you want to implement this logic there.

If VTK is installed to the superbuild's install prefix then there is no need to set PYTHONPATH. Are you installing VTK somewhere else? If there are just a few locations (like homebrew cellar) then we should hardcode them as hints in director/__init__.py.

@jamiesnape
Copy link

This is specific to the drake cmake superbuild, right?

No. This refers to building Director however Drake is built.

@patmarion
Copy link
Member

Ok, got it. I'm only aware of the spartan superbuild, drake superbuild, and openhumanoids using the USE_DRAKE option. Is there another usage?

@stonier
Copy link
Author

stonier commented Jun 12, 2017

Note that I know of.

@patmarion
Copy link
Member

patmarion commented Jun 12, 2017

Ok. So currently Director will call find_package(drake) which runs in module mode, not config mode, and executes Director's FindDrake.cmake script. Would you propose that we update this script to also call find_package(VTK) and implement the version checking / fail fast logic there?

Or should we wait until drake-config.cmake works and correctly exports its VTK version? I think that is tracked by drake issue RobotLocomotion/drake#5456

@jamiesnape
Copy link

If you can do the modifications quickly, then it would not hurt to add them to FindDrake, but not worth spending a huge amount of time.

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

No branches or pull requests

3 participants