Closed JohannesKauffmann closed 9 months ago
CMAKE_BUILD_TYPE is only used in single-configuration generators and may be empty. Setting it to something when it's not needed can cause strange behavior, which is detailed here:
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-configurations
That said, a user should always specify the configuration to build, so not installing the files in such a situation isn't really an issue since the build simply doesn't know what type of output to produce (it rather depends on the toolset defaults).
If anything, adding a note to the build documentation should suffice IMO.
Also checked that -DPROJECTM_STATIC_DEFINE
appears in all .pc files for Release and Debug configurations and all libs, which it does.
Describe the bugs:
Static library name and
Libs:
section for static buildWhen building shared, the library file is called
libprojectM-4.so
, while building static, the library file is calledlibprojectM.a
, instead oflibprojectM-4.a
. This seems to be caused by this line. Maybe this is intended, I'm not sure.However, that same CMake code also causes the
Libs:
section of the generated .pc files to contain the literal unexpanded CMake generator expression when building statically:... while it should contain
-l:projectM -lOpenGL
or similar. This does not happen when building shared and obviously breaks the .pc file. Removing theset_target_properties
call fixes the problem, but I'm not sure that's desirable as it seems to be introduced recently.The playlist library .pc file also suffers from this problem, caused by similar code.
Cflags:
section in .pc file when building staticSince bff9e52c6992b82fbd31bf83409af2360f8b5225, the
PROJECTM_STATIC_DEFINE
define was moved from atarget_compile_definitions
call to multipleset_source_files_properties
calls. This has the side effect that-DPROJECTM_STATIC_DEFINE
no longer ends up in theCflags:
section of the main projectM .pc file when building static.Conversely, the .pc file for the playlist library does contain
-DPROJECTM_STATIC_DEFINE
, precisely because it usestarget_compile_definitions
here.Side note: To me, it looks like the playlist library already links against the main projectM library (here), so this line looks unneeded: If the main library correctly specifies the static define as part of it's public interface (which it currently doesn't as explained above) and the playlist library publicly links to the main projectM library (which AFAICT it does), surely the playlist library will inherit and use that flag itself without having to specify it twice.
Finally, maybe a nitpick, but it looks like no .pc files are getting installed when no
CMAKE_BUILD_TYPE
is specified. Maybe it would be a good idea to default to either one of the four configurations (Debug, RelWithDebInfo, etc) when none is specified?Desktop: