Skip to content
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

flatpak manifest (org.flatpak.Gourmet.yml) and build instructions (flatpak.yaml)--also updated INSTALL.md #145

Closed
wants to merge 22 commits into from

Conversation

OzarkShepherd
Copy link

This flatpak manifest (org.flatpak.Gourmet.yml) works for me in Ubuntu 20.04, but the manifest should work in any distro. The build instructions (flatpak.yaml) will vary by distro, but it works in Ubuntu 20.04.

instructions for installing org.flatpak.Gourmet.yml flatpak
corrected some run symbols
Copy link
Collaborator

@cydanil cydanil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR!

It's looking good, but the file is malformed (the template I gave you was erroneous). It should begin so:

on:
  push:
    tags:
     - '*'

jobs: 
  flatpak:

    runs-on: ubuntu-20.04

    steps:
    - uses: actions/checkout@v2 
    - name: Install Flatpak
      run: |
        sudo apt install flatpak gnome-software-plugin-flatpak && flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo
        flatpak install flathub org.gnome.Platform//3.36 org.gnome.Sdk//3.36 org.flatpak.Builder
...

I also suggest that you add on workflow-dispatch, so that it can be triggered from a click, while we get the workflow running:

on:
  push:
    tags:
     - '*'
   workflow_dispatch

changed various errors in formatting and split the apt and flatpak install command in "Install Flatpak"
removed comment about working on most debian distros
I removed a vestige reference to python 2.7 from when I was working on the manifest before I found out about the kirienko fork
added workflow dispatch and removed the gnome-software-plugin
added option to run from github, and added -y for the flatpak runtime
split apt and flatpak command
@cydanil
Copy link
Collaborator

cydanil commented Jul 15, 2020

I think that the manifest should be called com.github.thinkle.Gourmet.yaml, which is more consistent with other flatpak packages.

@OzarkShepherd
Copy link
Author

I think that the manifest should be called com.github.thinkle.Gourmet.yaml, which is more consistent with other flatpak packages.

I got the name format from the flatpak documentation https://docs.flatpak.org/en/latest/first-build.html so I thought it was consistent with other flatpaks.

If the flatpak stays only on github and never makes it to flatpak then I could see naming it com.github.kirienko.Gourmet.yaml to avoid confusion with the original version and to make it clear that it won't go on flatpak. This flatpak will not work with the old Gourmet because too many dependencies are now deprecated or not even available. The ones that are available tend to be unmaintained forks that I wasn't sure about anyway. I did try to make a flatpak for that one first, after trying to contact the maintainer at launchpad for permission, but I failed to get it to work. So to say that the flatpak is for the thinkle github is not correct in my opinion, since it only works for this kirienko fork.

buildsystem: simple
build-options:
build-args:
- --share=network
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would also need - --socket=pulseaudio for the timer alerts

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I am trying to figure out where it will work. I tried it with where pyglet gets installed, but that didn't work just now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, it works when added to the run command. I'll adjust flatpak.yaml to enable the pulseaudio socket.

added the pulseaudio socket so timer works
@cydanil
Copy link
Collaborator

cydanil commented Jul 15, 2020

I think that the manifest should be called com.github.thinkle.Gourmet.yaml, which is more consistent with other flatpak packages.

I got the name format from the flatpak documentation https://docs.flatpak.org/en/latest/first-build.html so I thought it was consistent with other flatpaks.

That's true for the example. But the documentation prefers the io.github id. Since we don't have a homepage set up at the moment, I think that minder does it correctly.

If the flatpak stays only on github and never makes it to flatpak then I could see naming it com.github.kirienko.Gourmet.yaml to avoid confusion with the original version and to make it clear that it won't go on flatpak. This flatpak will not work with the old Gourmet because too many dependencies are now deprecated or not even available. The ones that are available tend to be unmaintained forks that I wasn't sure about anyway. I did try to make a flatpak for that one first, after trying to contact the maintainer at launchpad for permission, but I failed to get it to work. So to say that the flatpak is for the thinkle github is not correct in my opinion, since it only works for this kirienko fork.

