-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
net: use libSystem bindings for DNS resolution on macos if cgo is unavailable #30686
Conversation
This PR (HEAD: bb803d8) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Brad Fitzpatrick: Patch Set 2: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Elias Naur: Patch Set 2: This will break on iOS because of missing arm and arm64 assembly. If you take a stab at the implementation I can test it. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 2:
Ah woops, I will add those now. Thank you for help testing it. For learning purposes could you also tell me how I can test it myself? Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: 044733c) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Elias Naur: Patch Set 2:
Take a look at misc/ios/README. It's not trivial :) Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: 86893a3) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: 75578bc) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: b9747ec) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Grant Seltzer Richman: Patch Set 6: (2 comments)
Ah alright, thank you! Reading through it now, I just pushed the arm and arm64 assembly. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Elias Naur: Patch Set 6: (4 comments)
Thansk. Running "GOARCH=arm64 ./iostest.bash" on my macOS machine gives me: $ GOARCH=arm64 ./iostest.bash Note that CGO_ENABLED=1 is required for iOS. Perhaps that means you can get away with stubs for the assembly implementation on arm and arm64. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 6:
Ah alright. I'm having trouble running that script myself but I think it's a problem with my go install. I found a bunch of examples in the runtime package of arm assembly stubs so i'll push those up. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: a628c74) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Ian Lance Taylor: Patch Set 7: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 7: TryBots beginning. Status page: https://farmer.golang.org/try?commit=323b1189 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 7: Build is still in progress... Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: 05b9e5f) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Ian Lance Taylor: Patch Set 8: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 8: (4 comments) Had some real silly errors for undefined variables. Turns out the vscode extension i'm used to wasn't installed. Sorry about that. Also realized the the libcCall() function is in runtime. I can copy it over and all the associated functions and datatypes. I can also move the lookup routines to the runtime package. Any thoughts on what's best? Doesn't look like it's defined anywhere besides runtime. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Ian Lance Taylor: Patch Set 8:
Perhaps you can do what the syscall package does. You'll need a small helper in runtime/sys_darwin.go. Or moving a couple of functions to the runtime package is likely fine. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 8: (1 comment)
Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: 92de1b3) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Grant Seltzer Richman: Patch Set 9: I've gone ahead and rearranged this change set. The lookup routines that had The assembly routines are now in the runtime package. I added an exported function for the resolver search but I'm not happy with that design. I will look into what the syscall package does this weekend. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR (HEAD: 8f42cff) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: 45ca41c) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Signed-off-by: grant <[email protected]>
This PR (HEAD: a9ff324) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Brad Fitzpatrick: Patch Set 28: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 28: TryBots beginning. Status page: https://farmer.golang.org/try?commit=860cafae Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 28: Build is still in progress... Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 28: TryBot-Result-1 1 of 19 TryBots failed: Consult https://build.golang.org/ to see whether they are new failures. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 28:
I'm not exactly sure what's going on here, can someone help me diagnose? Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Keith Randall: Patch Set 28:
That looks unrelated, a timeout on something not darwin. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Signed-off-by: grant <[email protected]>
This PR (HEAD: ce26e64) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Grant Seltzer Richman: Patch Set 28:
Sounds good Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Keith Randall: Patch Set 29: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 29: TryBots beginning. Status page: https://farmer.golang.org/try?commit=0de65021 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 29: TryBot-Result+1 TryBots are happy. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Ian Lance Taylor: Patch Set 29:
This one already has a +2, you don't need to wait for me. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Keith Randall: Patch Set 29:
Mikio, want to take a final look? Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Brad Fitzpatrick: Patch Set 29: (4 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Signed-off-by: grant <[email protected]>
This PR (HEAD: 3c3ff6b) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/166297 to see it. Tip: You can toggle comments from me using the |
Message from Grant Seltzer Richman: Patch Set 29: (4 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Grant Seltzer Richman: Patch Set 30: How's this looking? Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Brad Fitzpatrick: Patch Set 30: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 30: TryBots beginning. Status page: https://farmer.golang.org/try?commit=0602f513 Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
Message from Gobot Gobot: Patch Set 30: TryBot-Result+1 TryBots are happy. Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
…vailable This change adds directives to link the res_search function in libSystem. The corresponding Go function is then used in `lookup_darwin.go` for resolution when cgo is disabled. This makes DNS resolution logic more reliable as macOS has some unique quirks such as the `/etc/resolver/` directory for specifying nameservers. Fixes #12524 Change-Id: I367263c4951383965b3ef6491196152f78e614b1 GitHub-Last-Rev: 3c3ff6b GitHub-Pull-Request: #30686 Reviewed-on: https://go-review.googlesource.com/c/go/+/166297 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]>
Message from Brad Fitzpatrick: Patch Set 30: RELNOTE=yes Please don’t reply on this GitHub thread. Visit golang.org/cl/166297. |
This PR is being closed because golang.org/cl/166297 has been merged. |
This change adds directives to link the res_search function in libSystem.
The corresponding Go function is then used in
lookup_darwin.go
forresolution when cgo is disabled. This makes DNS resolution logic more
reliable as macOS has some unique quirks such as the
/etc/resolver/
directory for specifying nameservers.
Fixes #12524