Skip to content

[libdwarf] New port#34382

Merged
JavierMatosD merged 47 commits intomicrosoft:masterfrom
jeremy-rifkin:jr/libdwarf
Oct 31, 2023
Merged

[libdwarf] New port#34382
JavierMatosD merged 47 commits intomicrosoft:masterfrom
jeremy-rifkin:jr/libdwarf

Conversation

@jeremy-rifkin
Copy link
Copy Markdown
Contributor

@jeremy-rifkin jeremy-rifkin commented Oct 9, 2023 •
edited
Loading

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
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 9, 2023

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
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 9, 2023

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

@jeremy-rifkin
Copy link
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 9, 2023

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
dg0yt
dg0yt reviewed Oct 10, 2023
View reviewed changes
@dg0yt
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 23, 2023

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

@Adela0814
Copy link
Copy Markdown
Contributor

Adela0814 commented Oct 24, 2023

@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
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 28, 2023

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
Copy Markdown
Contributor

Adela0814 commented Oct 30, 2023

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
Copy Markdown
Contributor

dg0yt commented Oct 30, 2023

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

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 30, 2023 •
edited
Loading

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
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 31, 2023

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

@jeremy-rifkin
Copy link
Copy Markdown
Contributor Author

jeremy-rifkin 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.

@dg0yt
Copy link
Copy Markdown
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
View reviewed changes
@Adela0814
Copy link
Copy Markdown
Contributor

Adela0814 commented Oct 31, 2023

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

@Adela0814 Adela0814 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 31, 2023
@jeremy-rifkin
Copy link
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 31, 2023

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

@jeremy-rifkin
Copy link
Copy Markdown
Contributor Author

jeremy-rifkin commented Oct 31, 2023

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

@JavierMatosD JavierMatosD merged commit 809e93f into microsoft:master Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@dg0yt dg0yt dg0yt left review comments

@Adela0814 Adela0814 Adela0814 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Adela0814 Adela0814

Labels

category:new-port The issue is requesting a new library to be added; consider making a PR! info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jeremy-rifkin @dg0yt @Adela0814 @JavierMatosD