CMake Config forces you to install conan packages

jmarrec commented 2 years ago

Issue overview

https://github.com/NREL/OpenStudio/blob/f3fba80d69e8fd6f0d9352951ab967c0bacda39a/src/lib/CMakeLists.txt#L23-L31

If I try to link to openstudio::openstudiolib in an external project, I get the following error (repeated for each of these)

CMake Error at CMakeLists.txt:219 (add_executable):
  Target "Model_Benchmark" links to target "CONAN_PKG::boost" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?

There's no reason to force users of openstudiolib to define these targets.

Indicentally, https://github.com/conan-io/cmake-conan used to have conan_cmake_run (which we use currently) but that's now considered deprecated, so if you use the example they provide it uses the cmake_find_package generator and not the cmake one (which is more intrusive), in which case you don't have any CONAN_PKG::xxxx targets. Instead of you would do a call to find_package(Boost) for eg.

Environment

Some additional details about your environment for this issue (if relevant):

Platform (Operating system, version): All
Version of OpenStudio (if using an intermediate build, include SHA): 3.3.0

Context

Toying with the idea of putting the microbenchmark files into a separate repo, so we could like to whichever installer we wanted.

jmarrec commented 2 years ago

I think they may be required after all. I commented out the INTERFACE_LINK_LIBRARIES in the generated openstudioConfig.cmake and I am getting errors when building my project

openstudioConfig.cmake

set_target_properties(openstudio::openstudiolib PROPERTIES
  INTERFACE_COMPILE_DEFINITIONS "SHARED_OS_LIBS"
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include;${_IMPORT_PREFIX}/include/openstudio"
  # INTERFACE_LINK_LIBRARIES "CONAN_PKG::boost;CONAN_PKG::jsoncpp;CONAN_PKG::cpprestsdk;CONAN_PKG::openssl;CONAN_PKG::pugixml"
)

FAILED: CMakeFiles/Model_Benchmark.dir/model/Model_Benchmark.cpp.o 
/usr/bin/ccache /usr/bin/c++ -DSHARED_OS_LIBS -D_GLIBCXX_USE_CXX11_ABI=1 -I/media/DataExt4/Software/Others/OpenStudio-benchmarks/build -isystem /home/julien/.conan/data/benchmark/1.5.2/_/_/package/c37768999b7e7b6e32039a984f1ef6b7d434478b/include -isystem /home/julien/.conan/data/boost/1.73.0/_/_/package/ece573b740a59a65813e4d3a9017e6b2c18c57a3/include -isystem /home/julien/.conan/data/zlib/1.2.11/_/_/package/19729b9559f3ae196cad45cb2b97468ccb75dcd1/include -isystem /home/julien/.conan/data/bzip2/1.0.8/_/_/package/91a8b22c2c5a149bc617cfc06cdd21bf23b12567/include -isystem /media/DataExt4/Software/Others/OpenStudio-benchmarks/build/OpenStudio-3.3.0/OpenStudio-3.3.0+ad235ff36e-Ubuntu-20.04/usr/local/openstudio-3.3.0/include -isystem /media/DataExt4/Software/Others/OpenStudio-benchmarks/build/OpenStudio-3.3.0/OpenStudio-3.3.0+ad235ff36e-Ubuntu-20.04/usr/local/openstudio-3.3.0/include/openstudio -fdiagnostics-color=always  -fPIC -fno-strict-aliasing -Winvalid-pch -Wall -Wextra -Werror -pedantic-errors -pedantic -Wno-overloaded-virtual -Wno-maybe-uninitialized -O3 -DNDEBUG -std=c++2a -MD -MT CMakeFiles/Model_Benchmark.dir/model/Model_Benchmark.cpp.o -MF CMakeFiles/Model_Benchmark.dir/model/Model_Benchmark.cpp.o.d -o CMakeFiles/Model_Benchmark.dir/model/Model_Benchmark.cpp.o -c /media/DataExt4/Software/Others/OpenStudio-benchmarks/model/Model_Benchmark.cpp
In file included from /media/DataExt4/Software/Others/OpenStudio-benchmarks/build/OpenStudio-3.3.0/OpenStudio-3.3.0+ad235ff36e-Ubuntu-20.04/usr/local/openstudio-3.3.0/include/openstudio/model/Model.hpp:37,
                 from /media/DataExt4/Software/Others/OpenStudio-benchmarks/model/Model_Benchmark.cpp:3:
/media/DataExt4/Software/Others/OpenStudio-benchmarks/build/OpenStudio-3.3.0/OpenStudio-3.3.0+ad235ff36e-Ubuntu-20.04/usr/local/openstudio-3.3.0/include/openstudio/utilities/filetypes/WorkflowJSON.hpp:39:10: fatal error: json/json.h: No such file or directory
   39 | #include <json/json.h>
      |          ^~~~~~~~~~~~~

kbenne commented 1 year ago

@jmarrec where do we stand on this issue? It seems to me that the libraries named as INTERFACE_LINK_LIBRARIES are indeed necessary due to includes in OpenStudio's API header files. I feel like we've never done a great job at clearly identiying which header files / APIs really need to be made public, but since basically all of the headers are public, it seems like we almost have to defensively include these interface libraries.

jmarrec commented 1 year ago

@kbenne I think we could try to not put them in a public header. For eg: https://github.com/NREL/OpenStudio/blob/1c6fe48c49987c16e95e90ee3bd088ad0649ab9c/src/utilities/filetypes/WorkflowJSON.hpp#L39 this is uncessary here, we don't have a Json::Value member anywhere. In some cases, having these includes in an _Impl.hpp header will be mandatory, in which case it depends if the user MUST include that _Impl.hpp (in which case they do need to provide the dependency) for reasons such as casting, or not.

NREL / OpenStudio