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

WIP: do not hardcode the Windows SDK path #8329

Closed
wants to merge 1 commit into from
Closed

WIP: do not hardcode the Windows SDK path #8329

wants to merge 1 commit into from

Conversation

AlessandroMenti
Copy link

Work in progress, do not merge

Introduce additional functions to avoid hardcoding the Windows SDK installation paths in portfiles.

@msftclas
Copy link

msftclas commented Sep 24, 2019

CLA assistant check
All CLA requirements met.

@AlessandroMenti
Copy link
Author

First, apologies for the late reply as I've been quite busy lately.

While this patch works, I am not fully sure if this is the best way to detect an installation of the Windows SDK.

As it is, the CMake script:

  1. tries to find a Windows 10 SDK, falling back to a Windows 8.1/8 SDK and, finally, to a Windows 7 SDK if more recent SDK versions are not found;
  2. it assumes that the Windows SDK desired version is always specified in the WindowsSDKVersion environment variable, which might not be the case.

@ras0219-msft, @PhoebeHui and @voskrese, do you think it would be worthwhile to "generalize" the search approach as follows (on a similar note to what this FindWindowsSDK module does):

  1. if the WindowsSDKDir and WindowsSDKVersion environment variables are set and correspond to a valid SDK installation path, use them;
  2. else, try searching for a Windows 10/8.1/8/7.1 SDK installation in the Registry; in any case, specifying a minimum admissible version of the SDK should be allowed;
  3. export the correct binary/include/library directories as CMake variables to allow ports to use them instead of hardcoding directory paths.

Do you think that would be a good approach?

@pwilkin
Copy link

pwilkin commented Feb 14, 2020

You forgot to include the patch for the actual ports file:

include(vcpkg_common_functions)

if(NOT VCPKG_CMAKE_SYSTEM_NAME OR VCPKG_CMAKE_SYSTEM_NAME STREQUAL "WindowsStore")
    vcpkg_get_program_files_32_bit(PROGRAM_FILES_32_BIT)
    vcpkg_get_windows_sdk(WINDOWS_SDK)
    vcpkg_get_windows_sdk_path(WINDOWS_SDK_PATH)

    if (WINDOWS_SDK MATCHES "10.")
        set(LIBGLFILEPATH  "${WINDOWS_SDK_PATH}\\Lib\\${WINDOWS_SDK}\\um\\${TRIPLET_SYSTEM_ARCH}\\OpenGL32.Lib")
        set(LIBGLUFILEPATH "${WINDOWS_SDK_PATH}\\Lib\\${WINDOWS_SDK}\\um\\${TRIPLET_SYSTEM_ARCH}\\GlU32.Lib")
        set(HEADERSPATH    "${WINDOWS_SDK_PATH}\\Include\\${WINDOWS_SDK}\\um")
    elseif(WINDOWS_SDK MATCHES "8.")
        set(LIBGLFILEPATH  "${WINDOWS_SDK_PATH}\\Lib\\winv6.3\\um\\${TRIPLET_SYSTEM_ARCH}\\OpenGL32.Lib")
        set(LIBGLUFILEPATH "${WINDOWS_SDK_PATH}\\Lib\\winv6.3\\um\\${TRIPLET_SYSTEM_ARCH}\\GlU32.Lib")
        set(HEADERSPATH    "${WINDOWS_SDK_PATH}\\Include\\um")

(...)

@BillyONeal
Copy link
Member

We believe that ports should use the environment variable provided by vcvarsall, WindowsSdkDir, rather than creating a specific vcpkg function to provide that location. We will fix up ports hard coding such paths to use that environment variable directly.

Thanks for your contribution, and sorry for the delay :(

@BillyONeal BillyONeal closed this Jul 2, 2020
BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Jul 2, 2020
…ding Windows SDK paths.

Also delete vcpkg_get_program_files_32_bit because it was used in only one place.

Resolves microsoft#8288
Obsoletes microsoft#11421, microsoft#11261, microsoft#8329
ras0219-msft pushed a commit that referenced this pull request Jul 6, 2020
…ding Windows SDK paths. (#12232)

Also delete vcpkg_get_program_files_32_bit because it was used in only one place.

Resolves #8288
Obsoletes #11421, #11261, #8329
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
…ding Windows SDK paths. (microsoft#12232)

Also delete vcpkg_get_program_files_32_bit because it was used in only one place.

Resolves microsoft#8288
Obsoletes microsoft#11421, microsoft#11261, microsoft#8329
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

Successfully merging this pull request may close these issues.

None yet

7 participants