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_pkgconfig] Check for more problems, add unit test #23898

Merged
merged 31 commits into from
Jan 20, 2023
Merged

[vcpkg_fixup_pkgconfig] Check for more problems, add unit test #23898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9ed5937
Print stack traces for failed unit tests
dg0yt Jul 2, 2022
5cc0072
Add test for vcpkg_fixup_pkgconfig.cmake
dg0yt Jul 2, 2022
470157c
Check for 'optimized' and 'debug' in pc files
dg0yt Mar 31, 2022
4100f17
Check for 'NOTFOUND' and for 'ns::target' in pc files
dg0yt Jul 2, 2022
49212b2
Check for, and fix, line continuations
dg0yt Jul 2, 2022
df6f595
Test file path substitutions
dg0yt Jul 3, 2022
250d3d6
Move contents processing into separate function
dg0yt Jul 3, 2022
80c4b04
Update and leverage line break normalization
dg0yt Jul 3, 2022
4103882
Pass prefix to data processing
dg0yt Jul 3, 2022
eabfabe
Validate only the collapsed Libs
dg0yt Jul 4, 2022
85e10cf
Test removal of '<field>.private'
dg0yt Jul 4, 2022
062b870
Declare unit test license (same as vcpkg)
dg0yt Jul 23, 2022
bf6373d
Replace ';' with ' ' in 'Libs:'
dg0yt Jul 24, 2022
0c44306
Disambiguate parameter variable names
dg0yt Jul 24, 2022
d75ce53
Test quoting of variables
dg0yt Jul 24, 2022
0684e8f
Fix quoting of variables
dg0yt Jul 24, 2022
b7aa0dc
Quote whole parameters
dg0yt Jul 24, 2022
a4cae5a
Process and quote libs item-wise
dg0yt Jul 24, 2022
547e832
Resolve keywords 'optimized', 'debug', 'debug'
dg0yt Jul 24, 2022
a64def1
Consistency
dg0yt Jul 24, 2022
636c4a5
Merge remote-tracking branch 'origin/master'
dg0yt Aug 24, 2022
effbd2f
CI [skip actions]
dg0yt Oct 15, 2022
a46d4ca
Merge remote-tracking branch 'origin/master' into HEAD
BillyONeal Oct 26, 2022
c6d1157
Merge remote-tracking branch 'origin/master' into fixup-pkgconfig
dg0yt Nov 13, 2022
28c95a9
CI [skip actions]
dg0yt Nov 17, 2022
7dfbec6
Don't fail on detected errors
dg0yt Nov 19, 2022
74752e2
Disable unit-testing for fatal errors
dg0yt Nov 20, 2022
691e40f
Merge branch 'microsoft:master' into fixup-pkgconfig
dg0yt Dec 17, 2022
e8f5ae6
Merge branch 'microsoft:master' into fixup-pkgconfig
dg0yt Dec 31, 2022
3e91f2e
Merge branch 'microsoft:master' into fixup-pkgconfig
dg0yt Jan 5, 2023
261671c
Merge branch 'microsoft:master' into fixup-pkgconfig
dg0yt Jan 12, 2023
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
176 changes: 111 additions & 65 deletions scripts/cmake/vcpkg_fixup_pkgconfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,108 @@
function(z_vcpkg_fixup_pkgconfig_check_files file config)
function(z_vcpkg_fixup_pkgconfig_process_data arg_variable arg_config arg_prefix)
# This normalizes all data to start and to end with a newline, and
# to use LF instead of CRLF. This allows to use simpler regex matches.
string(REPLACE "\r\n" "\n" contents "\n${${arg_variable}}\n")

string(REPLACE "${CURRENT_PACKAGES_DIR}" [[${prefix}]] contents "${contents}")
string(REPLACE "${CURRENT_INSTALLED_DIR}" [[${prefix}]] contents "${contents}")
if(VCPKG_HOST_IS_WINDOWS)
string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_packages_dir "${CURRENT_PACKAGES_DIR}")
string(REPLACE "${unix_packages_dir}" [[${prefix}]] contents "${contents}")
string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_installed_dir "${CURRENT_INSTALLED_DIR}")
string(REPLACE "${unix_installed_dir}" [[${prefix}]] contents "${contents}")
endif()