Yes! However, we aim to eventually bring all the changes upstream to thinkle/gourmet. Thus, I think it would be less error-prone to name the file seemingly incorrectly for now :)

split flatpak off into another job to get rid of an error when running.
try with sudo to add flathub repository
line 24 changed flatpak remote-add from sudo to --user due to error
changed the name of the manifest to io.github.thinkle.Gourmet.yml
@OzarkShepherd
Copy link
Author

Ok, the flatpak manifest has been renamed to io.github.thinkle.Gourmet.yml

Added Pyglet Player to dependencies to get rid of a terminal error, and added Ubuntu 20.04 specific instructions including a workaround for issue kirienko#107.  I also added a note at the beginning that this fork is still under development.
@OzarkShepherd
Copy link
Author

I also updated the INSTALL.md but did not realize that it would go into the same pull request. In the INSTALL.md I took out parts about Gourmet being prepackaged, changed the python to 3.8 and changed the dependencies, mentioned that this is still in development, and included a new section specifically for Ubuntu 20.04.

@OzarkShepherd OzarkShepherd changed the title flatpak manifest (org.flatpak.Gourmet.yml) and build instructions (flatpak.yaml) flatpak manifest (org.flatpak.Gourmet.yml) and build instructions (flatpak.yaml)--also updated INSTALL.md Jul 16, 2020
@OzarkShepherd
Copy link
Author

In the INSTALL.md, pulling in all the dependencies also adds unexpected packages like the chromium snap and some drivers. I don't know why it does that.

@cydanil
Copy link
Collaborator

cydanil commented Jul 17, 2020

This is probably due to the mention of selenium, which mention I'm unsure of. selenium is an GUI automation framework, and we shouldn't need it.

While I appreciate that you're updating INSTALL.md, PR should be as small as possible, and focus on a single issue.
I suggest that you split the PR into two: one for the flatpak, and one for the documentation update.

With regards with the Flatpak, I think that:

  • There should be a name to the workflow (eg. Create Flatpak);
  • workflow_dispatch does not need inputs;
  • The flatpak should be packed into a single file (gourmet.flatpak) that can be made available for download.
  • What about internationalization? Although I suppose it can be left out for a future PR.

The last point is tricky: the build dir is cannot be compressed directly, it seems. Rather, flatpak build-bundle requires the files to be available from some repo. Is that what flatpak build-export does? Or is it build-update-repo?

added name and removed inputs from workflow_dispatch
removed selenium from PipDependencies
@OzarkShepherd
Copy link
Author

-Ok, I took out selenium from the flatpak manifest and it still seems to work. It should also be taken out of https://github.com/kirienko/gourmet/blob/master/requirements.txt
-I also took out the workflow inputs and added a name to the flatpak.yaml
-I will look into how to compress the build dir into a flatpak file
-I thought elib.intl doesn't work anymore and don't know what to replace it with. There are other missing plugins but I don't know what packages to add for the plugins.
-I'll ask on slack about how to split the pull request, because it looks complicated from the google search I did.

@OzarkShepherd
Copy link
Author

I got a single compressed directory into a tar, which contains the repo. Size is 110 MB and does not include the Gnome 3.36 runtime which has to be installed separately. I could put instructions inside the tar if they would be useful. I could see this being useful for end users, but for testing it would have to be rebuilt each time a change is made. Also app launcher icons are not being installed at this time through this tar, which I believe is a sandbox permission problem during the building so it should just take some research to fix. I am hesitant to upload the new tar to my fork right now though because I don't want to make this PR more complicated.

OzarkShepherd and others added 3 commits July 17, 2020 11:51
Changed app id to reflect earlier name change
removed dbus since it wasn't necessary after all
added finish args so the gui will launch and have access to pulseaudio
This reverts commit 53505c8.

to get back to original
This reverts commit 0fb51fe.

to get back to original
@OzarkShepherd
Copy link
Author

the changes to INSTALL.md should now be reverted to the original before I modified it.

@OzarkShepherd
Copy link
Author

I am closing this PR so I can reset my fork and make branches. Then I can make smaller pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants