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

Jump to bottom

[vcpkg] Fix toolchain compatibility with cmake < 3.15 #18898

Closed
wants to merge 4 commits into from
Closed

[vcpkg] Fix toolchain compatibility with cmake < 3.15 #18898

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75ca248
Fix toolchain compatibility with cmake < 3.15
dg0yt Jul 10, 2021
16f02ef
Add function z_vcpkg_add_vcpkg_to_cmake_path
dg0yt Jul 13, 2021
4426ef5
Run test port
dg0yt Jul 13, 2021
59a7839
Apply suggestions from code review
strega-nil-ms Jul 20, 2021
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
21 changes: 13 additions & 8 deletions scripts/buildsystems/vcpkg.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,35 +380,40 @@ set(_VCPKG_INSTALLED_DIR "${VCPKG_INSTALLED_DIR}"
CACHE PATH
"The directory which contains the installed libraries for each triplet" FORCE)

# CMake 3.15 is required list(PREPEND ...).
if(VCPKG_PREFER_SYSTEM_LIBS)
set(Z_VCPKG_PATH_LIST_OP APPEND)
list(LENGTH CMAKE_PREFIX_PATH Z_VCPKG_PREFIX_PATH_POS)
list(LENGTH CMAKE_LIBRARY_PATH Z_VCPKG_LIBRARY_PATH_POS)
list(LENGTH CMAKE_FIND_ROOT_PATH Z_VCPKG_FIND_ROOT_PATH_POS)
else()
set(Z_VCPKG_PATH_LIST_OP PREPEND)
set(Z_VCPKG_PREFIX_PATH_POS 0)
set(Z_VCPKG_LIBRARY_PATH_POS 0)
set(Z_VCPKG_FIND_ROOT_PATH_POS 0)
endif()
Copy link
Contributor

@strega-nil strega-nil Jul 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

One could probably do:

function(z_vcpkg_path_list_op out_var lst paths)
    if(VCPKG_PREFER_SYSTEM_LIBS)
        set("${out_var}" "${lst};${paths}" PARENT_SCOPE)
    else()
        set("${out_var}" "${paths};${lst}" PARENT_SCOPE)
    endif()
endfunction()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does "one could probably do" mean I should update the PR? I would prefer to stick to plain cmake. If there shall be a function I would prefer a name which catures the specific operation "add" or "merge" instead of the generic "op", e.g. z_vcpkg_add_to_system_libs in analogy to VCPKG_PREFER_SYSTEM_LIBS.

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 prefer to switch to this; I don't like how this is currently working. Doing this weird INSERT bodging feels bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set("${out_var}" "${paths};${lst}" PARENT_SCOPE) doesn't work correctly for empty lists.
And list(INSERT ...) is unable to append.
😕

Copy link
Contributor

@Neumann-A Neumann-A Jul 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Doing this weird INSERT bodging feels bad.

@strega-nil INSERT is also used by VTK to insert into the beginning of CMAKE_MODULE_PATH. It is the correct way to PREPEND.

And list(INSERT ...) is unable to append.

@dg0yt: have you tried using -1 as an index? Otherwise that is the reason why list(APPEND) existed from the beginning while PREPEND was added much later (since INSERT 0 just worked for this instead)

To add: From the 3.0 docs:

If is -1 or lesser, it is indexed from the end of the list, with -1 representing the last list element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have you tried using -1 as an index? Otherwise that is the reason why list(APPEND) existed from the beginning while PREPEND was added much later (since INSERT 0 just worked for this instead)

To add: From the 3.0 docs:

If is -1 or lesser, it is indexed from the end of the list, with -1 representing the last list element.

Yes, I tried in a separate cmake script. INSERT inserts before the given list element, and INSERT -1 inserts before the last list element.
CMake lists aren't C++ containers...

Copy link
Contributor

Choose a reason for hiding this comment

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

You can do:

if(VCPKG_PREFER_SYSTEM_LIBS)
    list(APPEND lst "${paths}")
else()
    list(INSERT lst 0 "${paths}")
endif()
set("${out_var}" "${lst}" PARENT_SCOPE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can do ...

That's what I already did.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, sorry, I clearly didn't reread the PR.


if(CMAKE_BUILD_TYPE MATCHES "^[Dd][Ee][Bb][Uu][Gg]$" OR NOT DEFINED CMAKE_BUILD_TYPE) #Debug build: Put Debug paths before Release paths.
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_PREFIX_PATH
list(INSERT CMAKE_PREFIX_PATH ${Z_VCPKG_PREFIX_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}"
)
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_LIBRARY_PATH
list(INSERT CMAKE_LIBRARY_PATH ${Z_VCPKG_LIBRARY_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug/lib/manual-link"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/lib/manual-link"
)
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_FIND_ROOT_PATH
list(INSERT CMAKE_FIND_ROOT_PATH ${Z_VCPKG_FIND_ROOT_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}"
)
else() #Release build: Put Release paths before Debug paths. Debug Paths are required so that CMake generates correct info in autogenerated target files.
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_PREFIX_PATH
list(INSERT CMAKE_PREFIX_PATH ${Z_VCPKG_PREFIX_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug"
)
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_LIBRARY_PATH
list(INSERT CMAKE_LIBRARY_PATH ${Z_VCPKG_LIBRARY_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/lib/manual-link"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug/lib/manual-link"
)
list(${Z_VCPKG_PATH_LIST_OP} CMAKE_FIND_ROOT_PATH
list(INSERT CMAKE_FIND_ROOT_PATH ${Z_VCPKG_FIND_ROOT_PATH_POS}
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}"
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug"
)
Expand Down