Add initial flatpak-spawn support #570
Conversation
JayDoubleu
force-pushed
the
flatpak_experimental
branch
4 times, most recently
from
137a343
to
d5878fc
Compare
|
@mattrose Could you please have a look at the latest changes. I believe this should make it a stable experience when running in flatpak. It also fixes the #347 I these python changes make it to a release, fedora might include terminator as flatpak by default in their remote and it can also be submitted to flathub separately. Only difference on how it works in flatpak I can see right now is that the config file should be edited in |
JayDoubleu
force-pushed
the
flatpak_experimental
branch
from
d5878fc
to
e431d1f
Compare
|
I will definitely take a look at this, but It'll take me a bit because I want to give it a good once-over, and it's a lot of code to digest in an area I'm unfamiliar with. Hopefully before the end of the week. Fedora has included terminator as an rpm at least for the past 10 years or so. @dmaphy and I have been maintaining it. I'm not sure how to convert the fedora rpm into a flatpak, but maybe we can work that out as well as putting it up on flathub. |
|
No worries, I can help out with building it against fedora runtime as well. Nice thing about fedora flatpaks is that once the flatpak manifest has been written and accepted it just uses your RPM to build itself every time there is update to your RPM so you only have to maintain your RPM repo. They key to getting it working properly in flatpaks was to make terminator spawn shells using flatpa-spawn otherwise it would always get shell in flatpak sandbox. If that's good to go it should work in both fedora and flathub flatpaks. |
|
PS. I've added you @mattrose @dmaphy as admins to https://src.fedoraproject.org/flatpaks/terminator and I've also updated it for F35. AFAIK it only needs these steps to run https://docs.fedoraproject.org/en-US/flatpak/tutorial/#_building_in_koji which I can't do myself as I'm not in maintainers group. |
|
Thanks for all your work on this. |
|
So I've looked this over and I have 2 questions. We have tried hard to keep packaging files like .spec files and the Debian directory out of this repository. The Debian packaging files live in the debian repos, and the fedora packaging files live in the fedora repos. Is there a more appropriate place for the flatpak files to live, rather than in the terminator repo? Q 2 is more of a concern about the Profiles->Command Preferences pane. Can you put some indicator in there that the configuration options are not valid when you're running under flatpak. Other than that, this looks really good. |
|
Hey, my understanding is that there is no need for Flatpak folder to be present within the repo itself at all. It's there only for testing or showcasing it working with flatpak-builder ( flathub native type of flatpak, not OCI ) Fedora flatpaks manage flatpaks via flatpaks/terminator on src.fedoraproject.org (or other repo can be created for this purpose. Fedora runtime flatpaks are OCI type thus use different tooling for building as well as use different manifest format ) If you want to publish to flathub that can be done separately by following their instructions (mainly the flatpak manifest is required for such submission, manifest from this PR could be used for this purpose)
Would you be able to specify which options are no longer valid ? I was under impressed that functionality was working in latest commit (both running default shell as well as custom commands) or you're referring to things like |
JayDoubleu
force-pushed
the
flatpak_experimental
branch
from
c12b62b
to
a41a38b
Compare
|
@mattrose I've rebased and left out all the flatpak/flathub files out of this PR. It now has only a single commit containing changes required for terminator to work within flatpaks in general. If this gets into a release then building out fedora OCI flatpak should just work. I've moved the flatpak-builder manifest to my separate branch here: https://github.com/JayDoubleu/terminator/tree/flatpak_flathub_manifest |
| if len(set([args[0], args[1]])) == 1: | ||
| del args[0] |
There was a problem hiding this comment.
@mattrose
This will remove duplicated first command/shells which is caused by https://github.com/gnome-terminator/terminator/blob/master/terminatorlib/terminal.py#L1484 and to my understanding it's no longer needed when working with spawn_async() so it should be safe to do that.
|
This looks good, though I can't test the flatpak stuff atm. Is it safe to be merged in? |
I believe it is safe to be merged and should not affect any users who do not use flatpaks. Once merged and this version makes it's way to fedora I will be able to build and switch to fedora OCI flatpak full time and do some further testing. I guess in both cases the changes would need to be made outside of this repository if there is any tweaks to be done. PS there is no flatpak users as of yet unless someone had built it from these PRs. Perhaps it's worth adding "Discussions" tab to this repo to discuss flatpaks further ? |
|
@mattrose the https://github.com/JayDoubleu/terminator/tree/flatpak_flathub_manifest/experimental/flatpak is in line with this PR + flatpak manifest so in case if you would like to quickly test it on some Fedora VM or something you could use instructions from readme. |
|
hey @mattrose it's been a while.. do you think you'll find some time to look into this ? |
|
Sorry, I've been really swamped lately but I hope to look at it soon. |
|
Thanks @mattrose I've been using it since as flatpak and had zero problems with it so far. |
|
This is almost perfect, and definitely worth merging in. I have one change I'd like to make, but I'll just merge this in and open an issue for the other change. |
This is adding experimental flatpak support as well as definition.
It builds and works ok however flatpak-spawn doesn't seem to be handling tty properly (see flatpak/flatpak#3697)
It's usable and perhaps could be submitted to flathub beta and wait until flatpak-spawn resolves issues with tty or someone rewrites it.
Fixes: #206