Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: conditional compile when glibc static-crt #726

Merged
merged 17 commits into from
Feb 17, 2025
Merged

Conversation

Itsusinn
Copy link
Member

🤔 This is a ...

  • Refactoring

@ibigbug
Copy link
Member

ibigbug commented Feb 16, 2025

why we need this change?

@Itsusinn
Copy link
Member Author

  1. We should use “real” SystemResolver as much as possible (before changes gnu with -crt-static use hickory for example)
  2. Android dont support TokioResolver::tokio_from_system_conf but allow using “real” SystemResolver(libc's getaddrinfo)

@ibigbug
Copy link
Member

ibigbug commented Feb 16, 2025

i think the TokioResolver::tokio_from_system_conf was added for libc::getaddrinfo causing crash in certain cases, if we don't have that issue anymore we should look into remove the system TokioResolver some day

@Itsusinn
Copy link
Member Author

Itsusinn commented Feb 16, 2025

i think the TokioResolver::tokio_from_system_conf was added for libc::getaddrinfo causing crash in certain cases, if we don't have that issue anymore we should look into remove the system TokioResolver some day

libc::getaddrinfo will fail under static gnu libc, it a glibc bug existing for decades.
That means only when we have static-crt of gnu targets, we have to keep system TokioResolver

@ibigbug
Copy link
Member

ibigbug commented Feb 16, 2025

ok fair engouth. thanks!

ibigbug
ibigbug previously approved these changes Feb 16, 2025
@ibigbug
Copy link
Member

ibigbug commented Feb 16, 2025

Maybe we should put a comment explaining the issue

@Itsusinn
Copy link
Member Author

Itsusinn commented Feb 16, 2025

https://source.android.com/docs/core/ota/modular-system/dns-resolver?hl=zh-cn#dns-q

Android bonic libc's getaddrinfo is complicated, and broken on cross-test.

It works on real android device,tho.

@Itsusinn
Copy link
Member Author

From cross-rs

[1] libc = bionic; Only works with native tests, that is, tests that do not depends on the Android Runtime. For i686 some tests may fails with the error assertion failed: signal(libc::SIGPIPE, libc::SIG_IGN) != libc::SIG_ERR, see cross-rs/cross#140 for more information.

I gonnna skip test for android target

Signed-off-by: iHsin <root@itsusinn.eu.org>
Signed-off-by: iHsin <root@itsusinn.eu.org>
Signed-off-by: iHsin <root@itsusinn.eu.org>
Signed-off-by: iHsin <root@itsusinn.eu.org>
Signed-off-by: iHsin <root@itsusinn.eu.org>
Signed-off-by: iHsin <root@itsusinn.eu.org>
@Itsusinn Itsusinn merged commit bb86e3c into master Feb 17, 2025
29 checks passed
@Itsusinn Itsusinn deleted the refactor/resolver branch February 17, 2025 03:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants