nblockchain / geewallet

geewallet is a non-custodial, minimalistic & pragmatist opensource crossplatform lightweight brainwallet to hold the most important cryptoassets in the same application with ease & peace of mind
MIT License
61 stars 37 forks source link

scripts,GitHubCI,Backend: add .NET6 snap #207

Closed webwarrior-ws closed 5 months ago

webwarrior-ws commented 1 year ago

Use .NET6 to build snap package. Publish Frontend.Console as single executable in in snap_build script.

Modified launch script to point to gwallet executable generated with new settings.

Set InvariantGlobalization to true in Backend and Frontend.Console projects to fix error when launching gwallet installed by snap. Error in question was: Process terminated. Couldn't find a valid ICU package installed on the system. Please install libicu using your package manager and try again. Alternatively you can set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support.

Set correct configPath when GWallet is in a snap package.

Build mono snap package alongside .net6. Also upload mono snap package as artifact and test it. It is needed because the frontend branch can't switch yet to dotnet-based snap.

When building snap package with .NET6, only include single-file executable in package. Added "publish" command to make.fsx and Makefile.

knocte commented 1 year ago

Set InvariantGlobalization to true in Backend and Frontend.Console projects to fix error when launching gwallet installed by snap.

Fix error? What error? Please dump the whole error in the commit message, no need to be secretive about it.

One thing I noticed though is when making snap package with dotnet, all build artifacts end up in a package, even though we don't need them as everything is packed into 1 executable

This needs to be fixed indeed, otherwise the snap package is unnecessarily big.

And I'm not sure what is the best way to remove them. make install both builds in release mode and copies needed files. Maybe delete unneeded files in destination dir (under ./staging) after make install?

No. As I suggested in the review, first change make.fsx to call dotnet publish (the developer should not need to know the way to call dotnet publish, the point of using Makefiles is to have make do everything). After you've done that, change the dotnet publish call to not use the -o flag (to keep the executable in its proper publish folder, which signals in a good way that it's architecture-specific); and after that, change the 'install' target to make it check first if the publish folder exists (and if it does, copy just the single executable instead of the other files).

knocte commented 1 year ago

When addressing my last review, while you're at it, please squash into 1 commit, there's no reason to separate this into different commits, unless I'm missing something.

knocte commented 1 year ago

@webwarrior-ws 2 things:

knocte commented 1 year ago

2 things:

A 3rd thing actually: given that we're maintaining the mono snap package instead of changing it to always use dotnet, the title of commit msg (and PR title) needs to be updated.

knocte commented 1 year ago

Last thing to fix before I merge this is cosmetic:

webwarrior-ws commented 1 year ago

Updated commit message and PR title/description