Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renamed exported targets and files #361

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

vrcomputing
Copy link
Contributor

@vrcomputing vrcomputing commented Jan 9, 2024

Relates to #359

  • QSkinny Internal CMake Target Names: lowercase
# src/CMakeLists.txt
set(target qskinny)

if(BUILD_QSKDLL)
    qsk_add_library(${target} SHARED ${SOURCES} ${HEADERS} ${PRIVATE_HEADERS})
else()
    qsk_add_library(${target} STATIC ${SOURCES} ${HEADERS} ${PRIVATE_HEADERS})
endif()
# qmlexport/CMakeLists.txt
set(target qskqmlexport)
qsk_add_library(${target} SHARED ${SOURCES} ${HEADERS})
  • QSkinny Exported CMake Target Names: UpperCase

Example CMake Filenames

QSkinnyConfig.cmake
QSkinnyConfigVersion.cmake
QSkinnyQmlExportTargets-debug.cmake
QSkinnyQmlExportTargets.cmake
QSkinnyTargets-debug.cmake
QSkinnyTargets.cmake
QSkinnyTools.cmake
  • QSkinny Exported CMake Namespace: QSkinny
# <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyTargets.cmake
...
add_library(QSkinny::QSkinny SHARED IMPORTED)
...
# <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyQmlExportTargets.cmake
...
add_library(QSkinny::QSkinnyQmlExport SHARED IMPORTED)
...

TODOs

  • fix/unify/clarify weird qskqmlexport.lib and qskqmlexport.dll installation

This state creates the following structure in the installation folder:

QSkinny Install Binary Directory

  • <CMAKE_INSTALL_PREFIX>/bin
  • <CMAKE_INSTALL_PREFIX>/bin/qskqmlexport.dll ❓ why not next to qskqmlexport.lib
  • <CMAKE_INSTALL_PREFIX>/bin/svg2qvg.exe

QSkinny Install Include Directories

  • <CMAKE_INSTALL_PREFIX>/include
  • <CMAKE_INSTALL_PREFIX>/include/qskinny
  • <CMAKE_INSTALL_PREFIX>/include/qskinny/QskAbstractButton.h
  • <CMAKE_INSTALL_PREFIX>/include/qskinny/QskQml.h
  • ...

QSkinny Install Library Directories

  • <CMAKE_INSTALL_PREFIX>/lib
  • <CMAKE_INSTALL_PREFIX>/lib/qskqmlexport.lib ❓ why not next to qskqmlexport.dll

QSkinny Install CMake Directories

  • <CMAKE_INSTALL_PREFIX>/lib/cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyConfig.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyConfigVersion.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyQmlExportTargets-debug.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyQmlExportTargets.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyTargets-debug.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyTargets.cmake
  • <CMAKE_INSTALL_PREFIX>/lib/cmake/QSkinny/QSkinnyTools.cmake

QSkinny Install Core Library Directory

  • <CMAKE_INSTALL_PREFIX>/lib/qskinny
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/qskinny.dll
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/qskinny.lib

QSkinny Install Plugin Directories

  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/platforminputcontexts
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/platforminputcontexts/qskinputcontext.dll
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/platforminputcontexts/qskinputcontext.lib

QSkinny Install Skin Directories

  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/fluent2skin.dll
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/fluent2skin.lib
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/material3skin.dll
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/material3skin.lib
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/squiekskin.dll
  • <CMAKE_INSTALL_PREFIX>/lib/qskinny/plugins/skins/squiekskin.lib

.gitignore Show resolved Hide resolved
@@ -1 +1,3 @@
include("${CMAKE_CURRENT_LIST_DIR}/QSkinnyTargets.cmake")
include("${CMAKE_CURRENT_LIST_DIR}/QSkinnyTools.cmake" OPTIONAL)
include("${CMAKE_CURRENT_LIST_DIR}/QSkinnyQmlExportTargets.cmake" OPTIONAL)
Copy link
Contributor Author

@vrcomputing vrcomputing Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include all possible CMake scripts and mark them as optional if e.g. controlled by a CMake option

VERSION ${PACKAGE_VERSION}
COMPATIBILITY AnyNewerVersion)

