getsentry / sentry-native

Sentry SDK for C, C++ and native applications.
MIT License
404 stars 170 forks source link

[sentry-native] Fix windows usage #1005

Closed JonLiu1993 closed 5 months ago

JonLiu1993 commented 5 months ago

Hi! I have two issues regarding the using of the sentry-native port.

  1. While trying to generate the cmake project build configuration while using sentry-native as vcpkg dependency, i'm getting this error:
    1> [CMake] CMake Error at out/build/Windows-x86-Debug/vcpkg_installed/x86-windows/share/sentry/sentry_crashpad-targets.cmake:105 (set_target_properties):
    1> [CMake]   The link interface of target "sentry_crashpad::zlib" contains:
    1> [CMake] 
    1> [CMake]     ZLIB::ZLIB

    This problem can be solved by adding additional project dependency for zlib in vcpkg. Related issue:https://github.com/getsentry/sentry-native/issues/596 https://github.com/microsoft/vcpkg/issues/39114

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.83%. Comparing base (235f837) to head (1fea96e). Report is 11 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1005 +/- ## ========================================== - Coverage 86.53% 83.83% -2.71% ========================================== Files 40 53 +13 Lines 3736 5443 +1707 Branches 0 1207 +1207 ========================================== + Hits 3233 4563 +1330 - Misses 503 767 +264 - Partials 0 113 +113 ```
dg0yt commented 5 months ago

Outdated, invalid.

supervacuus commented 5 months ago

Hi @dg0yt and @JonLiu1993, can you minimally explain why this PR is no longer valid? Or provide a link to a vcpkg PR/issue that resolves the mentioned problem differently?

JonLiu1993 commented 5 months ago

@supervacuus, VCPKG has updated sentry-native to 0.7.6 a few days ago. I thought this PR had been merged into it. I checked and found that it was not. I reopened this PR. Sorry for the confusion. https://github.com/microsoft/vcpkg/pull/39255/files

dg0yt commented 5 months ago

Hi @dg0yt and @JonLiu1993, can you minimally explain why this PR is no longer valid?

It incorrectly reverts find_package. It incorrectly reflects the conditions.

dg0yt commented 5 months ago

Cf. #1011 for all recent comments transformed into a PR.

supervacuus commented 5 months ago

It incorrectly reverts find_package. It incorrectly reflects the conditions.

True, I saw that, but that could have been fixed here.

Cf. https://github.com/getsentry/sentry-native/pull/1011 for all recent comments transformed into a PR.

This includes the proposed feedback from here, so can we close this?

supervacuus commented 5 months ago

required changes introduced here: #1013