string(REGEX REPLACE "\n[\t ]*prefix[\t ]*=[^\n]*" "" contents "prefix=${arg_prefix}${contents}")
if("${arg_config}" STREQUAL "DEBUG")
# prefix points at the debug subfolder
string(REPLACE [[${prefix}/debug]] [[${prefix}]] contents "${contents}")
string(REPLACE [[${prefix}/include]] [[${prefix}/../include]] contents "${contents}")
string(REPLACE [[${prefix}/share]] [[${prefix}/../share]] contents "${contents}")
endif()
# Remove line continuations before transformations
string(REGEX REPLACE "[ \t]*\\\\\n[ \t]*" " " contents "${contents}")
# This section fuses XYZ.private and XYZ according to VCPKG_LIBRARY_LINKAGE
#
# Pkgconfig searches Requires.private transitively for Cflags in the dynamic case,
# which prevents us from removing it.
#
# Once this transformation is complete, users of vcpkg should never need to pass
# --static.
if("${VCPKG_LIBRARY_LINKAGE}" STREQUAL "static")
# how this works:
# we want to transform:
# Libs: $1
# Libs.private: $2
# into
# Libs: $1 $2
# and the same thing for Requires and Requires.private

foreach(item IN ITEMS "Libs" "Requires" "Cflags")
set(line "")
if("${contents}" MATCHES "\n${item}: *([^\n]*)")
string(APPEND line " ${CMAKE_MATCH_1}")
endif()
if("${contents}" MATCHES "\n${item}\\.private: *([^\n]*)")
string(APPEND line " ${CMAKE_MATCH_1}")
endif()

string(REGEX REPLACE "\n${item}(\\.private)?:[^\n]*" "" contents "${contents}")
if(NOT "${line}" STREQUAL "")
string(APPEND contents "${item}:${line}\n")
endif()
endforeach()
endif()

if(contents MATCHES "\nLibs: *([^\n]*)")
set(libs "${CMAKE_MATCH_1}")
if(libs MATCHES [[;]])
# Assuming that ';' comes from CMake lists only. Candidate for parameter control.
string(REPLACE ";" " " no_lists "${libs}")
string(REPLACE "${libs}" "${no_lists}" contents "${contents}")
set(libs "${no_lists}")
Comment on lines +60 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this in general not an upstream bug that should be fixed there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not in general. It is often introduced by vcpkg (with wrappers or patches), or by building configurations simply not supported upstream (static on non-windows, pkg-config on windows).

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say it is limited understand of what LIBRARIES might mean in CMake and simply assuming it is a singular file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... if it is using <Pkg>_LIBRARIES at all. vcpkg also turn <Pkg>_LIBRARY into a list sometimes.

endif()

separate_arguments(libs_list UNIX_COMMAND "${libs}")
set(skip_next 0)
set(libs_filtered "")
foreach(item IN LISTS libs_list)
if(skip_next)
set(skip_next 0)
continue()
elseif(item MATCHES "^(-l|-L)?optimized")
string(COMPARE EQUAL "${arg_config}" "DEBUG" skip_next)
continue()
elseif(item MATCHES "^(-l|-L)?debug")
string(COMPARE EQUAL "${arg_config}" "RELEASE" skip_next)
continue()
elseif(item MATCHES "^(-l|-L)?general")
continue()
endif()
if(item MATCHES "[\$`\"\\ ]")
set(item "\"${item}\"")
endif()
list(APPEND libs_filtered "${item}")
endforeach()
list(JOIN libs_filtered " " libs_filtered)
string(REPLACE "${libs}" "${libs_filtered}" contents "${contents}")
set(libs "${libs_filtered}")

if(libs MATCHES "[^ ]*-NOTFOUND")
message(WARNING "Error in ${file}: 'Libs' refers to a missing lib:\n...${CMAKE_MATCH_0}")
endif()
if(libs MATCHES "[^\n]*::[^\n ]*")
message(WARNING "Error in ${file}: 'Libs' refers to a CMake target:\n...${CMAKE_MATCH_0}")
endif()
endif()

# Quote -L, -I, and -l paths starting with `${blah}`
# This was already handled for "Libs", but there might be additional occurrences in other lines.
string(REGEX REPLACE "([ =])(-[LIl]\\\${[^}]*}[^ ;\n\t]*)" [[\1"\2"]] contents "${contents}")

set("${arg_variable}" "${contents}" PARENT_SCOPE)
endfunction()

function(z_vcpkg_fixup_pkgconfig_check_files arg_file arg_config)
set(path_suffix_DEBUG /debug)
set(path_suffix_RELEASE "")

Expand All @@ -9,15 +113,15 @@ function(z_vcpkg_fixup_pkgconfig_check_files file config)
endif()

vcpkg_host_path_list(PREPEND ENV{PKG_CONFIG_PATH}
"${CURRENT_PACKAGES_DIR}${path_suffix_${config}}/lib/pkgconfig"
"${CURRENT_PACKAGES_DIR}${path_suffix_${arg_config}}/lib/pkgconfig"
"${CURRENT_PACKAGES_DIR}/share/pkgconfig"
"${CURRENT_INSTALLED_DIR}${path_suffix_${config}}/lib/pkgconfig"
"${CURRENT_INSTALLED_DIR}${path_suffix_${arg_config}}/lib/pkgconfig"
"${CURRENT_INSTALLED_DIR}/share/pkgconfig"
)

