bazel-gazelle
bazel-gazelle copied to clipboard
bazelbuild
Allow tools to be built with cgo
What type of PR is this?
Bug fix
What package or component does this PR mostly affect?
cmd/gazelle
What does this PR do? Why is it needed?
See the linked issue for more information. It allows Gazelle's tools to be built with cgo as builds fail when the Go SDK enforces it.
Which issues(s) does this PR fix?
Fixes https://github.com/bazelbuild/bazel-gazelle/issues/1135
Other notes for review
In order to get something like this in, I would like to see some kind of regression test added so we can better understand what is broken right now, and what functionality this enables going forward.
That makes sense, thank you. I have some ideas in mind for regression tests but is there anything specific you were looking for?
I'd really like to understand the context behind forcefully disabling cgo in the first place. It was introduced as part of https://github.com/bazelbuild/bazel-gazelle/pull/382, but I'm not aware of cgo making builds non-deterministic? Please correct me if I'm wrong!
Somehow you came to the conclusion that this was a necessary change. Canonicalizing that and putting it in the repo as a regression test is where I would start.
I can't speak to why something I didn't review happened. Sorry about that! We should also double check that the binaries are still reproducibly built after making this change. I have vague recollections of /tmp/$RANDOMSTRING being a part of my binaries in the long ago times and I wonder if the linked PR is what fixed that. It'd be unfortunate to go back on that.
It looks like https://github.com/bazelbuild/rules_go/issues/3010 should resolve any issues with non-deterministic cgo builds.
Sorry for the lack of movement on this, will get round to writing a regression test soon.