RobotLocomotion / drake

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

disabling an external does not remove it from the build #3580

Closed RussTedrake closed 6 years ago

RussTedrake commented 8 years ago

I’m still having trouble building iris on my mac. After a clean build with all supported externals, i tried to disable iris, but it is still being built.

drake008% grep -i iris CMakeCache.txt
WITH_IRIS:BOOL=OFF
drake008% cmake ..
-- CMAKE_INSTALL_PREFIX=/Users/russt/drake-distro/build/install
-- Found MATLAB at /usr/local/bin/matlab
-- Preparing to build bertini
-- Preparing to build eigen
-- Preparing to build gloptipoly
-- Preparing to build gurobi
-- Preparing to build meshconverters
-- Preparing to build mosek
-- Preparing to build sedumi
-- Preparing to build snopt
-- Preparing to build spdlog
-- Preparing to build swigmake
-- Preparing to build yalmip
-- Preparing to build avl
-- Preparing to build bullet
-- Preparing to build cmake
-- Preparing to build dreal
-- Preparing to build gflags
-- Preparing to build googletest
-- Preparing to build google_styleguide
-- Preparing to build nlopt
-- Preparing to build spotless
-- Preparing to build octomap
-- Preparing to build swig_matlab
-- Preparing to build textbook
-- Preparing to build yaml_cpp
-- Preparing to build iris with dependencies eigen;mosek
-- Preparing to build lcm
-- Preparing to build libbot with dependencies lcm
-- Preparing to build bot_core_lcmtypes with dependencies lcm;libbot
-- Preparing to build director with dependencies bot_core_lcmtypes;lcm;libbot
-- Preparing to build signalscope with dependencies director
-- Preparing to build xfoil
-- Preparing to build drake with dependencies avl;bertini;bot_core_lcmtypes;bullet;cmake;director;dreal;eigen;gflags;gloptipoly;google_styleguide;googletest;gurobi;iris;lcm;libbot;meshconverters;mosek;nlopt;octomap;sedumi;snopt;spdlog;spotless;swig_matlab;swigmake;yalmip;yaml_cpp;xfoil
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/russt/drake-distro/build

note that it is still “preparing to build iris” despite my having turned it off.

jamiesnape commented 8 years ago

FYI @mwoehlke-kitware.

mwoehlke-kitware commented 8 years ago

This could happen if either WITH_ALL_PUBLIC_EXTERNALS or WITH_ALL_SUPPORTED_EXTERNALS are ON. Did you check that both of those are OFF?

RussTedrake commented 8 years ago

you caught me. WITH_ALL_PUBLIC_EXTERNALS was still on. That's a really nasty interface.

jwnimmer-tri commented 8 years ago

Perhaps reopen the ticket, with the fix being that CMake should fail-fast if the user selects conflicting options?

mwoehlke-kitware commented 8 years ago

Failing seems like it would defeat the purpose of the option. Maybe we could spit out a STATUS message instead telling why we are building the external anyway.

Alternatively, do we even need WITH_ALL_PUBLIC_EXTERNALS or WITH_ALL_SUPPORTED_EXTERNALS?

david-german-tri commented 8 years ago

Alternatively, do we even need WITH_ALL_PUBLIC_EXTERNALS or WITH_ALL_SUPPORTED_EXTERNALS?

Good question. I've always found those options confusing, but they predate me and maybe someone has a workflow that depends on them.

jamiesnape commented 8 years ago

I would vote to remove the options. There are several (mostly sensible) ways I can see that you could go when there are conflicting flags.

  1. Fail.
  2. Ignore WITH_ALL_*.
  3. Ignore WITH_* (current behavior?).
  4. Compute difference/union of WITH_ALL_* and WITH_*.
  5. Remove WITH_ALL_* options.
  6. Remove WITH_* options (just for completeness).

Which probably means someone will be confused (except for the last two options). For the ignore, there should at least be a warning.

RussTedrake commented 8 years ago

Another possibility that we discussed last Dec w/ kitware's initial visit was to have the WITH_* options toggle between more states (ON | OFF | IF_ALL_EXTERNALS ) or something like that.

I'm ok with removing the WITHALL* options (and updating the docs). The problem they were originally trying to solve was that many people don't have permissions for all of the externals (and then get a mysterious failure at the download step). Is there a better way to label the ones that require additional permissions? In the name (e.g. WITH_SNOPT_REQUIRES_PERMISSIONS) or having the toggle be ( OFF | ONREQUIRES_PERMISSIONS ) . OR we could have a separate option for HAVE_PERMISSIONS_SNOPT (true|false) and then WITH_SNOPT could be (OFF | IF_PERMISSIONS) and WITH_SNOPT_PRECOMPILED could be (OFF | IF_NO_SNOPT_PERMISSIONS) or something?

jwnimmer-tri commented 8 years ago

Or stop trying to push CMake past its limits, and admit we need a wrapper script around the initial configuration and superbuild step? Many of us already have our own custom bash or python script to invoke a first-time build, since it's already impossible to do correctly by hand. Putting that in-tree would be of benefit to everyone.

RussTedrake commented 8 years ago

would be happy to have something like that in-tree. can we write something useful for users that have different acks? thinking about how we could do that makes me realize there is another possibility using CMAKE. We could have the WITH_ALL_SUPPORTED_EXTERNALS and WITH_ALL_PUBLICEXTERNALS be effectively command line options only, instead of (redundant) options. If those variables are defined, then it changes the defaults of the WITH*, but it will never override their settings nor the the cache. That's actually my favorite solution yet.

so the doc would tell people to run

cmake -DWITH_ALL_PUBLIC_EXTERNALS

or

cmake -DWITH_ALL_SUPPORTED_EXTERNALS

if they have access to the internal repos. (we could potentially change that to have a special one for RLG, for TRI, ...)

mwoehlke-kitware commented 8 years ago

The problem they were originally trying to solve was that many people don't have permissions for all of the externals (and then get a mysterious failure at the download step). Is there a better way to label the ones that require additional permissions?

We could probably do the opposite; add an option like 'WITH_PRIVATE_EXTERNALS', and hide non-public externals if that is OFF.

There was also talk of moving the CI option sets into the drake proper repo. Maybe we could do that and leverage the same machinery for users also, somewhat like the immediately preceding comment.

jamiesnape commented 6 years ago

These various options no longer exist. All this is now handled by Bazel.