# First make sure everything is ok with the package and its deps
cmake_path(GET file STEM LAST_ONLY package_name)
debug_message("Checking package (${config}): ${package_name}")
cmake_path(GET arg_file STEM LAST_ONLY package_name)
debug_message("Checking package (${arg_config}): ${package_name}")
execute_process(
COMMAND "${PKGCONFIG}" --print-errors --exists "${package_name}"
WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
Expand Down Expand Up @@ -68,14 +172,8 @@ function(vcpkg_fixup_pkgconfig)
endforeach()
endif()

string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_packages_dir "${CURRENT_PACKAGES_DIR}")
string(REGEX REPLACE "^([a-zA-Z]):/" [[/\1/]] unix_installed_dir "${CURRENT_INSTALLED_DIR}")

foreach(config IN ITEMS RELEASE DEBUG)
debug_message("${config} Files: ${arg_${config}_FILES}")
if("${VCPKG_BUILD_TYPE}" STREQUAL "debug" AND "${config}" STREQUAL "RELEASE")
continue()
endif()
if("${VCPKG_BUILD_TYPE}" STREQUAL "release" AND "${config}" STREQUAL "DEBUG")
continue()
endif()
Expand All @@ -91,60 +189,8 @@ function(vcpkg_fixup_pkgconfig)
endif()
#Correct *.pc file
file(READ "${file}" contents)

# this normalizes all files to end with a newline, and use LF instead of CRLF;
# this allows us to use regex matches easier to modify these files.
if(NOT "${contents}" MATCHES "\n$")
string(APPEND contents "\n")
endif()
string(REPLACE "\r\n" "\n" contents "${contents}")

string(REPLACE "${CURRENT_PACKAGES_DIR}" [[${prefix}]] contents "${contents}")
string(REPLACE "${CURRENT_INSTALLED_DIR}" [[${prefix}]] contents "${contents}")
string(REPLACE "${unix_packages_dir}" [[${prefix}]] contents "${contents}")
string(REPLACE "${unix_installed_dir}" [[${prefix}]] contents "${contents}")

string(REGEX REPLACE "(^|\n) *prefix[\t ]*=[^\n]*" "" contents "${contents}")
if("${config}" STREQUAL "DEBUG")
# prefix points at the debug subfolder
string(REPLACE [[${prefix}/debug]] [[${prefix}]] contents "${contents}")
string(REPLACE [[${prefix}/include]] [[${prefix}/../include]] contents "${contents}")
string(REPLACE [[${prefix}/share]] [[${prefix}/../share]] contents "${contents}")
endif()
# quote -L, -I, and -l paths starting with `${blah}`
string(REGEX REPLACE " -([LIl])(\\\${[^}]*}[^ \n\t]*)" [[ -\1"\2"]] contents "${contents}")
# This section fuses XYZ.private and XYZ according to VCPKG_LIBRARY_LINKAGE
#
# Pkgconfig searches Requires.private transitively for Cflags in the dynamic case,
# which prevents us from removing it.
#
# Once this transformation is complete, users of vcpkg should never need to pass
# --static.
if("${VCPKG_LIBRARY_LINKAGE}" STREQUAL "static")
# how this works:
# we want to transform:
# Libs: $1
# Libs.private: $2
# into
# Libs: $1 $2
# and the same thing for Requires and Requires.private

foreach(item IN ITEMS "Libs" "Requires" "Cflags")
set(line "")
if("${contents}" MATCHES "(^|\n)${item}: *([^\n]*)")
string(APPEND line " ${CMAKE_MATCH_2}")
endif()
if("${contents}" MATCHES "(^|\n)${item}\\.private: *([^\n]*)")
string(APPEND line " ${CMAKE_MATCH_2}")
endif()

string(REGEX REPLACE "(^|\n)${item}(\\.private)?:[^\n]*\n" [[\1]] contents "${contents}")
if(NOT "${line}" STREQUAL "")
string(APPEND contents "${item}:${line}\n")
endif()
endforeach()
endif()
file(WRITE "${file}" "prefix=\${pcfiledir}/${relative_pc_path}\n${contents}")
z_vcpkg_fixup_pkgconfig_process_data(contents "${config}" "\${pcfiledir}/${relative_pc_path}")
file(WRITE "${file}" "${contents}")
endforeach()

