RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.22k stars 1.25k forks source link

doc: Need to improve docs for CMake build options (e.g. SNOPT) #12175

Open EricCousineau-TRI opened 4 years ago

EricCousineau-TRI commented 4 years ago

Per slack convo: https://drakedevelopers.slack.com/archives/C3YB3EV5W/p1570643928005400

hongkai.dai Today at 1:58 PM
When I build drake python binding with -DWITH_SNOPT=ON. But I got the error saying that  SNOPT source archive was NOT found at 'SNOPT_PATH-NOTFOUND'. I have set SNOPT_PATH=git but it doesn't help

eric.cousineau  1 hour ago
Was a bit confused by this too; apparently this has different semantics than Bazel.
If you want the equivalent of SNOPT_PATH=git in Bazel, then you should pass -DWITH_ROBOTLOCOMOTION_SNOPT=ON.
If you want the equivalent usage of SNOPT_PATH, you have to explicitly pass it as -DSNOPT_PATH, the CMakeLists.txt is not built to look at env variables.
\cc @jamiesnape (edited) 

hongkai.dai  1 hour ago
Got it, thanks!

hongkai.dai  1 hour ago
It would be helpful if we mention how to build with SNOPT in https://drake.mit.edu/python_bindings.html#building-the-python-bindings

eric.cousineau  1 hour ago
Yeah, better docs would help.
I think it's kinda weird that we have those instructions in the Python binding instructions... I'll see if there's a way to put those instructions in a more sensical location, and xref.
I can try to get to that next week.

\cc @jamiesnape

jwnimmer-tri commented 4 years ago

Still fine to improve the documentation, but a general tip: if you're not sure how to pass options to cmake, run ccmake instead and browse through the user interface. It works well.

rcory commented 4 years ago

Even updating the existing documentation would help, perhaps with a note that it will be improved later. I just got bit by this issue and was totally puzzled until I finally came accross this issue. A small note in the documentation would have helped.

mpetersen94 commented 3 years ago

This just bit me again today. Was following the instructions for building bindings and got the same error as Hongkai.

jwnimmer-tri commented 2 years ago

Relates to #12306 somewhat, as well.

shaoyuancc commented 1 year ago

I ran into this issue today

bernhardpg commented 1 year ago

I also ran into this issue now!

egordon commented 3 months ago

Just ran into this issue as well.

It appears that the CMake file sets SNOPT_PATH to "SNOPT_PATH-NOTFOUND", like the string literal (unless I'm misunderstanding both the set function and the error message).

I was able to fix it with a 1-line change to CMakeLists.txt (see patch below) that pulls from the environment variable instead (as per the documentation).

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -484,7 +484,7 @@ if(WITH_ROBOTLOCOMOTION_SNOPT OR WITH_SNOPT)
   if(WITH_ROBOTLOCOMOTION_SNOPT)
     string(APPEND BAZEL_REPO_ENV " --repo_env=SNOPT_PATH=git")
   else()
-    set(SNOPT_PATH SNOPT_PATH-NOTFOUND CACHE FILEPATH
+    set(SNOPT_PATH "$ENV{SNOPT_PATH}" CACHE FILEPATH
       "Path to SNOPT source archive"
     )
     if(NOT EXISTS "${SNOPT_PATH}")