Skip to content

Remove compile-time AsynchDNS check#1206

Merged
supervacuus merged 2 commits intogetsentry:masterfrom
gregcotten:fix-asynchdns-requirement
Apr 21, 2025
Merged

Remove compile-time AsynchDNS check#1206
supervacuus merged 2 commits intogetsentry:masterfrom
gregcotten:fix-asynchdns-requirement

Conversation

@gregcotten
Copy link
Copy Markdown
Contributor

Resolves #1205

I am not super confident with my sentry-config.cmake.in change. Can someone better versed in CMake let me know if that is right?

@gregcotten
Copy link
Copy Markdown
Contributor Author

@supervacuus based on your Issue / PR history this seems like one for you to look at when you get a moment!

Copy link
Copy Markdown
Collaborator

@supervacuus supervacuus left a comment

Choose a reason for hiding this comment

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

Unfortunately, this is not the proper fix. While 3.29.0 may fix something that rectified the horrible situation in many packaging environments from the CMake side, this would turn off the check for way more users than necessary.

I propose the opposite direction: since a significant portion of users work in a packaging environment, which leads to false negatives (for whatever reason, there seem to be multiple causes) during build time, let's turn off the component check altogether.

As you mentioned in the related issue, there will rarely be a curl installation that doesn't support this feature, and we also have runtime checks during initialization. Since the build check doesn't provide any feedback on the deployment situation anyway, we can remove it and solely rely on the runtime checks.

Please also add a changelog entry.

@gregcotten
Copy link
Copy Markdown
Contributor Author

@supervacuus - makes sense! done.

@supervacuus supervacuus merged commit fb301a6 into getsentry:master Apr 21, 2025
32 checks passed
@gregcotten gregcotten changed the title only check for AsynchDNS if using CMake 3.29.0+ Remove compile-time AsynchDNS check Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AsynchDNS CURL Component requirement can erroneously fail

2 participants