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

EXTRA_ARGS should be a multi-value arg #391

Closed
dg0yt opened this issue Oct 22, 2023 · 2 comments · Fixed by #393
Closed

EXTRA_ARGS should be a multi-value arg #391

dg0yt opened this issue Oct 22, 2023 · 2 comments · Fixed by #393
Labels
bug Something isn't working

Comments

@dg0yt
Copy link

dg0yt commented Oct 22, 2023

Environment

  • OS Version: Ubuntu 20.04
  • Source, vcpkg, version 3.4.1

Description

EXTRA_ARGS for gz_find_package should be pass forward to find_package.

# [EXTRA_ARGS]: Optional. Additional args to pass forward to find_package(~)

if(gz_find_package_EXTRA_ARGS)
list(APPEND ${PACKAGE_NAME}_find_package_args ${gz_find_package_EXTRA_ARGS})
endif()

However, only the first argument is actually passed on. Any other argument is interpreted separately, so the desired effect cannot be achieved.

Steps to reproduce

This should pass additional NAMES tinyxml2-expected to find_package.

gz_find_package(TINYXML2 PRETTY tinyxml2
     EXTRA_ARGS NAMES tinyxml2-expected
     REQUIRED_BY graphics
     PRIVATE_FOR graphics)

Output

CMake Warning (dev) at /vcpkg/installed/x64-linux/share/cmake/gz-cmake3/cmake3/GzUtils.cmake:313 (message):
  

  The build script has specified some unrecognized arguments for
  gz_find_package(~):

  tinyxml2-expected

  Either the script has a typo, or it is using an unexpected version of
  gz-cmake.  The version of gz-cmake currently being used is 3.4.1

Call Stack (most recent call first):
  /vcpkg/installed/x64-linux/share/cmake/gz-cmake3/cmake3/GzFindPackage.cmake:178 (_gz_cmake_parse_arguments)
  CMakeLists.txt:63 (gz_find_package)

tinyxml2-expected is parsed as a separate arg to gz_find_package.

Suggestion

EXTRA_ARGS should be moved to the multi-option args in these spots:

set(oneValueArgs VERSION PRETTY PURPOSE EXTRA_ARGS PKGCONFIG PKGCONFIG_LIB PKGCONFIG_VER_COMPARISON)
set(multiValueArgs REQUIRED_BY PRIVATE_FOR COMPONENTS OPTIONAL_COMPONENTS)

set(oneValueArgs VERSION PRETTY PURPOSE EXTRA_ARGS PKGCONFIG PKGCONFIG_LIB PKGCONFIG_VER_COMPARISON)
set(multiValueArgs REQUIRED_BY PRIVATE_FOR COMPONENTS OPTIONAL_COMPONENTS)

@dg0yt dg0yt added the bug Something isn't working label Oct 22, 2023
@j-rivero
Copy link
Contributor

Thanks Kai for issue! Would you mind sending a PR to fix it against the branch you are interested into? I'll take care of reviewing and back/forward porting.

@dg0yt
Copy link
Author

dg0yt commented Oct 29, 2023

I won't send a PR. The actual changes I needed/did for the vcpkg port moved into a different direction: patch the Find modules, so that we don't have to patch each consumer there.
(There are two other changes which are suitable for upstreaming:
https://github.com/microsoft/vcpkg/blob/06c79a9afa6f99f02f44d20df9e0848b2a56bf1b/ports/gz-cmake3/dependencies.patch#L93-L110
But I still have to find the time to do that.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants