Docker builds are broken
Closed this issue · 9 comments
Problem
CI automatically builds Docker images. They're currently broken, see e.g. https://github.com/onflow/flow-emulator/actions/runs/7212048234/job/19649026562#step:7:648
The build errors show problems in the crypto library.
Steps to Reproduce
See link above / "Docker Image" CI job's logs.
Acceptance Criteria
Docker images are built correctly.
@sideninja and I faced these problems yesterday with the CLI build as well.
First we need to enable cgo with CGO_ENABLED=1, it's also preferred to add the flag CGO_CFLAGS="-O -D__BLST_PORTABLE__" in case the building or target machines are old (this should be added to the Dockerfile), but then the main problem becomes cross-compilation. I can see the repo targets both x86 and aarch64 .
Not sure if one of the two builds had succeeded from the logs. I guess it also depends on the architecture of the github action machine. But anyway the Dockerfile needs to be updated to handle cross-compilation properly, since CGO is now being used.
I was planning to work on these tasks with the new crypto library repo by repo, but things went fast with the EVM releases.
A quick temp fix that worked for CLI is to go back to the previous crypto library onflow/flow-go/crypto@v.0.24.9 in a replace statement.
This also breaks building the Cadence language server to WebAssembly. Before it was possible to build without CGo, how come we need it now?
This also breaks building the Cadence language server to WebAssembly
Does the Cadence language server depend on the emulator?
how come we need it now?
In the past there was a build tag that is needed if BLS stuff need to be compiled. CLI and emulator always avoided that build tag and excluded BLS (mainly to avoid an extra setup step needed for BLS, not to avoid cgo). The new crypto module doesn't require the setup build, so the build tag was removed.
Can you please point me to the build script of the Cadence language server?
does emulator need BLS? I don't know if evm stuff needs it, but if not maybe we can use build tag again.
Regardless of EVM, Cadence natively supports BLS, so it makes sense for the emulator to support it too. It's been disabled on the emulator all this time. (CLI however does not need BLS and can disable it).
In general, I would like to move away from the build tags within the crypto library as they have been causing development friction. The next steps can be:
- fix the cross-builds for CLI and emulator
- separate all BLS related things in a sub-package
onflow/crypto/bls, so that importingonflow/cryptodoes not mean having to deal with BLS issues.
@tarakby Would moving to /bls package be hard for you? We can try and setup builds on actual OSs so we avoid cross-compiling, but if the bls sub-package is not a lot of work it kind of makes sense and it will also avoid similar issues for other possible tools.
You are right that the crypto library should be splitting sub-packages. I've tried it a while ago but noticed it requires larger refactors than I thought. It's not a quick fix unfortunately.
Moreover it only solves the build issue for CLI (because it doesn't require BLS), but won't solve the build issue of emulator (requires BLS).
I am fixing the build in #548 and I was wondering how I can make sure the built images can run correctly. Is there a test you usually perform for this?
I am fixing the build in #548 and I was wondering how I can make sure the built images can run correctly. Is there a test you usually perform for this?
That's always painful. One way to test is to use act https://github.com/nektos/act but for architecture critical tests it might be a problem since the env is not the same, the second way is to fork the repo and test there. The third way is to create a temporary tag that is something like v0-test and then make sure they are deleted.