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

[vcpkg] Add new function vcpkg_copy_tools #8749

Merged
merged 12 commits into from
May 1, 2020

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented Oct 26, 2019

There are so many ports that provides a tools/tool feature to let the users be able to install optional executables:

cpuinfo:

if(tools IN_LIST FEATURES)
    foreach(cpuinfo_tool cache-info cpuid-dump cpu-info isa-info)
        file(COPY
            ${CURRENT_PACKAGES_DIR}/bin/${cpuinfo_tool}${VCPKG_TARGET_EXECUTABLE_SUFFIX}
            DESTINATION ${CURRENT_PACKAGES_DIR}/tools/${PORT}
        )
        vcpkg_copy_tool_dependencies(${CURRENT_PACKAGES_DIR}/tools/${PORT})
    endforeach()

    if(VCPKG_LIBRARY_LINKAGE STREQUAL static)
        file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/bin)
    else()
        # TODO: Fix it once this lib supports dynamic building.
    endif()
endif()

czmq:

if(CMAKE_HOST_WIN32)
    set(EXECUTABLE_SUFFIX ".exe")
else()
    set(EXECUTABLE_SUFFIX "")
endif()

if ("tool" IN_LIST FEATURES)
    file(COPY ${CURRENT_PACKAGES_DIR}/bin/zmakecert${EXECUTABLE_SUFFIX}
        DESTINATION ${CURRENT_PACKAGES_DIR}/tools/${PORT})
    vcpkg_copy_tool_dependencies(${CURRENT_PACKAGES_DIR}/tools/${PORT})
endif()

if(VCPKG_LIBRARY_LINKAGE STREQUAL static)
    file(REMOVE_RECURSE
        ${CURRENT_PACKAGES_DIR}/bin
        ${CURRENT_PACKAGES_DIR}/debug/bin)
else()
    file(REMOVE
        ${CURRENT_PACKAGES_DIR}/debug/bin/zmakecert${EXECUTABLE_SUFFIX}
        ${CURRENT_PACKAGES_DIR}/bin/zmakecert${EXECUTABLE_SUFFIX})
endif()

libsvm:

if ("tools" IN_LIST FEATURES)
    if(VCPKG_TARGET_IS_WINDOWS)
        set(EXECUTABLE_SUFFIX ".exe")
    else()
        set(EXECUTABLE_SUFFIX "")
    endif()

    foreach (libsvm_tool svm-predict svm-scale svm-toy svm-train)
        if (EXISTS ${CURRENT_PACKAGES_DIR}/bin/${libsvm_tool}${EXECUTABLE_SUFFIX})
            file(
                COPY ${CURRENT_PACKAGES_DIR}/bin/${libsvm_tool}${EXECUTABLE_SUFFIX}
                DESTINATION ${CURRENT_PACKAGES_DIR}/tools/${PORT}
            )
            file(REMOVE ${CURRENT_PACKAGES_DIR}/bin/${libsvm_tool}${EXECUTABLE_SUFFIX})
        endif ()

        vcpkg_copy_tool_dependencies(${CURRENT_PACKAGES_DIR}/tools/${PORT})
    endforeach ()

    if (VCPKG_LIBRARY_LINKAGE STREQUAL static)
        file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/bin)
    endif ()
endif ()

As we can see, these code have been repeated again and again, so I think it is time to create a little shiny function for it. Here it is!

With vcpkg_copy_tools, those code snippets I mentioned above can be replaced as:

czmq:

if ("tool" IN_LIST FEATURES)
    vcpkg_copy_tools(zmakecert)
else()
    vcpkg_copy_tools()
endif()

cpuinfo:

if ("tools" IN_LIST FEATURES)
    vcpkg_copy_tools(cache-info cpuid-dump cpu-info isa-info)
endif()

libsvm:

if ("tools" IN_LIST FEATURES)
    vcpkg_copy_tools(svm-predict svm-scale svm-toy svm-train)
endif()

@myd7349 myd7349 marked this pull request as ready for review October 26, 2019 04:51
@JackBoosY
Copy link
Contributor

This feature is what we need!
We should test it in all triplets. If some tools are port features, the CI system cannot test it.

@myd7349
Copy link
Contributor Author

myd7349 commented Nov 2, 2019

I am wondering if I should put the cleanup work into another function(vcpkg_clean_executables, for example) as it seems a little weired:

if ("tool" IN_LIST FEATURES)
    vcpkg_copy_tools(zmakecert)
else()
    vcpkg_copy_tools() # Install nothing, but do cleanup
endif()

I feel comfortable with this form:

if ("tool" IN_LIST FEATURES)
    vcpkg_copy_tools(zmakecert) # After `zmakecert` is installed, call `vcpkg_clean_executables`
else()
    vcpkg_clean_executables() # Install nothing, but do cleanup
endif()

@JackBoosY JackBoosY requested a review from vicroms November 4, 2019 03:14
@grdowns grdowns self-assigned this Nov 18, 2019
@JackBoosY
Copy link
Contributor

/azp run

@vicroms
Copy link
Member

vicroms commented Jan 23, 2020

/azp run

@vicroms
Copy link
Member

vicroms commented Jan 24, 2020

/azp run

@ras0219-msft
Copy link
Contributor

/azp run

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Mar 19, 2020
@dan-shaw
Copy link
Contributor

/azp run

@longhuan2018
Copy link
Contributor

I have made some extensions to vcpkg_copy_tool_dependencies to solve some problems I encountered in use. I hope it can be helpful to you, so that the new vcpkg_copy_tools can also solve the problems I encountered. My PR #10653

@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil
Copy link
Contributor

Looks good to me! Thanks @myd7349 :)

@strega-nil strega-nil merged commit 0ab1a9e into microsoft:master May 1, 2020
@myd7349 myd7349 deleted the vcpkg-copy-tools branch May 6, 2020 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants