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 Binary Helpers #5

Closed
wants to merge 1 commit into from
Closed

Flatpak Binary Helpers #5

wants to merge 1 commit into from

Conversation

grhitch
Copy link

@grhitch grhitch commented Jun 6, 2020

Adding ;bin to the end of the merge-dirs line means that links are automatically created allowing plugins to find their binary helpers. See for example oesENC where I created this issue bdbcat/oesenc_pi#92 (comment).
I accept that there may be other ways to solve this problem, but this does seem to work OK. With this small change in place I made a flatpak for oernc that also behaves properly.
Greg

Adding ;bin to the end of the merge-dirs line means that links are automatically created allowing plugins to find their binary helpers. See for example oesENC where I created this issue bdbcat/oesenc_pi#92 (comment).
I accept that there may be other ways to solve this problem, but this does seem to work OK. With this small change in place I made a flatpak for oernc that also behaves properly.
Greg
@leamas
Copy link
Owner

leamas commented Jun 7, 2020

Hi!

Sorry for late reply.

First: leamas/OpenCPN is just a playground thing right now (besides the wiki). See it as place where I develop my own PRs now and then, nothing more. The correct way to update for example the flatpak packaging is to make a PR against OpenCPN/OpenCPN.

Secondly: Unfortunately, all of the instructions I have left around are not updated "blushes". So: which instructions did you follow when yoy ran into the problem described in bdbcat/oesenc_pi#92?

@grhitch
Copy link
Author

grhitch commented Jun 7, 2020

Hi Alec,
I wasn't sure where to post the PR and chose this fork as you seem to be doing a lot of the heavy lifting on the flatpak packaging. I am happy to create a new PR on OpenCPN/OpenCPN, but would like to be sure that it is an approach that you support.

As to which instructions I used, I started with #OpenCPN#1457 (comment). But I also looked at every set of instructions that you have 'left around'. In the end, I have installed from the beta repository and built and installed from leamas/OpenCPN and OpenCPN/OpenCPN. Every attempt resulted in the same problem.

As the documentation for flatpak extensions is a bit sparse (in my opinion), I am not sure that I have come up with the best solution. That said, I have tested it with oeRNC, oesENC and s63, and the approach seems to work.

@leamas
Copy link
Owner

leamas commented Jun 8, 2020

Hm... we have so for tried to handle this by patch the plugins instead, If you look into oesenc_pi at https://github.com/bdbcat/oesenc_pi/ you can see how this is handled there, basically by

At least for now; I'd prefer to do it this way just to keep the changes to the main code to a minimum until the 5.2 release is branched.

Thoughts?

@grhitch
Copy link
Author

grhitch commented Jun 8, 2020

Hi Alec,
thanks for the quick reply.

Unfortunately oesENC doesn't work like that from the flatpak, only the tarball. OpenCPN has to be patched (or some other fix). When I had trouble trying to get s63 to work, (which I see Dave has already done - thanks Dave!) I went back to the oseENC example that you have given. It works perfectly from the tarball, but not from a flatpak install because the binaries are not mapped to a findable directory (using find_in_paths as oesENC does). The details are in the oesENC issue from examining the insides of the sandbox.

So, as I see it the problem is only if you want to have installable flatpaks. I imagined that this was an intended future route. Also I find it very covnenient for testing. If you want the only installation path to be via tarballs, then it can be left as is. If that is your choice, then I suggest removing the install lines from the Makefiles so that things are internally consistent.

I am probably taking up time that you would prefer to be spending on other things. I just wanted you to be aware of the issue for future consideration.
Thanks
Greg

@leamas
Copy link
Owner

leamas commented Jun 8, 2020

I started with this approach, but have abandoned it with the feeling that the flatpak extension concept is not really usable as is.

So, for now I think we should focus on the tarball approach. We (really, I) should also remove lingering docs describing how to build regular flatpak extensions, sorry if you have been misguided...

You should be able to use the new "Import" function to install a plugin tarball directly for test purposes. Have you tried this?

@grhitch
Copy link
Author

grhitch commented Jun 8, 2020

Yes I have with no success, but that is probably my fault. Will try again.
Can I suggest that the install section of the Makefiles (in the flatpak directory) be removed at some future point? Perhaps even a note to this effect as I have spent many tens of hours to find a solution. Clearly, it is not urgent if I am the only person to find this problem.
I am happy that you are aware of the issue and will close it out for now.
Thanks again

@grhitch grhitch closed this Jun 8, 2020
@grhitch grhitch reopened this Jun 8, 2020
@grhitch
Copy link
Author

grhitch commented Jun 8, 2020

Sorry misread your reply. You had clearly understood the issue with the docs. Sorry for repeating needlessly

@grhitch grhitch closed this Jun 8, 2020
@leamas
Copy link
Owner

leamas commented Jun 8, 2020

The fix to flatpak/README.md is part of OpenCPN#1936.

I'm so sorry you have invested time in this after reading my outdated docs "ashamed".

Are you aware of other docs floating around which should be updated w r t this issue?

@grhitch
Copy link
Author

grhitch commented Jun 8, 2020

Don't stress I learned lot's in the process. I found the documentation on extensions under flatpak a bit sparse, but I did end up with quite a few plugins working quite happily.
I will have a bit of a trawl through documentation this week and let you know if I find anything that I think could be adjusted.
Greg

@grhitch grhitch closed this Jun 8, 2020
leamas pushed a commit that referenced this pull request Jul 8, 2020
leamas pushed a commit that referenced this pull request Sep 10, 2021
leamas pushed a commit that referenced this pull request Sep 16, 2021
leamas pushed a commit that referenced this pull request Sep 18, 2021
leamas pushed a commit that referenced this pull request Sep 19, 2021
leamas pushed a commit that referenced this pull request Sep 20, 2021
leamas pushed a commit that referenced this pull request Oct 9, 2021
leamas pushed a commit that referenced this pull request Sep 22, 2022
leamas pushed a commit that referenced this pull request Sep 25, 2022
leamas pushed a commit that referenced this pull request Oct 26, 2022
leamas pushed a commit that referenced this pull request Oct 27, 2022
leamas pushed a commit that referenced this pull request Nov 21, 2022
leamas pushed a commit that referenced this pull request Mar 1, 2023
leamas pushed a commit that referenced this pull request May 14, 2023
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