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.

Sign up for GitHub

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

[vcpkg_fixup_cmake_targets] Add NO_PREFIX_CORRECTION #12215

Merged
merged 6 commits into from
Nov 25, 2020
Merged

[vcpkg_fixup_cmake_targets] Add NO_PREFIX_CORRECTION #12215

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f9df473
[vcpkg/scripts/fixup_cmake] add option NO_PREFIX_CORRECTION to not ap…
Neumann-A Jul 2, 2020
7512554
add a bit of explanation why the prefix correction might be incorrect.
Neumann-A Jul 2, 2020
acdf9c2
apply the if only around the import prefix changes
Neumann-A Jul 2, 2020
cac4ea5
Update scripts/cmake/vcpkg_fixup_cmake_targets.cmake
Neumann-A Jul 14, 2020
36bdd02
Merge remote-tracking branch 'upstream/master' into add_option_disabl…
Neumann-A Aug 8, 2020
ce4c9d7
Merge remote-tracking branch 'origin/master' into HEAD
BillyONeal Nov 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions scripts/cmake/vcpkg_fixup_cmake_targets.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
## Passing this option disable such behavior, as it is convenient for ports that install
## more than one CMake package configuration file.
##
## ### NO_PREFIX_CORRECTION
## Disables the correction of_IMPORT_PREFIX done by vcpkg due to moving the targets.
## Currently the correction does not take into account how the files are moved and applies
## I rather simply correction which in some cases will yield the wrong results.
##
## ## Notes
## Transform all `/debug/<CONFIG_PATH>/*targets-debug.cmake` files and move them to `/<TARGET_PATH>`.
## Removes all `/debug/<CONFIG_PATH>/*targets.cmake` and `/debug/<CONFIG_PATH>/*config.cmake`.
Expand All @@ -43,7 +48,7 @@
## * [curl](https://github.com/Microsoft/vcpkg/blob/master/ports/curl/portfile.cmake)
## * [nlohmann-json](https://github.com/Microsoft/vcpkg/blob/master/ports/nlohmann-json/portfile.cmake)
function(vcpkg_fixup_cmake_targets)
cmake_parse_arguments(_vfct "DO_NOT_DELETE_PARENT_CONFIG_PATH" "CONFIG_PATH;TARGET_PATH" "" ${ARGN})
cmake_parse_arguments(_vfct "DO_NOT_DELETE_PARENT_CONFIG_PATH;NO_PREFIX_CORRECTION" "CONFIG_PATH;TARGET_PATH" "" ${ARGN})

if(_vfct_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "vcpkg_fixup_cmake_targets was passed extra arguments: ${_vfct_UNPARSED_ARGUMENTS}")
Expand Down Expand Up @@ -159,21 +164,40 @@ function(vcpkg_fixup_cmake_targets)
#Fix ${_IMPORT_PREFIX} in cmake generated targets and configs;
#Since those can be renamed we have to check in every *.cmake
file(GLOB_RECURSE MAIN_CMAKES "${RELEASE_SHARE}/*.cmake")

foreach(MAIN_CMAKE IN LISTS MAIN_CMAKES)
file(READ ${MAIN_CMAKE} _contents)
string(REGEX REPLACE
"get_filename_component\\(_IMPORT_PREFIX \"\\\${CMAKE_CURRENT_LIST_FILE}\" PATH\\)(\nget_filename_component\\(_IMPORT_PREFIX \"\\\${_IMPORT_PREFIX}\" PATH\\))*"
"get_filename_component(_IMPORT_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)\nget_filename_component(_IMPORT_PREFIX \"\${_IMPORT_PREFIX}\" PATH)\nget_filename_component(_IMPORT_PREFIX \"\${_IMPORT_PREFIX}\" PATH)"
_contents "${_contents}") # see #1044 for details why this replacement is necessary. See #4782 why it must be a regex.
string(REGEX REPLACE
"get_filename_component\\(PACKAGE_PREFIX_DIR \"\\\${CMAKE_CURRENT_LIST_DIR}/\\.\\./(\\.\\./)*\" ABSOLUTE\\)"
"get_filename_component(PACKAGE_PREFIX_DIR \"\${CMAKE_CURRENT_LIST_DIR}/../../\" ABSOLUTE)"
_contents "${_contents}")
string(REGEX REPLACE
"get_filename_component\\(PACKAGE_PREFIX_DIR \"\\\${CMAKE_CURRENT_LIST_DIR}/\\.\\.((\\\\|/)\\.\\.)*\" ABSOLUTE\\)"
"get_filename_component(PACKAGE_PREFIX_DIR \"\${CMAKE_CURRENT_LIST_DIR}/../../\" ABSOLUTE)"
_contents "${_contents}") # This is a meson-related workaround, see https://github.com/mesonbuild/meson/issues/6955
#This correction is not correct for all cases. To make it correct for all cases it needs to consider
#original folder deepness to CURRENT_PACKAGES_DIR in comparison to the moved to folder deepness which
#is always at least (>=) 2, e.g. share/${PORT}. Currently the code assumes it is always 2 although
#this requirement is only true for the *Config.cmake. The targets are not required to be in the same
#folder as the *Config.cmake!
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it would be best to fix this to handle the targets case instead of adding a switch to disable and then needing to manually write the same fixup code in every port that uses said switch. Do you have an idea of how difficult that would be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calculate relative path from CURRENT_PACKAGES_DIR/<targetfilelocation> to CURRENT_PACKAGES_DIR and count the backticks /.. do the same for CURRENT_PACKAGES_DIR/<newtargetfilelocation> to CURRENT_PACKAGES_DIR and also count the /...

