QoL: Buildtype variables for looping over buildtypes #7733
Conversation
|
Thanks for the PR! This in principle seems like it could be a useful abstraction, but I'd like to see it demonstrated in multiple contexts before merging. Specifically, if it turns out to only be useful in Qt5, then it should be private to Qt5. However, if it can be used to make code significantly cleaner and clearer in more contexts, I'd like to see this PR introduce at least two of those refactorings. |
|
TODO (only 2 of N):
|
# Conflicts: # scripts/cmake/vcpkg_common_definitions.cmake
|
Please resolve these conflicts. Thanks. |
# Conflicts: # ports/qt5-base/CONTROL # ports/qt5-base/cmake/configure_qt.cmake # ports/qt5-base/cmake/install_qt.cmake # scripts/cmake/vcpkg_build_cmake.cmake # scripts/cmake/vcpkg_configure_cmake.cmake # scripts/cmake/vcpkg_configure_qmake.cmake
|
Pinging @Neumann-A for response. Is work still being done for this PR? |
|
@JackBoosY: You put the PR back into Draft?!?! I consider it finished for the time unless there is something which needs to ne fixed which I don't know of or you want some changes to it. Maybe run a full rebuild of everything to check if everything is working as intended. |
# Conflicts: # scripts/cmake/vcpkg_common_definitions.cmake
# Conflicts: # scripts/cmake/vcpkg_configure_cmake.cmake # scripts/cmake/vcpkg_configure_qmake.cmake
|
|
JackBoosY
added
the
depends:different-pr
strega-nil
added
requires:discussion
and removed
depends:different-pr
There was a problem hiding this comment.
Thanks @Neumann-A, just some minor last changes and a few removals/deletions.
| ## VCPKG_BUILD_LIST List of VCPKG_BUILD_TYPE which the current port will build (uppercase) | ||
| ## VCPKG_BUILD_SHORT_NAME_<BUILDTYPE> Short name of the buildtype (e.g. DEBUG=dbg; RELEASE=rel) | ||
| ## VCPKG_BUILD_CMAKE_TYPE_<BUILDTYPE> CMAKE_BUILD_TYPE used for buildtype | ||
| ## VCPKG_BUILD_QMAKE_CONFIG_<BUILDTYPE> Required QMAKE CONFIG flags for buildtype | ||
| ## VCPKG_PATH_SUFFIX_<BUILDTYPE> Path suffix used for buildtype (e.g. /debug) | ||
| ## VCPKG_BUILD_TRIPLET_<BUILDTYPE> Fullname of the buildtriplet e.g. ${TARGET_TRIPLET}-${VCPKG_BUILD_SHORT_NAME_<BUILDTYPE>} | ||
| ## VCPKG_BUILDTREE_TRIPLET_DIR_<BUILDTYPE> Path to current buildtype buildtree (e.g. CURRENT_BUILDTREES_DIR/TRIPLET-rel ) |
There was a problem hiding this comment.
In discussion, we have agreed on the following things:
- We probably shouldn't define
VCPKG_BUILD_CMAKE_TYPE_*andVCPKG_BUILD_QMAKE_CONFIG_*, since they're build-system specific. - We should remove
VCPKG_BUILDTREE_TRIPLET_DIR_*, since that's not a concept we want to codify. - We need to rename these:
VCPKG_BUILD_LIST->VCPKG_BUILD_TYPESVCPKG_BUILD_SHORT_NAME_*->VCPKG_BUILD_TYPE_SHORT_NAME_*VCPKG_BUILD_TRIPLET_*->VCPKG_LOGNAME_STEM_*or something similar
VCPKG_PATH_SUFFIX_* is good as it is.
Additionally, we want to lowercase the build types in these, so that VCPKG_BUILD_TYPES becomes release;debug,
and the different variables are VCPKG_PATH_SUFFIX_release, VCPKG_PATH_SUFFIX_debug, etc.
There was a problem hiding this comment.
Please re-discuss the following or give arguments:
Additionally, we want to lowercase the build types in these
uppercase is the way to go since cmake variables itself are uppercase. If I don't use uppercase build types I always have to use a string(UPPER) for VCPKG_DETECTED_<SOMELANGUAGE>_FLAGS_<SOMEBUILDTYPE_UPPER> . I don't see any good reason to stick to lowercase build types here. (I mean I can also introduce VCPKG_BUILD_TYPE_UPPER_<config> if you plan to stick to lowercase configs but it will get complicated to access flags: MY_C_FLAGS_${VCPKG_BUILD_TYPE_UPPER_${config}} instead of MY_C_FLAGS_${config})
We should remove VCPKG_BUILDTREE_TRIPLET_DIR_*, since that's not a concept we want to codify.
The path is used so often and even inside portfiles. I can rename it to _VCPKG_INTERNAL_BUILDTREE_TRIPLET_DIR_
VCPKG_BUILD_TRIPLET_* -> VCPKG_LOGNAME_STEM_* or something similar
although often used for the logname it is not only used there. It is also used in messages and paths/folders.
We probably shouldn't define VCPKG_BUILD_CMAKE_TYPE_* and VCPKG_BUILD_QMAKE_CONFIG_*, since they're build-system specific.
TODO: I'll move them to the configure scripts.
There was a problem hiding this comment.
The thing is, release and debug aren't necessarily related to the CMake stuff, they're related to the vcpkg BUILD_TYPE stuff; even though the words are the same between VCPKG_BUILD_TYPE = release and CMAKE_BUILD_TYPE = Release, they're not necessarily the same thing, and we shouldn't use RELEASE just because CMake does for a related but different concept. It's important to understand that our usecase of CMake is not as a build system -- we are simply using CMake as a language for writing descriptions of existing build systems.
The big reason we want to do this is we wish to tie this to VCPKG_BUILD_TYPE. I think this would be a really unfortunate and weird feature if it weren't tied to VCPKG_BUILD_TYPE.
add VCPKG_COPY_EXE to 0 in the debug case.
rename VCPKG_BUILD_SHORT_NAME_ to VCPKG_BUILD_TYPE_SHORT_NAME_ move VCPKG_BUILD_CMAKE_TYPE_ and VCPKG_BUILD_QMAKE_CONFIG_ into the the appropiate functions
# Conflicts: # scripts/cmake/vcpkg_configure_cmake.cmake
| ## VCPKG_BUILD_CMAKE_TYPE_<BUILDTYPE> CMAKE_BUILD_TYPE used for buildtype | ||
| ## VCPKG_BUILD_QMAKE_CONFIG_<BUILDTYPE> Required QMAKE CONFIG flags for buildtype |
There was a problem hiding this comment.
| ## VCPKG_BUILD_CMAKE_TYPE_<BUILDTYPE> CMAKE_BUILD_TYPE used for buildtype | |
| ## VCPKG_BUILD_QMAKE_CONFIG_<BUILDTYPE> Required QMAKE CONFIG flags for buildtype |
|
Closing this PR since it seems that no progress is being made. Please reopen if work is still being done. |
While working on the qt scripts I often defined those variables below to avoid unnecessary code duplication to loop over the different buildtypes
TODO:
VCPKG_BUILD_TRIPLET_*to something different ?VCPKG_BUILDTREE_TRIPLET_DIR_?