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

[openssl] build failure #41796

Open
aboelens opened this issue Oct 26, 2024 · 6 comments
Open

[openssl] build failure #41796

aboelens opened this issue Oct 26, 2024 · 6 comments
Assignees
Labels
category:port-bug The issue is with a library, which is something the port should already support

Comments

@aboelens
Copy link

Operating system

Windows 10

Compiler

MSVC

Steps to reproduce the behavior

Install openssl in manifest mode using the toolchain from https://github.com/MarkSchofield/WindowsToolchain/tree/main

Failure logs

In the process of getting the build to work I encountered many errors:

  • missing nmake.exe
  • missing rc.exe
  • missing headers (winver.h)
  • etc

Additional context

Since many of the errors I encountered are also mentioned in bug reports, would it make sense to do a pull request with the changes I made to port.cake to get things to work?

@aboelens aboelens added the category:port-bug The issue is with a library, which is something the port should already support label Oct 26, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Oct 26, 2024

Maybe you can start with showing your changes without opening a PR. openssl triggers a lot of CI. At the moment it is not even clear if you used the toolchain correctly. (You must use vcpkg's toolchain file. You can chainload other toolchain file.)

@aboelens
Copy link
Author

Sure.

I got build errors about not being able to find rc.exe, so I used VCPKG_DETECTED_CMAKE_RC_COMPILER to set rc and add it to vcpkg_build_nmake with "RC=${rc}":

cmake_path(NATIVE_PATH VCPKG_DETECTED_CMAKE_AR NORMALIZE ar)
cmake_path(NATIVE_PATH VCPKG_DETECTED_CMAKE_LINKER NORMALIZE ld)
cmake_path(NATIVE_PATH VCPKG_DETECTED_CMAKE_RC_COMPILER NORMALIZE rc)

Parallel build did not work so I added the /FS flag for synchronized writing to pdb files:

if(VCPKG_DETECTED_MSVC OR "${VCPKG_DETECTED_CMAKE_C_COMPILER_FRONTEND_VARIANT}" STREQUAL "MSVC")
  string(APPEND VCPKG_COMBINED_C_FLAGS_DEBUG " /FS")
  string(APPEND VCPKG_COMBINED_C_FLAGS_RELEASE " /FS")
endif()

For the release build the pdb file was not generated so I added /DEBUG /OPT:REF

string(APPEND VCPKG_COMBINED_SHARED_LINKER_FLAGS_DEBUG " /DEBUG")
string(APPEND VCPKG_COMBINED_SHARED_LINKER_FLAGS_RELEASE " /DEBUG /OPT:REF")

I got lots of errors about missing headers, linking errors and missing nmake.exe, so I am finding these through the C compiler and RC compiler paths:

get_filename_component(CL_DIR "${VCPKG_DETECTED_CMAKE_C_COMPILER}" DIRECTORY)
get_filename_component(RC_DIR "${VCPKG_DETECTED_CMAKE_RC_COMPILER}" DIRECTORY)

Finding the used SDK based on the RC compiler chosen and setting the include environment variable:

# Set INCLUDE directories
string(REGEX MATCH "10\\.0\\.[0-9\\.]+" KIT_VERSION "${RC_DIR}")
file(TO_CMAKE_PATH "${RC_DIR}/../../../Include/${KIT_VERSION}/ucrt" KIT_INCLUDE_UCRT)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Include/${KIT_VERSION}/shared" KIT_INCLUDE_SHARED)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Include/${KIT_VERSION}/um" KIT_INCLUDE_UM)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Include/${KIT_VERSION}/winrt" KIT_INCLUDE_WINRT)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Include/${KIT_VERSION}/cppwinrt" KIT_INCLUDE_CPPWINRT)
cmake_path(NATIVE_PATH KIT_INCLUDE_UCRT NORMALIZE kit_include_ucrt)
cmake_path(NATIVE_PATH KIT_INCLUDE_SHARED NORMALIZE kit_include_shared)
cmake_path(NATIVE_PATH KIT_INCLUDE_UM NORMALIZE kit_include_um)
cmake_path(NATIVE_PATH KIT_INCLUDE_WINRT NORMALIZE kit_include_winrt)
cmake_path(NATIVE_PATH KIT_INCLUDE_CPPWINRT NORMALIZE kit_include_cppwinrt)
set(ENV{INCLUDE} "${kit_include_ucrt};${kit_include_shared};${kit_include_um};${kit_include_winrt};${kit_include_cppwinrt}")

Setting the lib environment variable based on the paths found above and the architecture:

# Set LIB directories
file(TO_CMAKE_PATH "${RC_DIR}/../../../Lib/${KIT_VERSION}/ucrt/${VCPKG_TARGET_ARCHITECTURE}" KIT_LIB_UCRT)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Lib/${KIT_VERSION}/ucrt_enclave/${VCPKG_TARGET_ARCHITECTURE}" KIT_LIB_UCRT_ENCLAVE)
file(TO_CMAKE_PATH "${RC_DIR}/../../../Lib/${KIT_VERSION}/um/${VCPKG_TARGET_ARCHITECTURE}" KIT_LIB_UM)
file(TO_CMAKE_PATH "${CL_DIR}/../../../Atlmfc/Lib/${VCPKG_TARGET_ARCHITECTURE}" MSVC_ATLMFC_LIB)
file(TO_CMAKE_PATH "${CL_DIR}/../../../Lib/${VCPKG_TARGET_ARCHITECTURE}" MSVC_LIB)
cmake_path(NATIVE_PATH KIT_LIB_UCRT NORMALIZE kit_lib_ucrt)
cmake_path(NATIVE_PATH KIT_LIB_UCRT_ENCLAVE NORMALIZE kit_lib_ucrt_enclave)
cmake_path(NATIVE_PATH KIT_LIB_UM NORMALIZE kit_lib_um)
cmake_path(NATIVE_PATH MSVC_ATLMFC_LIB NORMALIZE msvc_atlmfc_lib)
cmake_path(NATIVE_PATH MSVC_LIB NORMALIZE msvc_lib)
set(ENV{LIB} "${kit_lib_ucrt};${kit_lib_ucrt_enclave};${kit_lib_um};${msvc_atlmfc_lib};${msvc_lib}")

Setting CMAKE_PROGRAM_PATH so nmake.exe can be found based on the location of the C compiler:

# Set nmake directory
cmake_path(NATIVE_PATH CL_DIR NORMALIZE nmake)
list(APPEND CMAKE_PROGRAM_PATH "${nmake}")

Let me know what you think.

@dg0yt
Copy link
Contributor

dg0yt commented Oct 27, 2024

First: Why do you deal with nmake and parallel compilation? The openssl port is expected to use jom for parallel build jobs instead. That's related to openssl's proprietary configure/make approach.

Second: Think twice which flags actually belong to the CMake toolchain and which need to go to the portfile. Basically we want to inject the flags from the CMake toolchain into the openssl build. Again, openssl's proprietary configure/make is a barrier here.

What must be avoided is adding flags to individual ports which are actually a shortcoming of the CMake toolchain. OTOH using another toolchain for MSVC might show more general issues with assumptions which are hardcoded into some vcpkg scripts. However, openssl might not be the best port to find out.

Third: There is a known and frequent issue with the openssl port: ATM it doesn't like the MSVC tools being in paths with spaces (such as Program Files or Visual Studio). I don't use MSVC locally, and CI uses different paths, so I cannot fix it. And nobody else did. If you are willing to deal with the openssl port, this would be a great initial contribution regardless of the actual MSVC toolchain. An approach might be to adjust PATH, which might also help with rc.

@aboelens
Copy link
Author

Thanks for you feedback.

First: jom is indeed used for parallel builds. However, parallel builds kept failing because multiple processes tried to write to the same pdb file at the same time. This is fixed by the /FS flag, which synchronizes these writes.

Second: As far as I am aware CMAKE_RC_COMPILER is also set by CMake when no toolchain is specified. On MSVC/Clangcl based builds rc.exe is used and on mingw builds windres.exe is used. This is why I made the assumption that CMAKE_RC_COMPILER is always set. Do you know whether this is correct?

Third: That is an interesting question. Right now I would not know how to solve this without either using CMAKE_RC_COMPILER to get the Visual Studio Windows Target Platform Version or using CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION directly.

@dg0yt
Copy link
Contributor

dg0yt commented Oct 27, 2024

ports/openssl/windows/portfile.cmake uses vcpkg_build_nmake. Some of the proposed changes seem to be already handled (if not broken). However the effect might not be visible in logs because the function set environment variables.

vcpkg_list(APPEND make_opts ${arg_OPTIONS} ${arg_OPTIONS_DEBUG})
if(NOT arg_CL_LANGUAGE STREQUAL "NONE")
set(ENV{_CL_} "${VCPKG_DETECTED_CMAKE_${arg_CL_LANGUAGE}_FLAGS_DEBUG}")
endif()
set(ENV{_LINK_} "${VCPKG_DETECTED_CMAKE_SHARED_LINKER_FLAGS_DEBUG}")

  • /FS is added by vcpkg_build_nmake when /MP or -MP is in VCPKG_COMBINED_<language>_FLAGS_<config>.

    if(arg_PREFER_JOM AND VCPKG_CONCURRENCY GREATER "1")
    vcpkg_find_acquire_program(JOM)
    get_filename_component(JOM_EXE_PATH "${JOM}" DIRECTORY)
    vcpkg_add_to_path("${JOM_EXE_PATH}")
    if(arg_CL_LANGUAGE AND "${VCPKG_DETECTED_CMAKE_${arg_CL_LANGUAGE}_COMPILER_ID}" STREQUAL "MSVC")
    string(REGEX REPLACE " [/-]MP[0-9]* " " " VCPKG_COMBINED_${arg_CL_LANGUAGE}_FLAGS_DEBUG " ${VCPKG_COMBINED_${arg_CL_LANGUAGE}_FLAGS_DEBUG} /FS")
    string(REGEX REPLACE " [/-]MP[0-9]* " " " VCPKG_COMBINED_${arg_CL_LANGUAGE}_FLAGS_RELEASE " ${VCPKG_COMBINED_${arg_CL_LANGUAGE}_FLAGS_RELEASE} /FS")
    endif()
    else()

    Now there is a real bug (but not in port openssl): The VCPKG_COMBINED_ prefix is wrong here. It belongs to the vcpkg-cmake-get-vars port, but the non-port maintainer function vcpkg_build_nmake must use the non-port z_vcpkg_get_cmake_vars.

    From the gmp port (error logs), I know this value:

    set(VCPKG_DETECTED_CMAKE_C_FLAGS_RELEASE "-nologo -DWIN32 -D_WINDOWS -utf-8 -MP   -MD -O2 -Oi -Gy -DNDEBUG -Z7")
    

    So -MP is already there.

  • /DEBUG /OPT:REF is expected to come from the cmake toolchain.
    From the gmp port (error logs), I know this value:

    set(VCPKG_DETECTED_CMAKE_SHARED_LINKER_FLAGS_RELEASE "-machine:x64   -nologo -DEBUG -INCREMENTAL:NO -OPT:REF -OPT:ICF")
    

    So -OPT:REF -DEBUG is already there.

@aboelens
Copy link
Author

  • Thank you very much for that pull request. I'll wait for it to complete.

  • I just contacted the maintainer of the toolchain to get this fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

No branches or pull requests

3 participants