if(NOT arg_SKIP_CHECK) # The check can only run after all files have been corrected!
Expand Down
27 changes: 15 additions & 12 deletions scripts/test_ports/unit-test-cmake/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ function(unit_test_check_variable_unset utcvu_test utcvu_variable)
if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
unset_fatal_error()
set_has_error()
message(STATUS "${utcvu_test} had an unexpected FATAL_ERROR;
message(SEND_ERROR "${utcvu_test} had an unexpected FATAL_ERROR;
expected: \"${utcvu_value}\"")
message(STATUS "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
message(SEND_ERROR "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
return()
endif()

Expand All @@ -65,7 +65,7 @@ function(unit_test_check_variable_unset utcvu_test utcvu_variable)
endif()

if(DEFINED "${utcvu_variable}")
message(STATUS "${utcvu_test} set ${utcvu_variable};
message(SEND_ERROR "${utcvu_test} set ${utcvu_variable};
expected: \"${utcvu_variable}\" unset
actual : \"${utcvu_actual_value}\"")
set_has_error()
Expand All @@ -78,14 +78,14 @@ function(unit_test_check_variable_equal utcve_test utcve_variable utcve_value)
if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
unset_fatal_error()
set_has_error()
message(STATUS "${utcve_test} had an unexpected FATAL_ERROR;
message(SEND_ERROR "${utcve_test} had an unexpected FATAL_ERROR;
expected: \"${utcve_value}\"")
message(STATUS "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
message(SEND_ERROR "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
return()
endif()

if(NOT DEFINED "${utcve_variable}" AND NOT "${utcve_variable}" MATCHES "^ENV\\{")
message(STATUS "${utcve_test} failed to set ${utcve_variable};
message(SEND_ERROR "${utcve_test} failed to set ${utcve_variable};
expected: \"${utcve_value}\"")
set_has_error()
return()
Expand All @@ -105,7 +105,7 @@ function(unit_test_check_variable_equal utcve_test utcve_variable utcve_value)
endif()

if(NOT "${utcve_actual_value}" STREQUAL "${utcve_value}")
message(STATUS "${utcve_test} resulted in the wrong value for ${utcve_variable};
message(SEND_ERROR "${utcve_test} resulted in the wrong value for ${utcve_variable};
expected: \"${utcve_value}\"
actual : \"${utcve_actual_value}\"")
set_has_error()
Expand All @@ -118,9 +118,9 @@ function(unit_test_check_variable_not_equal utcve_test utcve_variable utcve_valu
if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
unset_fatal_error()
set_has_error()
message(STATUS "${utcve_test} had an unexpected FATAL_ERROR;
message(SEND_ERROR "${utcve_test} had an unexpected FATAL_ERROR;
expected: \"${utcve_value}\"")
message(STATUS "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
message(SEND_ERROR "FATAL_ERROR: ${Z_VCPKG_UNIT_TEST_FATAL_ERROR}")
return()
endif()

Expand All @@ -138,7 +138,7 @@ function(unit_test_check_variable_not_equal utcve_test utcve_variable utcve_valu
endif()

if("${utcve_actual_value}" STREQUAL "${utcve_value}")
message(STATUS "${utcve_test} failed to change ${utcve_variable};
message(SEND_ERROR "${utcve_test} failed to change ${utcve_variable};
unchanged: \"${utcve_value}\"")
set_has_error()
return()
Expand All @@ -149,15 +149,15 @@ function(unit_test_ensure_success utcve_test)
cmake_language(EVAL CODE "${utcve_test}")
if(Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
set_has_error()
message(STATUS "${utcve_test} was expected to be successful.")
message(SEND_ERROR "${utcve_test} was expected to be successful.")
endif()
unset_fatal_error()
endfunction()
function(unit_test_ensure_fatal_error utcve_test)
cmake_language(EVAL CODE "${utcve_test}")
if(NOT Z_VCPKG_UNIT_TEST_HAS_FATAL_ERROR)
set_has_error()
message(STATUS "${utcve_test} was expected to be a FATAL_ERROR.")
message(SEND_ERROR "${utcve_test} was expected to be a FATAL_ERROR.")
endif()
unset_fatal_error()
endfunction()
Expand Down Expand Up @@ -185,6 +185,9 @@ endif()
if("setup-pkgconfig-path" IN_LIST FEATURES)
include("${CMAKE_CURRENT_LIST_DIR}/test-z_vcpkg_setup_pkgconfig_path.cmake")
endif()
if("fixup-pkgconfig" IN_LIST FEATURES)
include("${CMAKE_CURRENT_LIST_DIR}/test-vcpkg_fixup_pkgconfig.cmake")
endif()

if(Z_VCPKG_UNIT_TEST_HAS_ERROR)
_message(FATAL_ERROR "At least one test failed")
Expand Down
Loading