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

arguments string for vcpkg command #1

Open
tksharpless opened this issue Oct 10, 2019 · 3 comments
Open

arguments string for vcpkg command #1

tksharpless opened this issue Oct 10, 2019 · 3 comments

Comments

@tksharpless
Copy link

Two possible problems. First, in vcpkg_install_packages you have
# Need the given list to be space-separated #string (REPLACE ";" " " PACKAGES_LIST_STR "${ARGN}")
but then
execute_process( COMMAND ${VCPKG_EXEC} install ${ARGN} WORKING_DIRECTORY ${VCPKG_ROOT} )
If the first line matters, then shouldn't that ${ARGN} be ${PACKAGES_LIST_STR}?

Second, the argument list could become too long for the command processor on 'some systems' (you know which I mean) . It should best be processed in a loop.

@tksharpless
Copy link
Author

Moreover....
The section to determine the platform architecture is needless, because in fact you can't force cmake to change the architecture selected at command level -- if it is wrong for vcpkg you are just out of luck.
So is the line that loads the toolchain file. Best to let cmake take care of that.
Also should use the official variable name VCPKG_TARGET_TRIPLET.

What I have finally is a 'main line' that checks if a toolchain file has been named.
If not it sets a default VCPKG_ROOT and CMAKE_TOOLCHAIN_FILE ;
if so it makes sure VCPCKG_ROOT agrees with the toolchain file path -- a point not covered by the original script.
Then in either case it runs_install_or_update_vcpkg() to make sure there is a working instance.

@ataulien
Copy link
Contributor

If the first line matters, then shouldn't that ${ARGN} be ${PACKAGES_LIST_STR}?

You're right, that looks suspicious. Although the conversion has been commented out, probably because I ran into problems when using the list-style value. Processing the packages one by one in a loop should be the better solution.

The section to determine the platform architecture is needless, because in fact you can't force cmake to change the architecture selected at command level.

I ran into issues on windows where I configured a x64 build but vcpkg just didn't want to pick that up and proceeded to build as x86. Why am I out of luck then and what do you suggest to do to fix the problem?

    check_type_size("void*" SIZEOF_VOID_P BUILTIN_TYPES_ONLY)
    
    if (SIZEOF_VOID_P EQUAL 8)
        message(STATUS "Using Vcpkg triplet 'x64-windows'")
        
        set(VCPKG_TRIPLET x64-windows)
    endif()

So is the line that loads the toolchain file. Best to let cmake take care of that.

For me this line didn't make CMake actually use the toolchain file:

set(CMAKE_TOOLCHAIN_FILE ${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake CACHE STRING "")

Only after adding the include it would use it. Any suggestions around that problem? Just removing the include does not work for me.

Since you seem to have tried a few things and found some of the CMake-script needless, do you mind putting up a Pull Request so I can try your changes on my system and the CI?

@tksharpless
Copy link
Author

tksharpless commented Oct 16, 2019 via email

ataulien pushed a commit that referenced this issue Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants