Skip to content

Commit

Permalink
Fix build failures with MSVC.
Browse files Browse the repository at this point in the history
In my Doom source-port, I use WildMIDI as a CMake subproject. When
running CMake for the first time, targetting MSVC, I do not set
`CMAKE_BUILD_TYPE` as MSVC uses a multi-configuration generator,
meaning that it should have no effect. However, if I make MSVC
attempt to compile a Release build, it will fail, claiming that the
WildMIDI binary could not be found. This seems to be caused by an odd
combination of things:

WildMIDI detects that `CMAKE_BUILD_TYPE` is not set, forcefully sets
it to 'Debug', and then sets `CMAKE_CONFIGURATION_TYPES` to match it,
which presumably disables every other build type for
multi-configuration generators.

This state-mutilation should be unnecessary, as the setting (or lack
thereof) of `CMAKE_BUILD_TYPE` should have no effect on
multi-configuration generators in the first place. Likewise, forcing
`CMAKE_BUILD_TYPE` to 'Debug' in the absense of an explicit value is
overbearing, as it is desirable to build software with no particular
build type, as it causes the minimum amount of compiler flags to be
automatically added by CMake. Notably, Arch Linux relies on this,
opting instead to manually provide its own compiler flags for
optimisations and hardening.

https://wiki.archlinux.org/title/CMake_package_guidelines#CMake_can_automatically_override_the_default_compiler_optimization_flag

Later on, `CMAKE_RUNTIME_OUTPUT_DIRECTORY` is redundantly set to the
`wildmidi_BINARY_DIR` variable. This causes the
automatically-generated per-configuration
`CMAKE_RUNTIME_OUTPUT_DIRECTORY_[CONFIG]` variables to become
undefined.

Finally, in an apparent attempt to counteract the above bug, the
per-configuration `CMAKE_RUNTIME_OUTPUT_DIRECTORY_[CONFIG]` variables
are manually redefined, using `CMAKE_CONFIGURATION_TYPES`. Because
`CMAKE_CONFIGURATION_TYPES` was forcefully set earlier, however, it
only contains 'Debug', causing only the Debug configuration to have a
valid output directory set. This causes all other configurations to
fail to produce a binary, resulting in a build failure as the binary
cannot be linked.

Personally, my rule-of-thumb is to keep CMake scripts as simple as
possible and trust that CMake has sane defaults. Seeing redundant
logic like this strikes me as a code smell, and this bug seems to
validate that.
  • Loading branch information
Clownacy committed Jan 28, 2024
1 parent ab9a671 commit b9a6dcf
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 18 deletions.
12 changes: 0 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,6 @@ INCLUDE(CheckIncludeFile)
INCLUDE(GNUInstallDirs)
INCLUDE(TestBigEndian)

# Set a default build type if none was specified
IF (NOT DEFINED CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "")
MESSAGE(STATUS "Setting build type to 'Debug' as none was specified.")
SET(CMAKE_BUILD_TYPE Debug CACHE STRING "Choose the type of build." FORCE)
# Set the possible values of build type for cmake-gui
SET_PROPERTY(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
ENDIF ()
SET(CMAKE_CONFIGURATION_TYPES "${CMAKE_BUILD_TYPE}")
MESSAGE(STATUS "Build Type: ${CMAKE_BUILD_TYPE}")

# Set our options
OPTION(BUILD_SHARED_LIBS "Build a dynamic wildmidi library" ON)
OPTION(WANT_PLAYER "Build WildMIDI player in addition to the libraries" ON)
Expand Down Expand Up @@ -270,8 +260,6 @@ ENDIF (APPLE)

IF (APPLE)
SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${APP_BUNDLE_DIR}/Contents/MacOS")
ELSE (APPLE)
SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${wildmidi_BINARY_DIR}")
ENDIF (APPLE)

# Setup up our config file
Expand Down
6 changes: 0 additions & 6 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,6 @@ IF (WIN32 AND MSVC)
SET(MT_BUILD "/MP")
ENDIF ()

foreach (OUTPUTCONFIG ${CMAKE_CONFIGURATION_TYPES})
STRING(TOUPPER ${OUTPUTCONFIG} OUTPUTCONFIG)
SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY_${OUTPUTCONFIG} "$(SolutionDir)$(Configuration)")
SET(CMAKE_LIBRARY_OUTPUT_DIRECTORY_${OUTPUTCONFIG} "$(ProjectDir)$(Configuration)")
endforeach (OUTPUTCONFIG)

IF (WANT_PLAYER)
# Release builds use the debug console
SET_TARGET_PROPERTIES(wildmidi PROPERTIES LINK_FLAGS_RELEASE "/SUBSYSTEM:CONSOLE")
Expand Down

0 comments on commit b9a6dcf

Please sign in to comment.