CMake fix in install rules about playrho HOT 6 CLOSED

Hi.
This is an actual problem I've run into.
I wanted to use PlayRho with Vcpkg, so I've created a portfile.
But Vcpkg was reporting errors during installation about include directories (COMPONENT and Library) that don't exist.
This is when I found out that order of the CMake "keywords" in install() matters.

My Vcpkg "port" looks like this (I've took one for box2d as an example to create it)

{
  "name": "playrho",
  "version-semver": "1.1.1",
  "port-version": 1,
  "description": "Real-time oriented physics engine and library that's currently best suited for 2D games. ",
  "homepage": "https://louis-langholtz.github.io/PlayRho/",
  "supports": "!uwp",
  "dependencies": [
    {
      "name": "vcpkg-cmake",
      "host": true
    },
    {
      "name": "vcpkg-cmake-config",
      "host": true
    }
  ]
}

vcpkg_check_linkage(ONLY_STATIC_LIBRARY)

vcpkg_from_github(
    OUT_SOURCE_PATH SOURCE_PATH
    REPO louis-langholtz/PlayRho
    REF fd50d14d747d892e26d8d445e123d65b4f37c161  #v1.1.1
    SHA512 25a7b403a34aac22e83cc46aff796f39e92f2124e5e3c0ed5a8ba479ce840fba592b7b0216b142e51fb0b4b3c5e33770d0535f78467499e4804717be668c2082
    HEAD_REF master
)

vcpkg_cmake_configure(
    SOURCE_PATH "${SOURCE_PATH}"
    OPTIONS
        -DPLAYRHO_INSTALL=ON
        -DPLAYRHO_BUILD_HELLOWORLD=OFF
        -DPLAYRHO_BUILD_UNIT_TESTS=OFF
        -DPLAYRHO_BUILD_BENCHMARK=OFF
        -DPLAYRHO_BUILD_TESTBED=OFF
)
vcpkg_cmake_install()

file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/playrho)

vcpkg_copy_pdbs()

file(INSTALL "${SOURCE_PATH}/LICENSE.txt" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)

from playrho.

@Zbyl Thank you for your interest in PlayRho and for filing this issue!

Looking at the latest CMake Installing Targets help, I see the following usage specification:

install(TARGETS targets... [EXPORT <export-name>]
        [RUNTIME_DEPENDENCIES args...|RUNTIME_DEPENDENCY_SET <set-name>]
        [[ARCHIVE|LIBRARY|RUNTIME|OBJECTS|FRAMEWORK|BUNDLE|
          PRIVATE_HEADER|PUBLIC_HEADER|RESOURCE|FILE_SET <set-name>|CXX_MODULES_BMI]
         [DESTINATION <dir>]
         [PERMISSIONS permissions...]
         [CONFIGURATIONS [Debug|Release|...]]
         [COMPONENT <component>]
         [NAMELINK_COMPONENT <component>]
         [OPTIONAL] [EXCLUDE_FROM_ALL]
         [NAMELINK_ONLY|NAMELINK_SKIP]
        ] [...]
        [INCLUDES DESTINATION [<dir> ...]]
        )

Reading this now and the issue you had linked, I'm recognizing how having the line

COMPONENT Library

after the

INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}

line (found in the PlayRho/PlayRho/CMakeLists.txt file) appears riskier.

I imagine this risk depends though on just how order dependent CMake is, and how greedily CMake evaluates arguments like <dir> ... - versus keywords. At least based on the above usage however, seems like COMPONENT is better listed before INCLUDES DESTINATION as that seems to be closer to the order suggested. Which seems to suggest:

install(TARGETS PlayRho
	EXPORT PlayRho-targets
	COMPONENT Library
	LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
	ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
	RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
	INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
	)

Are you actually running into a problem with this, or just reporting this riskiness?

I ask in part because none of my manual testing had detected this being a problem. CMake seems too fragile to me however and the usage specification suggests it could be a problem. I'd like to see the parse tree CMake builds out of this CMakeLists.txt file but the best I'm finding is the output of CMake with --trace-expand.

from playrho.

Need to also back port changes from #488 into release-1.1.

from playrho.

@Zbyl Thank you again. That's very appreciated insight & your vcpkg port is also super appreciated!

I have merged a change to master to address this. I intend to back port this next to the release 1.1 branch and associated release.

from playrho.

v1.1.2 has been released now with the change in it to address this. Hopefully it fixes this problem and doesn't introduce any new ones. Please let me know what your experience with it is. Thanks again.

from playrho.

Gonna assume this is fixed now.

from playrho.