DM-11902: use CMake installer to ensure we install everything #3
Conversation
ups/eupspkg.cfg.sh
Outdated
| # Eigen uses cmake, but fortunately the scripts are easy enough to | ||
| # emulate w/o having to have cmake as a dependency | ||
| # | ||
| (mkdir build && cd build && cmake -DCMAKE_INSTALL_PREFIX=$PREFIX .. && make) |
There was a problem hiding this comment.
This might be a bit pedantic, but as the cmake command is being used to generate the make file, it would be more consistent with the eupspkg system if this went into the separate config function. This would make the cfg file longer, but would give future users an easier way to run config separately from build if they ever wanted to.
Also, I dont know if this has been fixed or not, but in the past with other packages I have had issues where any directories that were created manually were not wiped out before a new build. This caused prior config outputs and such to be used and not reflect any changes that had been made on the system. Is that no longer the case?
There was a problem hiding this comment.
Moving the cmake command to to the config function is a good idea; will do (I was just copying what another cmake eupspkg.cfg.sh to start this one).
I'm not sure what the status of reusing directories is in the build system, or even whether it's considered a feature or a bug. For stack packages, it would almost always be the former, as it would permit SCons to avoid recompiling pieces of packages that haven't changed.
There was a problem hiding this comment.
So the problem could come in with the configuration step. I am not sure if cmake would suffer from this, but with ./configure it would write out the info on all its checks and paths it finds. If you run it a second time, it often knows that the checks have been done and does not re-run them leaving the configuration intact. If the system has changed, versions updated etc, then it is possible that the old configurations may be used instead of new
There was a problem hiding this comment.
I've both moved the cmake invocation to config() and added an rm command to delete the build directory before running it; while I'm not sure if that's desirable in all cases, the config step for Eigen is so simple that it seems harmless to run it every time dependencies have changed, as you've suggested. Just waiting on final Jenkins run.
Since LSST expects to do:
instead of
(which seems to be the way Eigen expects to be used) I also added symlinks from the eigen3 subdirectory back to the root include directory.