habitat-sim
habitat-sim copied to clipboard
Update to use modern CMake practices, tighten warning levels
Various good-to-have things to do:
- [ ] replace
include_directories()withtarget_include_directories(), preferringPRIVATEwhere possible; if a dependency provides a CMake target use that instead - [x] remove use of
file(GLOB), list sources explicitly (makes the build more predictable without things getting broken when there are untracked files in the working copy) -- #158 - [x] use
configure_file()instead ofadd_definitions()-- #143 - [x] convert
if(${FOO})to justif(FOO)-- #58 - [ ] look into TODOs with CMAKE_CXX_VISIBILITY_PRESET
- [x] ~~set up some JOB_POOLS so it's possible to use more cores for compilation without risking OOM issues (currently a compilation of one pybind11 file takes up to 1.8 GB of RAM, we definitely don't want to run that together with 7 other jobs)~~ will not be needed after #64 lands, since it reduces the bindings code quite a lot
- [ ] enable
CORRADE_USE_PEDANTIC_FLAGSfor all Habitat code (postponed from #116) - [x] ~~go back to CMake 3.5 since that's the oldest version currently in widespread use (Ubuntu 16.04)? -- blocked by tinyply, which requires 3.10 (#139) (which is blocked by magnum's mesh import rework)~~ no need to be that backwards compatible
- [ ] look into suppressing warnings from dependencies
- [x] glog has some (or go with #121 instead)
- [ ] assimp needs
-fvisibility-inlines-hiddento fixld: warning: direct access in function 'Assimp::IOSystem::CurrentDirectory() const' from file 'deps/magnum-plugins/src/MagnumPlugins/AssimpImporter/libAssimpImporter.a(AssimpImporter.cpp.o)' to global weak symbol 'Assimp::IOSystem::CurrentDirectory() const::Dummy' from file 'deps/assimp/code/libassimp.a(Importer.cpp.o)' means the weak symbol cannot be overri dden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
@mosra Can this be closed?
Why? There's still a few good-to-have items from the list that are not done yet. Feel free to tackle those, if you have time.
@mosra any more updates here?
Hmm. Do we have any other place to list such low-priority tech debt TODOs? In my case (with Magnum repos) I have a bunch of such long-lived "aggregate issues" full of checkboxes that act more as a reminder of what needs to be eventually solved, and I opened this one with a similar intent.
But if the aim here is to only have an issue open if it's likely to be resolved soon, feel free to close.
I'll leave this open for now until we establish such a list and then we can migrate the remainder there.