build: Disable Cgo on windows #6768
Conversation
| os.Setenv("CGO_ENABLED", "0") | ||
| } else { | ||
| os.Setenv("CC", cc) | ||
| } |
There was a problem hiding this comment.
I think we always should set cc as that is passed via the command line, I believe.
I think the only want CGO_ENABLED on android no and maybe OS X for watcher stuff?
Presumably the same issue would happen on Linux if I didn't have GCC around?
There was a problem hiding this comment.
It gets a bit murky there:
vendor/golang.org/x/sys/unix/zsyscall_aix_ppc.go:import "C"
vendor/golang.org/x/sys/unix/mkerrors.sh: echo 'import "C"'
vendor/golang.org/x/sys/unix/zsyscall_aix_ppc64_gccgo.go:import "C"
vendor/github.com/syncthing/notify/watcher_fen_cgo.go:import "C"
vendor/github.com/syncthing/notify/watcher_fsevents_cgo.go:import "C"
vendor/github.com/DataDog/zstd/zstd.go:import "C"
vendor/github.com/DataDog/zstd/zstd_stream.go:import "C"
vendor/github.com/DataDog/zstd/errors.go:import "C"
vendor/github.com/shirou/gopsutil/disk/disk_darwin_cgo.go:import "C"
We do have ppc builds, they might need (or profit from) cgo.
Feels like hand-holding cgo is at least as bad as creating a badger fork removing the cgo bits - though as mentioned that might be wasted effort soon.
There was a problem hiding this comment.
The CC argument still holds tho
There was a problem hiding this comment.
To be honest, my gut says that debian et all might complain when we suddenly require a c compiler.
There was a problem hiding this comment.
Only set CGO_ENABLED=0 if the builder did not already set CGO_ENABLED to anything, I think.
There was a problem hiding this comment.
Did that and never set it on solaris and darwin, as it's needed for notify there. That means building for ppc/ppc64/aix maybe requires the caller of build.go to set CGO_ENABLED, though probably that's not needed.
There was a problem hiding this comment.
We already set CGO_ENABLED=0 for everything (I think) on the build server, so for us this is a no-op. The solaris etc builds end up being without notifications enabled.
There was a problem hiding this comment.
I wasn't aware of that, I thought we the mac builder was in place for that. Actually now I am confused why our mac builds even pass: I don't see us setting a kqueue build tag anywhere and then notify should try to build fsevents, which requires cgo.
| os.Setenv("CGO_ENABLED", "0") | ||
| } else { | ||
| os.Setenv("CC", cc) | ||
| } |
There was a problem hiding this comment.
We already set CGO_ENABLED=0 for everything (I think) on the build server, so for us this is a no-op. The solaris etc builds end up being without notifications enabled.
* main: (218 commits) gui, man, authors: Update docs, translations, and contributors lib/protocol: Use sha256.Sum256 in NewDeviceID (syncthing#6775) lib/scanner: Fix Validate docs (syncthing#6776) build: Set CGO_ENABLED=0 by default, except on specific platforms (syncthing#6768) gui, lib/api: Add more version info to about dialog (syncthing#6773) lib/model: Use right db snap when scanning recvonly folder (syncthing#6769) build: Change version strings to not have plus in them (ref syncthing#6758) (syncthing#6760) lib/model: Don't ignore stat failure in performFinish (syncthing#6766) lib/fs: Add support for Windows duplicate extents (syncthing#6764) lib/model: Don't stay scanning forever on fail (syncthing#6761) gui, lib/ignore: Handle editing ignores with error (fixes syncthing#5425) (syncthing#6757) lib: Print nicely rounded durations (syncthing#6756) all: Add copy-on-write filesystem support (fixes syncthing#4271) (syncthing#6746) lib/rand: Various minor fixes (syncthing#6752) lib/db: Improve error message on meta inconsistency (syncthing#6751) lib/db: Use explicit byte type for type prefixes (syncthing#6754) gui, man, authors: Update docs, translations, and contributors lib/model: Check dir before deletion when pulling (syncthing#6741) build: Update integration tests and add to build.go (syncthing#6744) cmd/stdiscosrv: Don't abuse wrong header (syncthing#6749) ...
st-review
added
the
frozen-due-to-age
Cgo is optionally required by badger only (not for long any more hopefully dgraph-io/badger#1162). Lets just not use that and prevent any hassle around getting a C compiler set up.