-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
feat: add flatpak builds #1230
base: master
Are you sure you want to change the base?
feat: add flatpak builds #1230
Conversation
I went in a different route by trying to get the app to build for flatpak specifically, but it does seem to create some problems of its own, not to mention the considerable increase in build time (which is probably a concern with GitHub actions) |
@ZhenyaPav Heya. I've got a good idea of how I'd do this, but I'd love to see how you decided to do it. Making the Flatpak bundle takes a while already and I'd rather keep the builds short, as runners aren't cheap and if the project ever moves to something without free runners that would be an issue (which is why I'm also testing what amounts of caching works and what doesn't). I'll let you know when the PR is ready to merge and I'm open to any comments or suggestions. Yes, there's a CI generated Flatpak in the Linux zip of the runner artifacts on my fork. |
What I came up with is this:
I was able to build the flatpak with this, but it didn't launch due to |
Checked it out on Fedora Silverblue, seems to be working |
@ZhenyaPav Yeah, the idea of building it within the Flatpak runtime is nice for reproducibility, but it doesn't really add any security guarantees and it seems to add too much overhead (longer build times, more technical debt). Having it existing would be nice, however. I think someone in the community is attempting to do a build with Guix, that could potentially help.
I don't see any immediate issues but I'd imagine you need to (and should) use a full path in the |
making |
The sample Flatpak repo solution using flat-manager instead of GH pages took a lot of files and config to work, so I moved it and the accompanying docs to https://gitlab.com/Jabster28/flatman-haveno-test since I wasn't sure if it belonged here. |
@TheTollingBell has rebased the AppImage PR. Do you mind to squash your commits and rebase on the AppImage PR, so there are a total of two commits, please? Otherwise I can test / merge their AppImage PR, then you'll need to rebase your changes on master, but was hoping to see how both look together. |
Sure, and I'll try to minimise the number of commits afterwards. |
@Jabster28 The AppImage PR has been merged to master, so this is ready to rebase on master to resolve the merge conflict, then I can do final testing. |
Seems to install and work for me. Just needs a bit of testing and final looks over. |
@ZhenyaPav Maybe you want to review this? |
mv desktop/build/temp-*/binaries/haveno_*.AppImage ${{ github.workspace }}/release-appimage/Haveno-${{ env.VERSION }}-linux-x86_64.AppImage | ||
else | ||
mv desktop/build/temp-*/binaries/Haveno-*.dmg ${{ github.workspace }}/release/Haveno-${{ env.VERSION }}-mac-installer.dmg | ||
fi | ||
cp desktop/build/temp-*/binaries/desktop-*.jar.SHA-256 ${{ github.workspace }}/release | ||
cp desktop/build/temp-*/binaries/desktop-*.jar.SHA-256 ${{ github.workspace }}/release-flat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cp
command is duplicated below for release-flat
, so can be removed.
@@ -80,8 +80,11 @@ | |||
import java.util.concurrent.atomic.AtomicInteger; | |||
|
|||
@Slf4j | |||
public abstract class HavenoExecutable implements GracefulShutDownHandler, HavenoSetup.HavenoSetupListener, UncaughtExceptionHandler { | |||
public abstract class HavenoExecutable | |||
implements GracefulShutDownHandler, HavenoSetup.HavenoSetupListener, UncaughtExceptionHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please preserve the formatting here, so it's on one line.
|
||
// TODO: find a less janky way to get the app name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update comment with more context, i.e.:
// TODO: regular expression is used to parse application name for flatpak installer, find less janky way
// Don't edit the next line unless you're only editing in between the quotes.
Note: Please see [flatpak.md](../../docs/flatpak.md) for information on | ||
distributing Haveno via Flatpak. | ||
|
||
Note: Please see [flatpak.md](../../docs/flatpak.md) for information on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note is duplicated and can be removed.
@@ -42,7 +48,7 @@ Haveno data folder on Mac: `/Users/<username>/Library/Application Support/Haveno | |||
6. Click "OK" to save the changes and exit the dialog box. | |||
7. Windows will download and install the required files and components to enable the .NET Framework 3.5. This may take several minutes, depending on your internet connection speed and system configuration. | |||
8. Once the installation is complete, you will need to restart your computer to apply the changes. | |||
2. Install Wix Toolset 3: https://github.com/wixtoolset/wix3/releases/tag/wix314rtm | |||
2. Install Wix Toolset 3: <https://github.com/wixtoolset/wix3/releases/tag/wix314rtm> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add <>
here?
at haveno.tools.Utils.renameFile(Utils.java:36) | ||
at io.github.zlika.reproducible.StipZipFile.strip(StipZipFile.java:35) | ||
at haveno.tools.DeterministicBuildTool.main(DeterministicBuildTool.java:24) | ||
at haveno.tools.Utils.renameFile(Utils.java:36) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please preserve this indentation which is standard for exception stack traces.
export _JAVA_OPTIONS="-Djava.io.tmpdir=/storage/tmp" | ||
``` | ||
|
||
#### MacOs | ||
You will also need `flatpak` and `flatpak-builder`. On Debian/Ubuntu: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These dependencies are always needed on Debian/Ubuntu?
In that case they should be added to the Linux instructions above, which currently has sudo apt install -y rpm fuse
.
</description> | ||
<screenshots> | ||
<screenshot type="default"> | ||
<image>https://files.catbox.moe/8pahgg.png</image> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not be fetching this screenshot from a remote repo imo, which could become unavailable.
Can we read it locally from the desktop package instead, if it's useful?
</li> | ||
<li>There is No token, because we don't need it. Transactions between traders are secured by non-custodial multisignature transactions on the Monero network. | ||
</li> | ||
<li>The revenue generated by Haveno will be managed by an entity called Council (more info soon), composed by members of the Monero/Haveno community, not the Haveno Core Team and will be used to fund Haveno and Monero development. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the description is outdated and should be removed (using a council has been abandoned).
def newManifest = file('exchange.haveno.Haveno.yaml') | ||
newManifest.write(manifest.text.replace("- --share=network", "- --share=network\n - --filesystem=~/.local/share/${defaultAppName}:create")) | ||
flatpakManifestFile = 'exchange.haveno.Haveno.yaml' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove 3 newlines, so there's only one extra newline, since it's all related to flatpak.
@@ -415,6 +468,8 @@ task packageInstallers { | |||
} | |||
} | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra newlines.
[flat-manager](https://github.com/flatpak/flat-manager). | ||
|
||
An example Haveno flat-manager solution using `docker-compose` has been created | ||
and documented at <https://gitlab.com/Jabster28/flatman-haveno-test.git> if you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test resource should be self-contained within the Haveno repos, to avoid creating dependencies on third party repositories which are outside our control.
I've created a new repository for this purpose: https://github.com/haveno-dex/flatman-haveno-test
Do you mind opening a PR with these changes so it's all under the haveno-dex umbrella?
@Jabster28 Thanks very much for the update! Some changes requested and I think it's ready to merge. |
The process is a little bit finicky at the moment, but the general idea is:
core/src/main/java/haveno/core/app/HavenoExecutable.java
with a janky regex--filesystem=
that matches the app name.deb
file (should use the app-image (not appimage) from jpackage in the future)If you're repackaging Haveno and would like to change the name and other things:
desktop/src/main/resources/images/task_bar_icon_windows.png
.exchange.haveno.Haveno
and replace them with something else (perhapsexchange.haveno.${APPNAME}
or something similar), as leaving them the same will cause issues with installs~/.local/share/${APPNAME}
, so ensure your app name isn't Steam or something commonfilesystem
to apersist
(the data will stay in Flatpak's sandbox instead of your home dir)desktop/package/linux/exchange.haveno.Haveno.yml
desktop/package/linux/exchange.haveno.Haveno.metainfo.xml
I've never used Gradle before and a lot of weird choices were from myself and Stack Overflow so if something looks wrong, please let me know.