search

mortbopet / Ripes

A graphical processor simulator and assembly editor for the RISC-V ISA
https://ripes.me/
MIT License
2.52k stars 272 forks source link

Build and publish for snapcraft/flatpak/... #107

Open mortbopet opened 3 years ago

mortbopet commented 3 years ago

To avoid having Linux-based users go to GitHub and download Ripes, it would be nice to have Ripes available through some of the various package managers out there.

It would be nice if the generation and publishing of images can be integrated with CI when publishing new releases. https://snapcraft.io/docs/qt5-kde-applications

Petross404 commented 2 years ago

To avoid having Linux-based users go to GitHub and download Ripes, it would be nice to have Ripes available through some of the various package managers out there.

It would be nice if the generation and publishing of images can be integrated with CI when publishing new releases. https://snapcraft.io/docs/qt5-kde-application

So you basically want to provide a precompiled image? That's a good idea.

tinywrkb commented 2 years ago

PoC Flatpak packaging, it's not ready for Flathub publishing, but it's a good start. edit: updated packaging is here
It's missing AppStream appdata/metainfo, and also should be tested that everything works as expected.
Also, it would be nice to have an install target, instead of manually copying the install assets.

mortbopet commented 2 years ago

PoC Flatpak packaging

Nice!

it would be nice to have an install target, instead of manually copying the install assets.

Is the current installation target insufficient for this? I'm wondering if whether setting -DCMAKE_INSTALL_PREFIX=${FLATPAK_DEST} in the config-opts would be sufficient?

tinywrkb commented 2 years ago

Is the current installation target insufficient for this?

Oh, I missed that, it's in master, but not in the latest tagged release.

I'm wondering if whether setting -DCMAKE_INSTALL_PREFIX=${FLATPAK_DEST} in the config-opts would be sufficient?

It's being set automatically by flatpak-builder.

I updated the manifest to build from the latest commit, and the installation target works as expected.
If there's nothing wrong with the app, then it just needs an AppStream appdata/metainfo, and a screenshot, and I think it's pretty much ready for Flathub.

mortbopet commented 2 years ago

Oh, I missed that, it's in master, but not in the latest tagged release.

I see, sorry about that. I foresee a new release coming within the next month or so.

then it just needs an AppStream appdata/metainfo, and a screenshot, and I think it's pretty much ready for Flathub.

I've gone ahead and generated a file using https://www.freedesktop.org/software/appstream/metainfocreator/#/ available here. Let me know if you need anything else from me :+1: .

tinywrkb commented 2 years ago

I sent a #187 with changes to the metainfo file. Details in the commit message.

It's preferable that an app developer will maintain the Flathub packaging.
I mean, I don't mind maintaining it, but it's much better for the users and the platform that app developers will be involved in the distribution of the app, and will build and test in Flatpak environment as part of the development. See more details here.

Anyway, it's pretty much ready for submission, my branch for PR against the Flathub new-pr is here, and you're willing to submit the app, then you can pick it up from there.

It's obviously missing a Risc-V toolchain (riscv64 only?), I don't think it should block submission and publishing, but I'll leave the decision to you.
I prefer not to add a toolchain directly to the app, as it seems like we're going to need to build from source, especially if we want to offer an aarch64 host (by default, Flathub CI builds for both x86_64 and aarch64).
I don't like the idea of building a toolchain in an app packaging, it's not the right place for it.

There are a few options here:

tinywrkb commented 2 years ago

I forgot to mention that I didn't add filesystem access permissions, as it seems like the app is using the File Portal, and doesn't need to open/save files from outside the sandbox without going through the file dialog.

It's not the best medium, but if anyone wants to test before a PR is sent to Flathub, then I'm attaching a Flatpak bundle. Ripes-999dc4e2e7c7bcb5bfec9504aac13fba11ebc59d.tar.gz

tinywrkb commented 2 years ago

@mortbopet thanks for merging the metainfo changes.

I updated the io.github.mortbopet.Ripes branch accordingly, and also dropped the unneeded cleanup, and added a flathub.json so PRs that are sent by flathubbot after it runs flatpak-external-data-checker (f-e-d-c) will not be automatically merged if the CI build run is successful.
I squashed all my changes, but I kept the un-squashed commits in the io.github.mortbopet.Ripes-old branch.

Note that the way I set the f-e-d-c's properties under x-checker-data, flathubbot will open a new PR automatically when a newer tag than 2.2.3 will be available, a tag property will be added, and the commit property will be updated.

tinywrkb commented 2 years ago

We need to avoid using the File Portal when selecting the compiler with the file dialog.

diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp
index ba8c263..3e6af80 100644
--- a/src/settingsdialog.cpp
+++ b/src/settingsdialog.cpp
@@ -177,6 +177,9 @@ QWidget* SettingsDialog::createCompilerPage() {
     auto* pathBrowseButton = new QPushButton("Browse");
     connect(pathBrowseButton, &QPushButton::clicked, [=, ccpath = ccpath] {
         QFileDialog dialog(this);
+#ifdef FLATPAK_TARGET
+        dialog.setOption(QFileDialog::DontUseNativeDialog);
+#endif
         dialog.setAcceptMode(QFileDialog::AcceptOpen);
         if (dialog.exec()) {
             ccpath->setText(dialog.selectedFiles().at(0));
mortbopet commented 2 years ago

flathubbot will open a new PR automatically when a newer tag than 2.2.3 will be available

Cool!

We need to avoid using the File Portal when selecting the compiler with the file dialog.

Why is this?

tinywrkb commented 2 years ago

The File Portal designed to give access to files outside the sandbox, the file dialog is run on the host, it has no access to files inside the sandbox (the mount namespace), only the file selected will be accessible, and its path will be a different one from where it's originated from ($XDG_RUNTIME_DIR/doc/...), see here.

When we use the widget toolkit's file dialog (and not native), then we can select files from the sandbox environment/mount namespace.

With the Flatpak --filesystem permission can bind mount folders (and files) from the host onto the same paths in the sandbox environment.

IIUC, in GTK3, the File Portal was not forced on apps, developers have to switch and use FileChooserNative.
In Qt5, the File Portal is the default.

mortbopet commented 2 years ago

@tinywrkb Set the flag in https://github.com/mortbopet/Ripes/commit/73d75685b45c73d4c1e3c5accbe891bc15a1157d. Would this not in theory be required for all file dialogs used in the program?

tinywrkb commented 2 years ago

Would this not in theory be required for all file dialogs used in the program?

It seems like it. Loading files (c source,...) through the File Portal works as expected, but not saving. I mean, it saves the file in the correct folder, but with the wrong filename, using a temporary filename instead.
I'm guessing here (I didn't try to understand the code) that this is probably because the app uses the file dialog to read the file path, and then reuses this path, instead of opening the file directly for writing with the file dialog and using the same file descriptor.

We should be able to avoid disabling the File Portal when saving and loading files, but this will likely require more changes to the app, so maybe take the easy route, and disable the File Portal for all the file dialogs.