Drake: Spotless still tries to build when deselected in CMake

[sherm1]

Spotless sub-build failed on Windows. (I don't think that caused any trouble though since I'm not using Matlab.) Next time I cleaned out all the binaries and CMake cache files, I unselected Spotless from the make options CMake GUI. That didn't seem to have any effect -- the next build attempted to build Spotless again and failed in the same way.

[RussTedrake]

there is a cmake option for REMOVE_UNUSED_EXTERNALS which is OFF by default (for obvious reasons). But if all was going well, the reference to snopt should have been removed from the generated makefiles at the superbuild level. So it should not have actually been called. Asking our cmake experts to take a look.

[sammy-tri]

I've seen a similar behavior (at least, I think it might be the same thing) occur when toggling WITH_SNOPT from ON to OFF. When drake builds later, build/lib/pkgconfig/snopt_c.pc is still present, so snopt_c_FOUND ends up being set to true even though WITH_SNOPT is OFF. I manually worked around this by deleting the .pc file. Not sure what the correct solution is.

[Lucy-tri]

"Make clean" usually throws an error before it completes. This issue and the failure of make clean are causing some confusing hours of time when working on externals (like SNOPT).

[jonathan-owens]

@sherm1 Are you still able to reproduce this?

From what I can tell, spotless only appears after a MATLAB executable is found. When you say you're not using MATLAB, do you mean that you don't have it installed or you're just planning on not using it now?

Anyways, I can build spotless or disable spotless and it no longer tries to build.

[sherm1]

When you say you're not using MATLAB, do you mean that you don't have it installed or you're just planning on not using it now?

@jonathan-owens -- I have Matlab on my machine but the license expired. So it does find the executable. What is probably missing is a WITH_MATLAB option that I could turn off.

[jonathan-owens]

@sherm1 Thanks for the info.

Does anyone have any objection about working on a WITH_MATLAB flag? Is that something that has been discussed before?

[RussTedrake]

The WITH_ flags imply you are going to make a local install. If WITH_SNOPT is OFF but you have a system install of snopt, cmake will still find and link against it. Perhaps the name should be more clear, but that is the current contract.

[sherm1]

Maybe add a DONT_USE_MATLAB build option? This would be useful for those of us working on the C++ only version of Drake since many are likely to have Matlab installed for other reasons.

[RussTedrake]

Why is it bad if more of the code builds?

[jwnimmer-tri]

It is useful for developers to be able to test in every end-user configuration. The build system may have a defect when matlab is absent, that I would never be able to discover if my own builds always had matlab enabled.

[RussTedrake]

But that is true of every dependency. Is matlab special? And do we want to add that much more complexity in the cmake options? And right now it could be accomplished by having matlab disappear from the cmake search path.

We did discuss in the github issues titled "critica design review of build system" from the initial kitware visit the idea of having more than just a boolean flag for each option. PTAL -- that's probably the right solution?

In my experience, having the nightly tests run more minimal builds has been sufficient. If and when they fail because we assumed a dependency, the fix has almost always been very simple.

[jwnimmer-tri]

But that is true of every dependency. Is matlab special?

Every config that #1923 comes up with must be reliably testable by a Drake developer. Assuming that at least an "all options off" and "all options on" are in the supported set, then yes, we need to be able to pretend matlab is absent, somehow.

And right now it could be accomplished by having matlab disappear from the cmake search path.

As long as there is some way to it ("disappear from the cmake search path" sound fine), I don't care how it gets done. If it's difficult or brittle to do in CMakeLists, we'll just have to find another way.

[sherm1]

And right now it could be accomplished by having matlab disappear from the cmake search path.

I tried erasing the Matlab executable which showed as the value of the CMake variable matlab in the CMake-gui that comes up with make options on Windows. It remained blank after configure/generate. Then I built Drake using make at the top level. It still executed Matlab (which stopped my build in order to run the Matlab licensing GUI that wanted me to get a new Matlab license). Later it tried and failed to build spotless.

Is matlab special?

I think it is. It is very expensive and Drake has been dependent on it for full functionality. It is highly likely to be present for reasons unrelated to Drake. We are trying to untangle from Matlab so it would be nice if the build system made that easy.

[jonathan-owens]

I think ignoring MATLAB could most easily be done with a USE_MATLAB or DONT_USE_MATLAB CMake flag. In that sense, we don't change the contract provided with the WITH_ commands, but still allow the flexibility to not use MATLAB, even if it's present.

And right now it could be accomplished by having matlab disappear from the cmake search path.

I think this is not a simple as it sounds. If you look at the FindMatlab module (run cmake --help-module FindMatlab), you'll see:

The variable ``Matlab_ROOT_DIR`` may be specified in order to give
the path of the desired Matlab version. Otherwise, the behaviour is platform
specific:

* Windows: The installed versions of Matlab are retrieved from the
  Windows registry
* OS X: The installed versions of Matlab are given by the MATLAB
  paths in ``/Application``. If no such application is found, it falls back
  to the one that might be accessible from the PATH.
* Unix: The desired Matlab should be accessible from the PATH.

Given this, I don't think it is a good idea to mess with where CMake looks for MATLAB or where one installs MATLAB. With a USE_MATLAB = false flag, we could simply never call find_program(matlab) in the CMakeLists.txt file. Then none of the other logic in the CMakeLists.txt file would have to change -- when it checks for matlab in configuring the various dependencies, it will always be false.

[jonathan-owens]

Given this, I don't think it is a good idea to mess with where CMake looks for MATLAB or where one installs MATLAB. With a USE_MATLAB = false flag, we could simply never call find_program(matlab) in the CMakeLists.txt file. Then none of the other logic in the CMakeLists.txt file would have to change -- when it checks for matlab in configuring the various dependencies, it will always be false.

I revoke this last statement. It's not so easy because of the way the build system is structured. It seems that, throughout the project, it's determined whether or not to use MATLAB via more find_program calls, instead of a variable that is set at the top level. Which, of course, makes sense as to why @RussTedrake was suggesting removing it from the search path.

However, I still maintain that that's also a difficult proposition based on the mechanics of FindMatlab (see my previous comment).

I have no problem refactoring the CMakeLists.txt files to check a USE_MATLAB variable, instead of FindMatlab. In fact, I think that would be a more coherent method than calling find_program at many spots. What are everyone else's thoughts?

[sherm1]

I think that would be a more coherent method than calling find_program at many spots. What are everyone else's thoughts?

One call to FindMatlab seems like the right number!

[jwnimmer-tri]

This discussion kind of got hijacked into a "we should have a USE_MATLAB option", which I think is super-great (+1) but didn't really address the original post.

Sherm nuked his world, asked for "no spotless please" during "make options", and spotless still built. I don't think anyone has addressed that problem yet?

[billhoffman]

Sherm still had matlab in his PATH I think.

[jwnimmer-tri]

I unselected Spotless from the make options CMake GUI ...
the next build attempted to build Spotless again and failed in the same way.

Is that the expected result? Why does having matlab or not matter for that?

[jonathan-owens]

Sherm nuked his world, asked for "no spotless please" during "make options", and spotless still built. I don't think anyone has addressed that problem yet?

That's because I have been unable to reproduce the error he was having. I can deselect spotless during make options and spotless will not build.

As make clean seems to be working now, perhaps it's worth running that, calling make options, de-selecting spotless, and trying to build again?

[jwnimmer-tri]

@sherm1 I guess nobody can reproduce this issue -- can you see if you can produce a better recipe for others to see this failure, or else close the issue?

[jwnimmer-tri]

I have filed the USE_MATLAB feature request as #2080 now.

[sherm1]

@sherm1 I guess nobody can reproduce this issue -- can you see if you can produce a better recipe for others to see this failure, or else close the issue?

@billhoffman diagnosed this many comments back -- if there is a matlab executable on the system then spotless gets built no matter what. My problem was that I had a matlab executable but no license. So the real issue is the lack of a USE_MATLAB flag that could be used to steer clear of Matlab even if it is present on the build system. So I'm happy with #2080 and will close this -- thanks!

[jwnimmer-tri]

I don't understand that explanation, but I suppose I just don't understand how CMake works, so I guess if the ticket is closed I will leave well-enough alone.

[sherm1]

It's not CMake's fault as I understand it. Our use of it assumes that if Matlab is found on a machine then the user wants to build Drake with it, and Spotless goes along for the ride. We didn't provide any way to say "don't build with Matlab even if you see it on my machine".

[jwnimmer-tri]

My confusion is that I'm not sure what setting WITH_SPOTLESS to OFF (instead of CMakeLists.txt:73 default of ON) is supposed to do? You set it to OFF, and then the build still tried to compile spotless and failed again.

Perhaps setting a WITH_FOO only supposed to work from a fresh checkout -- but once you've tried with it ON, you need to nuke your world to turn of OFF?

[sherm1]

Well, yeah -- that's what I thought too! You would think that setting WITH_SPOTLESS off would turn spotless off, but I guess we're just naive :smile:. I had the same hypothesis and tried wiping everything, and even hand-wiping the found Matlab executable. Nothing I could find stops the spotless build if Matlab is around. And the spotless build fails if Matlab is there but unlicensed. None of that behavior is inherent to CMake though, just our use of it.

[jonathan-owens]

I think there has been some confusion here, as the discussion took a number of turns away from the original problem. Feel free to correct me if I am wrong, @sherm1. The original problem was that you ran make options which found (an expired) MATLAB on your machine. This automatically triggers the build system to turn WITH_SPOTLESS to ON. However, this would fail because your MATLAB license is expired and thus the build fails. You then go back, turn WITH_SPOTLESS to OFF, rebuild, but it tries to build spotless again. This is even after you have 'nuked your world'.

What I have been saying is that I cannot reproduce this behavior. I can run make options, the super build will find MATLAB, and build with spotless. I can then either stop the build mid-spotless or let it finish, go back, make clean, turn WITH_SPOTLESS to off, and it doesn't try and build spotless anymore. So I cannot reproduce the original issue, as reported. If someone else can reproduce this issue, then we can try and address that here. I know a number of changes have been pushed since this originally happened, so I'm wondering if it somehow got worked out, perhaps with the advent of correct make clean functionality. Otherwise, I say we leave this closed.

@billhoffman had pointed out was that you had an expired MATLAB in your PATH which was being found and, by default, spotless was being turned on. If you remove that from your PATH, then you no longer have that issue. A number of us agreed that's not really an ideal solution to the problem. One should be able to build without MATLAB if he so chooses, regardless of whether or not it is on his machine.

The hiccup with this is that, perhaps due to the nature of PODs, MATLAB is found in both spotless and the super build, possibly among others, independently of whether or not you tell hide MATLAB in the super build, which, by default, will still turn WITH_SPOTLESSon. This would be best overcome by passing around the MATLAB information found in the super build and not finding it in the PODs. This can be addressed by the feature request created by @jwnimmer-tri yesterday.

[sherm1]

Thanks for the clarification, @jonathan-owens. At the moment my Matlab license is dead again so I'm using the workaround of renaming the Matlab installation directory so the Drake build can't find it. That is good enough for now, and #2080 addresses the problem well.