Skip to content

Conversation

@JonLiu1993
Copy link
Contributor

@JonLiu1993 JonLiu1993 commented Jun 4, 2024

Fixes #39114

  1. Fix sentry-native:x64-windows-static usage
  2. Apply upstream change fix sentry-native linux runtime error. related issue: pthread_create_linux.cc:53 - Check failed: next_pthread_Create. dlsym: RTLD_NEXT used in code not dynamically loaded getsentry/sentry-native#596 (comment)
  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@JonLiu1993 JonLiu1993 added category:port-bug The issue is with a library, which is something the port should already support info:internal labels Jun 4, 2024
@JonLiu1993 JonLiu1993 marked this pull request as ready for review June 4, 2024 10:35
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[sentry-native] Fix linux build error

AFAICT there is no build error. There is a runtime error.
And that's why CI won't sufficiently indicate that the change is okay.

Comment on lines 9 to 14
- if(LINUX)
- target_sources(crashpad_handler PRIVATE
- ../client/pthread_create_linux.cc
- )
- endif()
-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this change is incorrect. Cf. second part of #39114 (comment)

@JonLiu1993 JonLiu1993 marked this pull request as draft June 5, 2024 01:49
@JonLiu1993 JonLiu1993 changed the title [sentry-native] Fix linux build error [sentry-native] Fix linux runtime error Jun 5, 2024
@JonLiu1993
Copy link
Contributor Author

Tested usage successfully by sentry-native:x64-windows-static

@JonLiu1993 JonLiu1993 marked this pull request as ready for review June 6, 2024 05:51
@JonLiu1993 JonLiu1993 changed the title [sentry-native] Fix linux runtime error [sentry-native] Fix linux runtime error and windows usage Jun 6, 2024
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The port usage tests pass with the following triplets:

  • x64-linux
  • x64-windows-static

@WangWeiLin-MV WangWeiLin-MV added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jun 6, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Jun 7, 2024

The changes should also be upstreamed, to have them included in future releases.

@JonLiu1993
Copy link
Contributor Author

I have commit PR for upstream :
getsentry/sentry-native#1005
getsentry/crashpad#103

@vicroms vicroms merged commit 1fcea05 into microsoft:master Jun 7, 2024
@JonLiu1993 JonLiu1993 deleted the dev/Jon/sentry-native branch June 7, 2024 09:59
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 info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sentry-native port requires zlib dependency on windows + problems on linux

4 participants