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

[libdwarf] New port #34382

Merged
merged 47 commits into from
Oct 31, 2023
Merged

[libdwarf] New port #34382

merged 47 commits into from
Oct 31, 2023

Conversation

jeremy-rifkin
Copy link
Contributor

@jeremy-rifkin jeremy-rifkin commented Oct 9, 2023

Trying to add libdwarf. Needed for #34217.

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@jeremy-rifkin jeremy-rifkin changed the title Jr/libdwarf [libdwarf] New port Oct 9, 2023
@jeremy-rifkin
Copy link
Contributor Author

I'm running into /usr/bin/ld: cannot find -lZLIB locally. Not sure what I'm doing wrong here.

@jeremy-rifkin jeremy-rifkin marked this pull request as ready for review October 9, 2023 21:48
@jeremy-rifkin
Copy link
Contributor Author

Ok, looks like it's just z not ZLIB for linking.

@jeremy-rifkin
Copy link
Contributor Author

I'm seeing dwarf_init_finish.c(57): fatal error C1083: Cannot open include file: 'zlib.h': No such file or directory on windows, not really sure why.

@Adela0814 Adela0814 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Oct 10, 2023
ports/libdwarf/v0.8.0-patches.patch Outdated Show resolved Hide resolved
ports/libdwarf/v0.8.0-patches.patch Outdated Show resolved Hide resolved
@dg0yt
Copy link
Contributor

dg0yt commented Oct 10, 2023

You must transfer these suggestions by hand.
Baseline: Use CMake targets to carry the right library names (z/zlib/zlibd) and include dirs (vcpkg_root/include). Use alias targets to keep the patch minimal.

@jeremy-rifkin jeremy-rifkin marked this pull request as ready for review October 23, 2023 17:11
@jeremy-rifkin
Copy link
Contributor Author

Should be good now @Adela0814, thank you for the review!

@Adela0814
Copy link
Contributor

@jeremy-rifkin Usage test failed on x64-windows.
Usage:

  # this is heuristically generated, and may not be correct
  find_package(libdwarf CONFIG REQUIRED)
  target_link_libraries(main PRIVATE libdwarf::dwarf-static)

Error message:

1> CMake generation started for configuration: 'x64-Debug'.
1> Command line: "C:\WINDOWS\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Ninja"  -DCMAKE_BUILD_TYPE:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="E:\CMakeTest\out\install\x64-Debug" -DCMAKE_TOOLCHAIN_FILE:STRING="E:\\5\vcpkg\\scripts\\buildsystems\\vcpkg.cmake" -DCMAKE_C_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe" -DCMAKE_CXX_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe"   -DCMAKE_MAKE_PROGRAM="C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\Ninja\ninja.exe" "E:\CMakeTest" 2>&1"
1> Working directory: E:\CMakeTest\out\build\x64-Debug
1> [CMake] -- The C compiler identification is MSVC 19.37.32824.0
1> [CMake] -- The CXX compiler identification is MSVC 19.37.32824.0
1> [CMake] -- Detecting C compiler ABI info
1> [CMake] -- Detecting C compiler ABI info - done
1> [CMake] -- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting C compile features
1> [CMake] -- Detecting C compile features - done
1> [CMake] -- Detecting CXX compiler ABI info
1> [CMake] -- Detecting CXX compiler ABI info - done
1> [CMake] -- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting CXX compile features
1> [CMake] -- Detecting CXX compile features - done
1> [CMake] -- Configuring done (6.2s)
1> [CMake] CMake Error at E:/5/vcpkg/installed/x64-windows/share/libdwarf/libdwarf-targets.cmake:60 (set_target_properties):
1> [CMake]   The link interface of target "libdwarf::dwarf-static" contains:
1> [CMake] 
1> [CMake]     ZLIB::ZLIB
1> [CMake] 
1> [CMake]   but the target was not found.  Possible reasons include:
1> [CMake] 
1> [CMake]     * There is a typo in the target name.
1> [CMake]     * A find_package call is missing for an IMPORTED target.
1> [CMake]     * An ALIAS target is missing.
1> [CMake] 
1> [CMake] Call Stack (most recent call first):
1> [CMake]   E:/5/vcpkg/installed/x64-windows/share/libdwarf/libdwarf-config.cmake:2 (include)
1> [CMake]   E:/5/vcpkg/scripts/buildsystems/vcpkg.cmake:859 (_find_package)
1> [CMake]   CMakeTest/CMakeLists.txt:17 (find_package)
1> [CMake] -- Generating done (0.0s)
1> [CMake] CMake Generate step failed.  Build files cannot be regenerated correctly.
1> 'C:\WINDOWS\system32\cmd.exe' '/c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Ninja"  -DCMAKE_BUILD_TYPE:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="E:\CMakeTest\out\install\x64-Debug" -DCMAKE_TOOLCHAIN_FILE:STRING="E:\\5\vcpkg\\scripts\\buildsystems\\vcpkg.cmake" -DCMAKE_C_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe" -DCMAKE_CXX_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe"   -DCMAKE_MAKE_PROGRAM="C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\Ninja\ninja.exe" "E:\CMakeTest" 2>&1"' execution failed with error: ''C:\WINDOWS\system32\cmd.exe' '/c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Ninja"  -DCMAKE_BUILD_TYPE:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="E:\CMakeTest\out\install\x64-Debug" -DCMAKE_TOOLCHAIN_FILE:STRING="E:\\5\vcpkg\\scripts\\buildsystems\\vcpkg.cmake" -DCMAKE_C_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe" -DCMAKE_CXX_COMPILER:FILEPATH="C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.37.32822/bin/Hostx64/x64/cl.exe"   -DCMAKE_MAKE_PROGRAM="C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\Ninja\ninja.exe" "E:\CMakeTest" 2>&1"' returned with exit code: 1'.

@Adela0814 Adela0814 marked this pull request as draft October 24, 2023 08:05
@jeremy-rifkin jeremy-rifkin marked this pull request as ready for review October 28, 2023 06:33
@jeremy-rifkin
Copy link
Contributor Author

Thank you, I think the best solution is just to provide a usage text including instructions to find_package(ZLIB CONFIG REQUIRED). It's a bit misleading that the libdwarf target is dwarf-static regardless of static or dynamic linkage but I'm not sure it's worth the effort to fix.

Should be good to review again now.

@Adela0814
Copy link
Contributor

Thank you, I think the best solution is just to provide a usage text including instructions to find_package(ZLIB CONFIG REQUIRED). It's a bit misleading that the libdwarf target is dwarf-static regardless of static or dynamic linkage but I'm not sure it's worth the effort to fix.

Should be good to review again now.

It's better to add find_dependency(ZLIB) to ***Config.cmake.

@dg0yt
Copy link
Contributor

dg0yt commented Oct 30, 2023

@jeremy-rifkin I will send you a PR instead of review comments.

@dg0yt
Copy link
Contributor

dg0yt commented Oct 30, 2023

jeremy-rifkin#1

  • Updates dependency import and export. Skips the try-compile for zlib and zstd because it doesn't work well with targets, and we know the result.
  • Install proper pkg-config.
  • Fixes library linkage, and ensures dllexport.
  • Installs the dwarfdump tool.
  • Update printed usage.

@jeremy-rifkin
Copy link
Contributor Author

I think it should now be good to go for another review. Thanks so much for all the help!

@jeremy-rifkin
Copy link
Contributor Author

The target is named differently for shared/static, which seems sub-ideal. I might be able to add an alias to take care of that if needed.

@dg0yt
Copy link
Contributor

dg0yt commented Oct 31, 2023

The target is named differently for shared/static, which seems sub-ideal. I might be able to add an alias to take care of that if needed.

Please restore the usage file. It is the correct solution to the target name variation.

Adela0814
Adela0814 previously approved these changes Oct 31, 2023
@Adela0814
Copy link
Contributor

No feature needs to test.
Usage tested pass on x64-windows.

@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Oct 31, 2023
@jeremy-rifkin
Copy link
Contributor Author

@dg0yt My apologies, I didn't realize you had updated the usage file :)

@jeremy-rifkin
Copy link
Contributor Author

Your usage file works for me locally for x64 static and shared

@JavierMatosD JavierMatosD merged commit 809e93f into microsoft:master Oct 31, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants