这是indexloc提供的服务,不要输入任何密码
Skip to content

Fix linking regression #50

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 13, 2025
Merged

Conversation

bushkov
Copy link
Contributor

@bushkov bushkov commented May 5, 2025

#44 has introduced a regression that breaks building statically linked applications that depend on resolv-conf as explained in #49.

This PR

  • uses gethostname from libc crate to solve the issue with static builds,
  • bumps version to 0.7.4

Note that since now there is an optional dependency on libc, I put get_system_domain behind system feature to avoid pulling it when it's not needed.

@djc
Copy link
Member

djc commented May 6, 2025

I don't think we want to add back a dependency on libc. Instead, we should copy the definition of gethostname() that libc is using.

@bushkov bushkov marked this pull request as draft May 6, 2025 08:17
@bushkov
Copy link
Contributor Author

bushkov commented May 6, 2025

I don't think we want to add back a dependency on libc. Instead, we should copy the definition of gethostname() that libc is using.

@djc Copying the definition of gethostname() that libc is using would work on Linux systems, but I don't see how it would work on macOS without relying on a syscall since macOS doesn't store the host name in a static, readable file. Or do you want to support only Linux systems?

@djc
Copy link
Member

djc commented May 6, 2025

I don't think we want to add back a dependency on libc. Instead, we should copy the definition of gethostname() that libc is using.

@djc Copying the definition of gethostname() that libc is using would work on Linux systems, but I don't see how it would work on macOS without relying on a syscall since macOS doesn't store the host name in a static, readable file. Or do you want to support only Linux systems?

Well, we could make it conditional on the target requiring static linking similar to the stuff that David mentioned in the issue right? We can use the current approach on macOS I think? Or do you need static linking on macOS?

@bushkov
Copy link
Contributor Author

bushkov commented May 6, 2025

Well, we could make it conditional on the target requiring static linking similar to the stuff that David mentioned in the issue right? We can use the current approach on macOS I think? Or do you need static linking on macOS?

I don't see how it would be possible to statically link on macOS. I will go with the conditional approach based on the target.

@bushkov bushkov marked this pull request as ready for review May 7, 2025 16:36
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for all the iterations!

@djc djc merged commit 6854d3e into hickory-dns:main May 13, 2025
6 checks passed
@djc
Copy link
Member

djc commented May 13, 2025

  • Published resolv-conf v0.7.4 at registry crates-io
  • [new tag] v0.7.4 -> v0.7.4
  • Release notes

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.

2 participants