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

make versioned-binaries doesn't build all the binaries anymore on linux/macOS #1447

Open
spacepluk opened this issue Mar 11, 2024 · 4 comments
Assignees
Labels
Bug The issue represents a bug, malfunction, regression

Comments

@spacepluk
Copy link

Recent versions of flow-cli fail to build using make versioned-binaries. I used to use this to build a version of flow-cli with some minor patches on macOS and it would build all the binaries with no issues.

I've tried to build v1.12.0-cadence-v1.0.0-M8-2, v1.14.1 and feature/stable-cadence and only the build that matches the host os/arch seems to work (except for windows which I couldn't manage to get it to run the makefile).

Here's the output of an attempt to build on ubuntu:

spacepluk@ubuntu-vm:~/flow-cli$ uname -a
Linux ubuntu-vm 6.5.0-25-generic #25-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb  7 14:58:39 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
spacepluk@ubuntu-vm:~/flow-cli$ go version
go version go1.21.1 linux/amd64
spacepluk@ubuntu-vm:~/flow-cli$ make versioned-binaries
cd /home/spacepluk/go; \
mkdir -p /home/spacepluk/go; \
GO111MODULE=on go install github.com/axw/gocov/gocov@latest; \
GO111MODULE=on go install github.com/matm/gocov-html/cmd/gocov-html@latest; \
GO111MODULE=on go install github.com/sanderhahn/gozip/cmd/gozip@latest; \
GO111MODULE=on go install github.com/vektra/mockery/[email protected];
go generate ./...
make OS=linux ARCH=amd64 ARCHNAME=x86_64 versioned-binary
make[1]: Entering directory '/home/spacepluk/flow-cli'
GOOS=linux GOARCH=amd64 make BINARY=./cmd/flow/flow-x86_64-linux- binary
make[2]: Entering directory '/home/spacepluk/flow-cli'
make[2]: Nothing to be done for 'binary'.
make[2]: Leaving directory '/home/spacepluk/flow-cli'
make[1]: Leaving directory '/home/spacepluk/flow-cli'
make OS=linux ARCH=arm64 versioned-binary
make[1]: Entering directory '/home/spacepluk/flow-cli'
GOOS=linux GOARCH=arm64 make BINARY=./cmd/flow/flow-arm64-linux- binary
make[2]: Entering directory '/home/spacepluk/flow-cli'
CGO_ENABLED=1 \
CGO_CFLAGS="-O2 -D__BLST_PORTABLE__" \
GO111MODULE=on go build \
	-trimpath \
	-ldflags \
	"-X github.com/onflow/flow-cli/build.commit=8a5dfa79ad9653d1c1f2c0c7bc7aee2d67220aef -X github.com/onflow/flow-cli/build.semver= -X github.com/onflow/flow-cli/internal/accounts.accountToken=lilico:sF60s3wughJBmNh2"\
	-o ./cmd/flow/flow-arm64-linux- ./cmd/flow
# runtime/cgo
gcc_arm64.S: Assembler messages:
gcc_arm64.S:30: Error: no such instruction: `stp x29,x30,[sp,'
gcc_arm64.S:34: Error: operand size mismatch for `mov'
gcc_arm64.S:36: Error: no such instruction: `stp x19,x20,[sp,'
gcc_arm64.S:39: Error: no such instruction: `stp x21,x22,[sp,'
gcc_arm64.S:42: Error: no such instruction: `stp x23,x24,[sp,'
gcc_arm64.S:45: Error: no such instruction: `stp x25,x26,[sp,'
gcc_arm64.S:48: Error: no such instruction: `stp x27,x28,[sp,'
gcc_arm64.S:52: Error: operand size mismatch for `mov'
gcc_arm64.S:53: Error: operand size mismatch for `mov'
gcc_arm64.S:54: Error: operand size mismatch for `mov'
gcc_arm64.S:56: Error: no such instruction: `blr x20'
gcc_arm64.S:57: Error: no such instruction: `blr x19'
gcc_arm64.S:59: Error: no such instruction: `ldp x27,x28,[sp,'
gcc_arm64.S:62: Error: no such instruction: `ldp x25,x26,[sp,'
gcc_arm64.S:65: Error: no such instruction: `ldp x23,x24,[sp,'
gcc_arm64.S:68: Error: no such instruction: `ldp x21,x22,[sp,'
gcc_arm64.S:71: Error: no such instruction: `ldp x19,x20,[sp,'
gcc_arm64.S:74: Error: no such instruction: `ldp x29,x30,[sp],'
make[2]: *** [Makefile:61: cmd/flow/flow-arm64-linux-] Error 1
make[2]: Leaving directory '/home/spacepluk/flow-cli'
make[1]: *** [Makefile:79: versioned-binary] Error 2
make[1]: Leaving directory '/home/spacepluk/flow-cli'
make: *** [Makefile:72: versioned-binaries] Error 2
spacepluk@ubuntu-vm:~/flow-cli$ 

Some discussion about it on discord.

@tarakby
Copy link

tarakby commented Mar 11, 2024

Thanks for the issue.
The new crypto library uses CGO and that requires specifying the C compiler when cross building (the env variable CC). The compiler is different depending on the target ARCH and OS. I believe this was addressed properly in the release build(host is linux), but not yet addressed in versioned-binaries.
When building natively (target matches the host), this should work properly using the host tool-chain, as you noticed.
You mentioned that this fails on windows though.
I haven't tried building on windows myself, but I can see this note in the crypto library used underneath: "To compile on Windows one has to have MinGW gcc on the %PATH%". Can you please try adding your mingw gcc to your path and see if it solves the windows native build?

For versioned-binaries cross building, there needs to be a tool-chain installation for each host/target pair, which can be a pain to cover all combinations. Is there a specific combination that is blocking you that we should address first?

I agree that either way we should update the current versioned-binaries target, or at least document its behaviour/assumptions.
cc @chasefleming @jribbink

@tarakby tarakby added Bug The issue represents a bug, malfunction, regression and removed Feedback labels Mar 11, 2024
@jribbink
Copy link
Contributor

jribbink commented Mar 11, 2024

Since installing all of the cross compiliers is a bit of a headache, another option we can pursue is migrating this to the same go-releaser-cross docker workflow that is used by our CI/CD system currently. I don't think there's anything stopping us from running this locally and it would reduce the dependency to only docker instead of having to configure various cross compilers, etc.

@tarakby
Copy link

tarakby commented Mar 11, 2024

I agree that would simplify the build and reduce the dependency to docker only. We could still leave the "binary" target for native builds only.

@turbolent
Copy link
Member

Good idea to reuse what we already have on CI, it seems to work 👍

@chasefleming chasefleming removed their assignment May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug The issue represents a bug, malfunction, regression
Projects
Status: No status
Development

No branches or pull requests

5 participants