Skip to content

Enable non cgo build#8

Merged
tarakby merged 15 commits into
mainfrom
tarak/build-without-cgo
Apr 3, 2024
Merged

Enable non cgo build#8
tarakby merged 15 commits into
mainfrom
tarak/build-without-cgo

Conversation

@tarakby

@tarakby tarakby commented Apr 1, 2024

Copy link
Copy Markdown
Collaborator

Previously, cgo is required to build the module.

However:

  • not all primitives supported in the module require cgo. Only the primitives based on the BLS12-381 do. Applications not requiring these primitives should be able to build without cgo.
  • some applications requiring one or a few of the cgo primitives may want to build without cgo in some cases, and not the run the binary. These applications would want all the exported functions of the modules to remain exported with or without cgo.

This PR solves both issues above:

  • it enables building without cgo: this requires CGO_ENABLED=0 and using the build tag no_cgo.
  • export all public functions of the repo when building without cgo. Any of the non-supported functions panics when called.

Changes to CI jobs:

  • add lint job for building without cgo
  • test the module when building without cgo (including making sure non supported functions panic)
  • test non allowed build combinations (in particular building without cgo and fogetting the no-cgo tag)
  • reorganize ci jobs

@tarakby tarakby requested review from jribbink and turbolent April 1, 2024 23:52
@tarakby tarakby force-pushed the tarak/build-without-cgo branch from 76b8422 to 1354295 Compare April 2, 2024 01:27

@turbolent turbolent left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread internal/blst/non_cgo.go

@turbolent turbolent left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM 👍

@turbolent

Copy link
Copy Markdown
Member

Don't know, but wanted to flag: The build is green, but a GH action is flagging issues at the bottom of the PR: https://github.com/onflow/crypto/pull/8/files#file-sign-go-L77

@jribbink jribbink left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tarakby

tarakby commented Apr 2, 2024

Copy link
Copy Markdown
Collaborator Author

Don't know, but wanted to flag: The build is green, but a GH action is flagging issues

Good flag. This is because of a new check I added to CI in 65daa31. The test checks that the build with incorrect flags fail. The errors are expected but I would prefer that they don't show in every PR 🤔

@tarakby tarakby force-pushed the tarak/build-without-cgo branch from bb3ee7a to 9c0feda Compare April 2, 2024 22:34
@tarakby

tarakby commented Apr 3, 2024

Copy link
Copy Markdown
Collaborator Author

re displayed errors while CI pass : errors are not displayed anymore by disabling the go matcher in the workflow 9c0feda

@tarakby tarakby merged commit e9ca850 into main Apr 3, 2024
@tarakby tarakby deleted the tarak/build-without-cgo branch April 3, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet