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

Update Go to 1.17.x #3073

Closed
denyeart opened this issue Nov 23, 2021 · 3 comments · Fixed by #3115
Closed

Update Go to 1.17.x #3073

denyeart opened this issue Nov 23, 2021 · 3 comments · Fixed by #3115
Assignees
Milestone

Comments

@denyeart
Copy link
Contributor

denyeart commented Nov 23, 2021

Update Go to 1.17.x in fabric and related repositories.

Prep PRs:
Fix build tags for Go 1.17 - #3070
Resolve Go 1.17 staticcheck issues for dependency projects - #3114

@denyeart denyeart self-assigned this Nov 23, 2021
@susha1377 susha1377 added this to the 2021Q4 milestone Dec 1, 2021
@denyeart
Copy link
Contributor Author

denyeart commented Dec 13, 2021

/integration/pluggable integration test fails on Go 1.17 when trying to compile the validation plugin with command:
go build -x -buildmode=plugin -o testdata/plugins/validation/plugin.so github.com/hyperledger/fabric/integration/pluggable/testdata/plugins/validation

Turns out there are two dependencies that aren't compatible with Go 1.17:

$ go mod why -m github.com/cespare/xxhash/v2
# github.com/cespare/xxhash/v2
github.com/hyperledger/fabric/common/metrics/prometheus
github.com/prometheus/client_golang/prometheus
github.com/cespare/xxhash/v2

github.com/cespare/xxhash/v2 has a fix, see:
cespare/xxhash#54
golang/go#48468

But, we can't pull in the latest prometheus that has the xxhash fix without also updating protobuf, which is an issue for etcd (being worked in #3117). For now we can do a replace in go.mod to update github.com/cespare/xxhash/v2.

$ go mod why -m github.com/consensys/gnark-crypto
# github.com/consensys/gnark-crypto
github.com/hyperledger/fabric/msp
github.com/IBM/idemix
github.com/IBM/mathlib
github.com/IBM/mathlib/driver/gurvy
github.com/consensys/gnark-crypto/ecc/bn254

The integration test failure reported in #3115 is:

2021-12-13T04:34:47.8916227Z [build-plugin-validation] /opt/hostedtoolcache/go/1.17.5/x64/pkg/tool/linux_amd64/compile -o $WORK/b223/_pkg_.a -trimpath "$WORK/b223=>" -dynlink -p github.com/consensys/gnark-crypto/ecc/bn254/fp -lang=go1.16 -installsuffix dynlink -buildid Q3Pyikak647lX18LmeU6/Q3Pyikak647lX18LmeU6 -goversion go1.17.5 -symabis $WORK/b223/symabis -importcfg $WORK/b223/importcfg -pack -asmhdr $WORK/b223/go_asm.h /home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp/arith.go /home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp/asm.go /home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp/element.go /home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp/element_ops_amd64.go
2021-12-13T04:34:47.8917985Z [build-plugin-validation] cd /home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp
2021-12-13T04:34:47.8919317Z [build-plugin-validation] /opt/hostedtoolcache/go/1.17.5/x64/pkg/tool/linux_amd64/asm -p github.com/consensys/gnark-crypto/ecc/bn254/fp -trimpath "$WORK/b223=>" -I $WORK/b223/ -I /opt/hostedtoolcache/go/1.17.5/x64/pkg/include -D GOOS_linux -D GOARCH_amd64 -dynlink -o $WORK/b223/element_mul_amd64.o ./element_mul_amd64.s
2021-12-13T04:34:47.8920333Z [build-plugin-validation] # github.com/consensys/gnark-crypto/ecc/bn254/fp
2021-12-13T04:34:47.8922413Z [build-plugin-validation] asm: element_mul_amd64.s:86: when dynamic linking, R15 is clobbered by a global variable access and is used here: 00013 (/home/vsts/work/1/fabric/vendor/github.com/consensys/gnark-crypto/ecc/bn254/fp/element_mul_amd64.s:86)	MULXQ	DI, R14, R15
2021-12-13T04:34:47.8923409Z [build-plugin-validation] asm: assembly failed

It doesn't look like there is a fix in github.com/consensys/gnark-crypto yet, so we will either need to remove this dependency or see if that project can do a similar fix as was done in github.com/cespare/xxhash/v2.

@denyeart
Copy link
Contributor Author

@ale-linux Could you look at the last issue above?

@ale-linux
Copy link
Contributor

I think that this

Consensys/gnark-crypto#112

fixes it. Let's see what the maintainers of gnark-crypto say.

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 a pull request may close this issue.

3 participants