lenmus / lomse

A C++ library for rendering, editing and playing back music scores.
MIT License
117 stars 28 forks source link

Support include through CMake Package Manager + Fix recent breaking change in freetype #400

Closed lancelotblanchard closed 2 weeks ago

lancelotblanchard commented 3 weeks ago

2 small fixes:

cecilios commented 3 weeks ago

Thank you for the PR.

Using PROJECT_SOURCE_DIR instead of CMAKE_SOURCE_DIR seems like a good idea but it is required to investigate why the automatic checks fail to build. For instance in AppVeyor:

cd build
cmake -DLOMSE_ENABLE_COMPRESSION=OFF -DLOMSE_ENABLE_PNG=OFF  -DFREETYPE_INCLUDE_DIRS=C:\projects\ExternalLibraries\freetype\include -DFREETYPE_LIBRARY=C:\projects\ExternalLibraries\freetype\win32\freetype.lib -DUNITTEST++_INCLUDE_DIR=C:\projects\ExternalLibraries\unittest-cpp-2.0.0\UnitTest++ -DUNITTEST++_LIBRARY=C:\projects\ExternalLibraries\unittest-cpp-2.0.0\Debug\UnitTest++.lib -DLOMSE_RUN_TESTS=OFF ../lomse
-- Building for: Visual Studio 14 2015
************** ERROR *************************************
  Lomse should not be configured & built in the Lomse
  source directory. You must run cmake in a build directory.
  For example:
      mkdir build; cd build; cmake ..
  NOTE: Given that you already tried to make an in-source
  build CMake have already created several files & directories
  in your source tree. Remove them by doing:
      rm -rf CMakeCache.txt CMakeFiles
**********************************************************
CMake Error at CMakeLists.txt:233 (message):
  Quitting configuration

And the same error in Linux builds.

Is this failure associated to the PROJET_SOURCE_DIR change?

As to the second patch to fix the recent breaking change in freetype, I'm not sure how to proceed, because if the proposed patch is applied, the lomse build would fail on machines with a FreeType package from a version prior to the breaking change, and if your patch is not applied, it would fail on machines that have a later version. Would it be necessary to include some preprocessor variable to choose between unsigned char or char depending on the FreeType version installed?

lancelotblanchard commented 2 weeks ago

My bad, this error was caused by the fact that the check for the build location was done before the name of the project was set, which meant that ${PROJECT_SOURCE_DIR} was not set yet. I just fixed that.

You're completely right about supporting earlier versions of freetype. I included a build time check of FREETYPE_MAJOR, FREETYPE_MINOR, and FREETYPE_PATCH to apply the patch accordingly.

cecilios commented 2 weeks ago

Thank you. Regression test also passed so I will merge your PR