hyperledger-archives / aries-framework-go

Hyperledger Aries Framework Go provides packages for building Agent / DIDComm services.
https://wiki.hyperledger.org/display/ARIES/aries-framework-go
Apache License 2.0
240 stars 161 forks source link

Android build issues from using mathlib #3594

Closed Moopli closed 1 year ago

Moopli commented 1 year ago

We use gomobile to build android libs in one of our projects that depends on afgo, and updating to the latest commit revision has introduced a number of build errors:

gomobile: go build -ldflags -w -s -X 'github.com/trustbloc/wallet-sdk/cmd/wallet-sdk-gomobile/version.version=testVer' -X 'github.com/trustbloc/wallet-sdk/cmd/wallet-sdk-gomobile/version.gitRev=testRev' -X 'github.com/trustbloc/wallet-sdk/cmd/wallet-sdk-gomobile/version.buildTime=testTime' -buildmode=c-shared -o=/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/gomobile-work-2843267514/android/src/main/jniLibs/x86/libgojni.so ./gobind failed: exit status 2
# github.com/hyperledger/fabric-amcl/amcl/FP256BN
Error: /Users/runner/work/wallet-sdk/go/pkg/mod/github.com/hyperledger/fabric-amcl@v0.0.0-20210603140002-2670f91851c8/amcl/FP256BN/FP.go:110:19: cannot convert MConst (constant 30564559323915749 of type Chunk) to type int
# github.com/hyperledger/fabric-amcl/core/FP256BN
Error: /Users/runner/work/wallet-sdk/go/pkg/mod/github.com/hyperledger/fabric-amcl@v0.0.0-20210603140002-2670f91851c8/core/FP256BN/FP.go:111:19: cannot convert MConst (constant 30564559323915749 of type Chunk) to type int
# github.com/IBM/mathlib/driver/kilic
Error: /Users/runner/work/wallet-sdk/go/pkg/mod/github.com/!i!b!m/mathlib@v0.0.3-0.20230428120512-8afa4e643d4c/driver/kilic/custom.go:137:2: undefined: mul

The last error is fixed by adding the generic build flag. The MConst cast issue causes an undocumented loss of support for 32-bit architectures - specifying the android build target to only 64-bit architectures allows the build to succeed.

sudeshrshetty commented 1 year ago

@ale-linux Can you please help us out in this issue

ale-linux commented 1 year ago

On it. @Moopli, could you please help me reproduce the issue?

Moopli commented 1 year ago

I figured out a way to reproduce the build errors without having to use trustbloc/wallet-sdk:

  1. Install build prerequisites: gomobile@latest, and android SDK including NDK
  2. In cmd/aries-agent-mobile/Makefile:84, remove the -v (verbose) flag and add the -androidapi X flag for the SDK version you installed (in my case, 22).
  3. Run (cd cmd/aries-agent-mobile/; make bindings-android)

In my case, I got the same build errors as I specified above.

If I amend the same Makefile line to replace the -target flag with -target=android/amd64,android/arm64, and add the flag -tags generic, android bindings build successfully, but this disables support for 32-bit architectures.

ale-linux commented 1 year ago

thanks @Moopli , I think I understand the issue and I can quickly push out a fix. There are a few options, and in order to understand what the best one is I'd like to know what GOARCH you're using for the build that fails. Could you provide me that info pls?

Moopli commented 1 year ago

@ale-linux the failing targets are android/386 and android/arm.

ale-linux commented 1 year ago

Gotcha - yes, exactly what I suspected. fabric-amcl (which packages code from https://github.com/miracl/amcl in a go-gettable way) only contains the 64-bit implementation of FP256BN. We have two options: either just drop FP256BN from mathlib (it's anyway insecure these days) or leave it and add support for the 32 bit version. In the interest of backward compatibility I'm shooting for the second option. Expect from me a PR shortly

ale-linux commented 1 year ago

@Moopli, please take a look at https://github.com/hyperledger/aries-framework-go/pull/3595. All the amcl-related issues were fixed. The undefined: mul issue will likely still be there (and we would need to solve it), but I wanted to have a quick fix out for the thing that got you stuck so that you could validate it.

ale-linux commented 1 year ago

wasm unit tests fail, since the patch inadvertently dropped support for wasm. I'll work to fix it, but in the meantime you can report on the fix status for your platform.

ale-linux commented 1 year ago

wasm test works well now

Moopli commented 1 year ago

@ale-linux The undefined: mul error seems fine to handle just by adding the generic build tag, to enable this implementation in mathlib.

Since you're more familiar with mathlib, would you say that this is an issue to fix in mathlib? Or is this just expected usage?

ale-linux commented 1 year ago

It's definitely a mathlib issue - I just went ahead and fixed it too. Could you please retry https://github.com/hyperledger/aries-framework-go/pull/3595 ?

Moopli commented 1 year ago

Fixed in https://github.com/hyperledger/aries-framework-go/pull/3595