trezor / trezord-go

:smiling_imp: Trezor Communication Daemon (written in Go)
GNU Lesser General Public License v3.0
241 stars 146 forks source link

Rework build, replace xgo with golang-crossbuild #258

Closed igor-hnizdo closed 1 year ago

igor-hnizdo commented 1 year ago

xgo is deprecated. Let's replace it with Elastic's Golang-crossbuild.

It is better documented and more maintained.

https://github.com/elastic/golang-crossbuild

That docker conveniently also has lipo, which will make the fat binary.

igor-hnizdo commented 1 year ago

Ahh it seems like that, yes. Seems like the base image is arm... hmmm.

tsusanka commented 1 year ago

I am not saying it is a must-have. If we have working builds in CI I am fine with that. But it would be nice.

igor-hnizdo commented 1 year ago

Ah no, the error was because of the .package step, which is still not really fixed.

Revert your local changes and run just make .binary, that should work now. I see that it worked in your log, and failed on the .package step.

I am now fixing .package step.

tsusanka commented 1 year ago

Oh right 🤦 . Great, that works 🎉 . So let's wait for the other fixes or do you want to make a new PR?

igor-hnizdo commented 1 year ago

I can add it here yeah

igor-hnizdo commented 1 year ago

I fixed the release stuff now too.

There are several issues that needed to be fixed:

I can instead pull the code into monorepo here, I don't know what is preferable.

igor-hnizdo commented 1 year ago

Note that notarization still does not work.

There is this project that implements Apple notarization in Rust, I think it will work. I need the JSON keys though :) let's discuss this off-band

https://gregoryszorc.com/docs/apple-codesign/main/index.html

https://gregoryszorc.com/docs/apple-codesign/main/apple_codesign_getting_started.html#obtaining-a-code-signing-certificate

tsusanka commented 1 year ago

xar and bomutils have old source code that is failing on ARM and on new Fedora. The projects are no longer maintained; I have added PRs to the most popular forks, but still I made my own forks.

Is there any chance replacing these with something else? Both have last commits in 2014, and patching such old libraries seems crazy. Isn't there some new alternative? Forking and patching these is a slippery slope..

tsusanka commented 1 year ago

Now I see your fork of bomutils is a fork of another fork which has some additional fixes. Let's wait if they accept your PR https://github.com/BC-SECURITY/bomutils/pull/2. So the question above applies for xar mainly.

tsusanka commented 1 year ago

Btw we use xar for producing the .pkg package installer, correct? Can we replace that with .dmg? Because we distribute Suite like that, and I think it is just fine. It would solve the above problem altogether..

igor-hnizdo commented 1 year ago

I don't know.

I don't think dmg can register itself as launch agent and start on login. It's much more work :) this was easier.

Yeah it's to make the .pkg

igor-hnizdo commented 1 year ago

I made a similar PR here

https://github.com/tpoechtrager/xar/pull/1

igor-hnizdo commented 1 year ago

Anyway, I fixed it by using CFLAGS that set char as signed. So I don't need my forks anymore.

It seems that C standard does not define whether char is by default signed or unsigned, and it defaults as unsigned on ARM for performance reasons.

Fun with C!

Anyway yeah it works now. I don't think I will rework this to DMG, sorry. :( I don't want to debug how to auto-start trezord with that setup.

tsusanka commented 1 year ago

Anyway yeah it works now. I don't think I will rework this to DMG, sorry. :( I don't want to debug how to auto-start trezord with that setup.

Yeah sure, don't worry, that was just an idea how to get rid off the forks, now we do not need that :). Let me have a look!

tsusanka commented 1 year ago

I would merge then and let's do the notarization and windows/linux fixes in another PRs.