search

bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.37k stars 651 forks source link

feat(mode): add `purego` tag when `pure` #3901

Closed mattyclarkson closed 4 months ago

mattyclarkson commented 6 months ago

The community has agreed1 that "pure" builds of Go code should use the purego build tag.

This change adds that de-facto tag.

Spun out from bazelbuild/bazel-central-registry#1629 to solve building of circl.

mattyclarkson commented 6 months ago

For what it's worth: this works with cloudflare/circl#492

mattyclarkson commented 5 months ago

This branch updates Gazelle to 0.36.0 to get the purego patch.

The Bazel 6.0.0 CI states:

Error computing the main repository mapping: The MODULE.bazel file of gazelle@0.36.0 declares overrides

That would come from the single_version_override that is in the Gazelle release.

I can reproduce that locally:

$ USE_BAZEL_VERSION=6.0.0 bazelisk build --announce_rc -- //...
Starting local Bazel server and connecting to it...
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=190
INFO: Reading rc options for 'build' from /home/matt-clarkson/git/github/bazelbuild/rules_go/tests/bcr/.bazelrc:
  Inherited 'common' options: --enable_bzlmod
ERROR: Error computing the main repository mapping: The MODULE.bazel file of gazelle@0.36.0 declares overrides

That is a a bug with Bazel 6.0.0, 6.1.0, 6.2.0. Fixed in 6.3.0. We may have to update to 6.3.0 in the CI.

FYI: @tyler-french, @fmeum

fmeum commented 5 months ago

I think it's fine to require 6.3.0+ and make the according change to CI. Minor updates are assumed to be backwards compatible anyway.

mattyclarkson commented 5 months ago

OK. Updated to 6.3.0. All green on CI 🎉

fmeum commented 5 months ago

@mattyclarkson Thanks!

@tyler-french Could you test this on the Ãœber codebase?

mattyclarkson commented 4 months ago

@tyler-french / @fmeum if you have time in the next release cycle: it'd be great to get this in. It would allow cloudflare/circl#492 to build without modification to the generated Gazelle build files (and possibly other modules). Thanks.

tyler-french commented 4 months ago

@tyler-french / @fmeum if you have time in the next release cycle: it'd be great to get this in. It would allow cloudflare/circl#492 to build without modification to the generated Gazelle build files (and possibly other modules). Thanks.

Yes. I was blocked on something else but I will get this tested this week.

tyler-french commented 4 months ago

I tested this and its compatible with all existing rules, but I'm trying to remove patches from github.com/cloudflare/circl that add the cgo tags and export the .h files and it's failing.

Are there other changes needed to fix this build?

mattyclarkson commented 4 months ago

I tested this and its compatible with all existing rules, but I'm trying to remove patches from github.com/cloudflare/circl that add the cgo tags and export the .h files and it's failing.

Are there other changes needed to fix this build?

You need cloudflare/circl#492 which switches all their build tags to purego. That is in circl@1.3.8.

Gazelle learnt to understand purego tags in bazelbuild/bazel-gazelle#1767 which is in gazelle@0.36.0.

With those two things and this patch, you should be able to generate working build files direcly with Gazelle.

If you have a Go module that depends on cloudflare/circl in go.mod and are using the Gazelle extension, upgrading to 1.3.8 should make everything work.

zecke commented 3 months ago

This change causes a significant performance (from ms to seconds) impact when handling protobuf. I would like to continue using pure binaries and allow unsafe/pointer arithmetic when compiling protobuf.

mattyclarkson commented 3 months ago

The performance regression discussion is continuing in #3959