The first count gives the number of get_filename_component to replace while the later defines the number of get_filename_component required. If the count is equal no replacement is necessary. It is not too difficult but probably requires a ton of try and error to just get it to work correctly.

I just added NO_PREFIX_CORRECTION as a quick fix since I need it for one of my personal ports. You can always deprecate the parameter later if you have the above mentioned fix. (And probably not a lot of ports require it.)

if(NOT _vfct_NO_PREFIX_CORRECTION)
string(REGEX REPLACE
"get_filename_component\\(_IMPORT_PREFIX \"\\\${CMAKE_CURRENT_LIST_FILE}\" PATH\\)(\nget_filename_component\\(_IMPORT_PREFIX \"\\\${_IMPORT_PREFIX}\" PATH\\))*"
"get_filename_component(_IMPORT_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)\nget_filename_component(_IMPORT_PREFIX \"\${_IMPORT_PREFIX}\" PATH)\nget_filename_component(_IMPORT_PREFIX \"\${_IMPORT_PREFIX}\" PATH)"
_contents "${_contents}") # see #1044 for details why this replacement is necessary. See #4782 why it must be a regex.
string(REGEX REPLACE
"get_filename_component\\(PACKAGE_PREFIX_DIR \"\\\${CMAKE_CURRENT_LIST_DIR}/\\.\\./(\\.\\./)*\" ABSOLUTE\\)"
"get_filename_component(PACKAGE_PREFIX_DIR \"\${CMAKE_CURRENT_LIST_DIR}/../../\" ABSOLUTE)"
_contents "${_contents}")
string(REGEX REPLACE
"get_filename_component\\(PACKAGE_PREFIX_DIR \"\\\${CMAKE_CURRENT_LIST_DIR}/\\.\\.((\\\\|/)\\.\\.)*\" ABSOLUTE\\)"
"get_filename_component(PACKAGE_PREFIX_DIR \"\${CMAKE_CURRENT_LIST_DIR}/../../\" ABSOLUTE)"
_contents "${_contents}") # This is a meson-related workaround, see https://github.com/mesonbuild/meson/issues/6955
endif()

#Fix wrongly absolute paths to install dir with the correct dir using ${_IMPORT_PREFIX}
#This happens if vcpkg built libraries are directly linked to a target instead of using
#an imported target for it. We could add more logic here to identify defect target files.
#Since the replacement here in a multi config build always requires a generator expression
#in front of the absoulte path to ${CURRENT_INSTALLED_DIR}. So the match should always be at
#least >:${CURRENT_INSTALLED_DIR}.
#In general the following generator expressions should be there:
#\$<\$<CONFIG:DEBUG>:${CURRENT_INSTALLED_DIR}/debug/lib/somelib>
#and/or
#\$<\$<NOT:\$<CONFIG:DEBUG>>:${CURRENT_INSTALLED_DIR}/lib/somelib>
#with ${CURRENT_INSTALLED_DIR} being fully expanded
string(REPLACE "${CURRENT_INSTALLED_DIR}" [[${_IMPORT_PREFIX}]] _contents "${_contents}")
file(WRITE ${MAIN_CMAKE} "${_contents}")
endforeach()
Expand Down