nblockchain
scripts,GitHubCI,Backend: add .NET6 snap
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.
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).
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.
@webwarrior-ws 2 things:
- When I give feedback on your PR; for each comment of mine that you address please reply with "Done" and click on "Resolve" button after pushing a commit that addresses the comment
- Nit: I didn't ask you to explain every single detail of what you're doing in the commit message body; I only ask you to add things to the commit message body when they are not self-explanatory (e.g. renaming snap_pkg to have _dotnet suffix because you also add another snap_pkg_mono suffix is self-explanatory; do not waste time explaining it).
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.
Last thing to fix before I merge this is cosmetic:
- If the PR has only 1 commit, what's the reason for the PR title and PR description to not match exactly with commit message title and commit message body? There's no reason for them to be different.
- WRT the commit msg title, what in the world does FE mean? I only used the abbreviation FE once myself, but it was used along the suffix .Console, that is,
FE.Console, because the areas to put in that part of the commit message have to map to real projects. There's no "FE" or "Frontend" project, so if you use this in the title, one might think that this applies to all frontends, which is not the case (because this commit is not modifying Android or Gtk frontends, for example). I think a better area for this commit should start withscripts,GitHubCI,Backend:because the minimal change in FE.Console.fsproj doesn't really matter, the area is already too crowded. Also "using mono and .NET6" is not a good title because master branch is already using mono, so you don't need to mention it. A better title would simply bescripts,GitHubCI,Backend: add .NET6 snap.