# Copy QSkinnyConfig.cmake to build dir
configure_file(${QSK_CMAKE_DIR}/${PACKAGE_NAME}Config.cmake
${CMAKE_BINARY_DIR}/_QSkinny/${PACKAGE_NAME}Config.cmake
${CMAKE_CURRENT_BINARY_DIR}/${PACKAGE_NAME}Config.cmake
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subjectively I don't think we need an additional sub directory in the CMAKE_CURRENT_BINARY_DIR since it is already QSkinny's own build folder. It's not wrong, just not necessary...

Using CMAKE_CURRENT_BINARY_DIR makes us support the "QSkinny as a sub folder" scenario

@lgtmak lgtmak mentioned this pull request Jan 10, 2024
@vrcomputing
Copy link
Contributor Author

@uwerat Besides the following weird placement this PR is ready for review/merging:

<CMAKE_INSTALL_PREFIX>/bin/qskqmlexport.dll ❓ why not next to qskqmlexport.lib
<CMAKE_INSTALL_PREFIX>/lib/qskqmlexport.lib ❓ why not next to qskqmlexport.dll

Is there a reason why <CMAKE_INSTALL_PREFIX>\lib\qskinny\qskinny.dll and <CMAKE_INSTALL_PREFIX>\lib\qskqmlexport.lib are in different directories?

Suggestion: All libraries in <CMAKE_INSTALL_PREFIX>\lib

  • <CMAKE_INSTALL_PREFIX>\lib\qskinny.dll
  • <CMAKE_INSTALL_PREFIX>\lib\qskinny.lib ( .lib for 🪟 s )
  • <CMAKE_INSTALL_PREFIX>\lib\qskqmlexport.dll
  • <CMAKE_INSTALL_PREFIX>\lib\qskqmlexport.lib ( .lib for 🪟 s )

@vrcomputing
Copy link
Contributor Author

vrcomputing commented Jan 10, 2024

PS: @uwerat is there a reason not to include .pdb in the package?

e.g. next to .lib / .dll files <CMAKE_INSTALL_PREFIX>\lib\qskinny.pdb
e.g. next to .lib / .dll files <CMAKE_INSTALL_PREFIX>\lib\qskqmlexport.pdb

@vrcomputing
Copy link
Contributor Author

PPS: we can also just put the

If we remove set(PACKAGE_NAME QSkinnyQmlExport) from qskinny\qmlexport\CMakeLists.txt change set_target_properties(${target} PROPERTIES EXPORT_NAME QSkinnyQmlExport) we can put all exported targets into the collective QSkinnyTargets.cmake e.g. <CMAKE_INSTALL_PREFIX>\lib\cmake\QSkinny\QSkinnyTargets-debug.cmake

# generated
...
# Import target "QSkinny::QSkinny" for configuration "Debug"
set_property(TARGET QSkinny::QSkinny APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
set_target_properties(QSkinny::QSkinny PROPERTIES
  IMPORTED_IMPLIB_DEBUG "${_IMPORT_PREFIX}/lib/qskinny/qskinny.lib"
  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/lib/qskinny/qskinny.dll"
  )

list(APPEND _cmake_import_check_targets QSkinny::QSkinny )
list(APPEND _cmake_import_check_files_for_QSkinny::QSkinny "${_IMPORT_PREFIX}/lib/qskinny/qskinny.lib" "${_IMPORT_PREFIX}/lib/qskinny/qskinny.dll" )

# Import target "QSkinny::QSkinnyQmlExport" for configuration "Debug"
set_property(TARGET QSkinny::QSkinnyQmlExport APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
set_target_properties(QSkinny::QSkinnyQmlExport PROPERTIES
  IMPORTED_IMPLIB_DEBUG "${_IMPORT_PREFIX}/lib/qskqmlexport.lib"
  IMPORTED_LINK_DEPENDENT_LIBRARIES_DEBUG "QSkinny::QSkinny"
  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/bin/qskqmlexport.dll"
  )

list(APPEND _cmake_import_check_targets QSkinny::QSkinnyQmlExport )
list(APPEND _cmake_import_check_files_for_QSkinny::QSkinnyQmlExport "${_IMPORT_PREFIX}/lib/qskqmlexport.lib" "${_IMPORT_PREFIX}/bin/qskqmlexport.dll" )
...

@uwerat uwerat merged commit 690a3c5 into uwerat:master Jan 11, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants