Skip to content
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

Use CGO_ENABLED=0 #421

Merged
merged 6 commits into from
Nov 4, 2021
Merged

Use CGO_ENABLED=0 #421

merged 6 commits into from
Nov 4, 2021

Conversation

RealOrangeOne
Copy link
Contributor

@RealOrangeOne RealOrangeOne commented Mar 24, 2021

Fixes #295

Might fix #338

Also impacts #48 and #68

Naive attempt to create statically linked binaries, suitable for dstros like Alpine. Unlike previously, binaries now execute on Alpine.

Binary still appears to execute and connect to tunnels fine on Arch.

$ file nebula
nebula: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, Go BuildID=-XXX, not stripped

Set `CGO_ENABLED` to 0 when building
@CLAassistant
Copy link

CLAassistant commented Mar 24, 2021

CLA assistant check
All committers have signed the CLA.

@RealOrangeOne RealOrangeOne marked this pull request as ready for review March 24, 2021 23:11
@wadey
Copy link
Member

wadey commented Mar 26, 2021

I agree with this change, but only for linux builds. I think cgo does some things on darwin, windows that might be necessary there. The only thing that CGO does for us on linux is hostname lookups, but I don't think that should be an issue for most people. The power of having repeatable builds by disabling CGO would be very nice.

I wish it was easier to find a definitive list of the drawbacks for disabling CGO for each OS. Do you know of a good reference?

Do you think you could change this to only disable CGO on linux builds?

@RealOrangeOne
Copy link
Contributor Author

A quick google search yields no really useful references other than "Where Go calls C". This is a bit of an interesting read.

I don't have access to a windows or macOS machine to try this patch out, but perhaps instead we should test this on windows and macos to see if it's better / no worse? Using the same compiler flags for each OS is probably a good thing, especially if that 1 OS is the one most of CI runs against.

Alternatively, perhaps instead specifically using netgo is an option (go build -tags osusergo,netgo). I'll give that a try this evening to see if that also solves the issue without completely disabling CGo.

This is in theory more stable and cross-OS than CGo
@RealOrangeOne RealOrangeOne changed the title Disable CGo Use netgo for networking stack Mar 26, 2021
@RealOrangeOne
Copy link
Contributor Author

@wadey I found a simpler way. By explicitly using netgo explicitly instead of the cgo implementation, we keep the benefits of using cgo for some things, but with the cross-platform benefits.

I think I prefer this approach?

@wadey
Copy link
Member

wadey commented Mar 26, 2021

I just did some reviewing of the stdlib for everything using +build cgo, and now I'm pretty convinced that disabling CGO won't break anything. It seems pretty clear that the only two things the stdlib uses for CGO are the net and user cases, and we don't care about the user cases at all.

So now I'm on the side of lets just disable CGO like your original PR here, but I want to think a bit more.

@RealOrangeOne
Copy link
Contributor Author

I know little about Go's internals, but yeah I do kinda prefer the cgo approach, that's also how Hugo does it.

I'll leave the branch as-is for now. Think and I can revert if needed.

I'm really keen to get this merged and shipped ASAP, as it'll help out a lot of platforms!

@wadey wadey changed the title Use netgo for networking stack Use CGO_ENABLED=0 Nov 4, 2021
@wadey
Copy link
Member

wadey commented Nov 4, 2021

I've updated this PR to just set CGO_ENABLED=0, which I think is the better option. I have been testing with this and see no issues.

@wadey wadey added this to the v1.5.0 milestone Nov 4, 2021
@wadey wadey merged commit eb66e13 into slackhq:master Nov 4, 2021
@RealOrangeOne RealOrangeOne deleted the disable-cgo branch November 5, 2021 09:11
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.

openwrtx64 can't run nebula static link binary for Linux
4 participants