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

Plugin installer #1457

Merged
merged 65 commits into from
Dec 15, 2019
Merged

Plugin installer #1457

merged 65 commits into from
Dec 15, 2019

Conversation

leamas
Copy link
Collaborator

@leamas leamas commented Sep 15, 2019

This is a reboot of the previous PR #1390, an extension of the current plugin manager supporting downloading, installation and updates of Opencpn plugins.

As of now the oesenc, radar, squiddio and bsb4 plugins are supported.

The user interface is basically about some fixes to current plugins window:

https://user-images.githubusercontent.com/405541/61213252-c6e33e80-a704-11e9-9970-44946c6202dc.png

And a new window to install/update plugins:
download-plugins

The interface as well as the overall installer is feature complete after adding catalog maintenance design, backend and UI. Se also the catalog project at https://github.com/opencpn/plugins.

The installer is written based on that we should support Debian, flatpak, Windows and MacOS builds. Flatpak packaging is part of this PR as well as fixes to make the mingw build chain.

New builds from this PR is available on https://cloudsmith.io/~alec-leamas/repos/opencpn/packages/. Flatpak can be tested using

$ flatpak --user remote-add --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo
$ flatpak --user remote-add --no-gpg-verify plug-mgr \
     http://opencpn.duckdns.org/opencpn-beta/website/repo
$ flatpak --user install plug-mgr org.opencpn.OpenCPN
$ flatpak run org.opencpn.OpenCPN

The initial flatpak download is huge, ~400Mb. Subsequent updates weighs in at about 40 Mb.

More info:

I have made basic testing on all platforms. @rgleason has been extremely active and helpful with windows testing. Of course, I am interested in all kinds of feedback: reviews, testing etc.

EDIT: Known issues:

EDIT : Fixed issues:

@Hakansv
Copy link
Contributor

Hakansv commented Sep 16, 2019

@leamas
Since the boat season is close to an end here around I may get time to test this and not less the radar_pi PR. First at Windows VS2017 starting next week and maybe later Raspberry.
I'm sorry I haven't followed your and Rick's discussion in all details. Are there any preparations I've to perform apart from merge the PRs to test branches? My O repository is up to date with current Git master branch.

@leamas
Copy link
Collaborator Author

leamas commented Sep 16, 2019

I'm sorry I haven't followed your and Rick's discussion in all details

Don't be ;)

Are there any preparations I've to perform apart from merge the PRs to test branches?

Not that I'm aware of.

and not less the radar_pi PR

Right, note however opencpn-radar-pi/radar_pi#117

Vi hörs... /a

@rgleason
Copy link
Collaborator

rgleason commented Sep 16, 2019

Leamas,
I realized just today, that the scripts don't include the traditional windows install.exe file, so that as we transfer and accept your PR's for each plugin, that standard way of installing is dropped. Once we fully transition to the new system I don't think this is a problem, but this early in the game, before everyone has transitioned to the new version of OpenCPN, I believe that we still need to have EXE created, so for me, that will be a disincentive to transition to the new system.

Also I believe Bdbcat said that the new system should be compatible or mesh with the existing. I thinkk it It would be best to have a script for creating windows installer.exe with appveyor in your PR's. Then users that don't have the new version of O will still have the windows installer.exe to use.

I hope this does net seem too demainding, since all the plugins presently have that capacity.

@leamas
Copy link
Collaborator Author

leamas commented Sep 17, 2019

Really? IIRC, the .exe files are still produced. More important, that discussion should take place in some of the plugin PR, e. g., radar_pi. Please continue discussion on that subject there.

@leamas
Copy link
Collaborator Author

leamas commented Sep 17, 2019

Just to finish the discussion in this PR: The plugin PRs I have made does include the same artifacts as before, including the .exe packages.

@rgleason
Copy link
Collaborator

Then, I guess merging the changes will be ok because we can continue deploying manually as before while necessary.

@taruti
Copy link
Contributor

taruti commented Sep 22, 2019

Hi,

Thank you for working on this! Tried building this on desktop Linux (Arch...), cmake does not run cleanly. I'm sorry to always bring build system issues:

[t@localhost build]$ cmake ..
-- The C compiler identification is GNU 9.1.0
-- The CXX compiler identification is GNU 9.1.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- cmake version: 3.15.3
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.6.3")
-- *** Build Architecture is i386
-- Setting C++11 standard via cmake standard mechanism
-- Default compiler options:
-- CMAKE_CXX_FLAGS:
-- CMAKE_CXX_FLAGS_DEBUG: -g
-- CMAKE_CXX_FLAGS_MINSIZEREL: -Os -DNDEBUG
-- CMAKE_CXX_FLAGS_RELEASE: -O3 -DNDEBUG
-- CMAKE_CXX_FLAGS_RELWITHDEBINFO: -O2 -g -DNDEBUG
CMake Error at cmake/TargetSetup.cmake:40 (string):
  string sub-command STRIP requires two arguments.
Call Stack (most recent call first):
  CMakeLists.txt:517 (include)


CMake Error at cmake/TargetSetup.cmake:41 (string):
  string no output variable specified
Call Stack (most recent call first):
  CMakeLists.txt:517 (include)


CMake Error at cmake/TargetSetup.cmake:42 (string):
  string sub-command STRIP requires two arguments.
Call Stack (most recent call first):
  CMakeLists.txt:517 (include)


CMake Error at cmake/TargetSetup.cmake:43 (string):
  string no output variable specified
Call Stack (most recent call first):
  CMakeLists.txt:517 (include)


-- *** Staging to build opencpn 5.0.0 ***
...

This is at commit 5d986b824508a332530e5779a3a2947df3038a38 with cmake version 3.15.3.

How would I install the binary flatpak version for testing? Building it needs deps that would be too complex for today.

@leamas
Copy link
Collaborator Author

leamas commented Sep 22, 2019

Oops... I didn't see that coming.

Could be related to faulty or missing lsb_release. What does lsb_release -is say on your box?

@leamas
Copy link
Collaborator Author

leamas commented Sep 22, 2019

How would I install the binary flatpak version for testing? Building it needs deps that
would be too complex for today.

Yes... I need to publish that in a better, flatpakish way. Thanks for reminder, stay tuned.

@taruti
Copy link
Contributor

taruti commented Sep 23, 2019

Hi, lsb_release is not installed on Arch by default. After installing lsb_release cmake runs successfully. Need to patch things for the Cairo Harfbuzz stuff from the other PR before building though.

@leamas
Copy link
Collaborator Author

leamas commented Sep 23, 2019

OK, great. And thanks! I have added those issues to the #1457 (comment) initial comment -- trying to keep things stable while we are testing.

Now, the PR builds and that's good. Does it work for you i. e., can you install and uninstall the supported plugins?

@leamas
Copy link
Collaborator Author

leamas commented Sep 24, 2019

How would I install the binary flatpak version for testing? Building it needs deps that
would be too complex for today.

I have added flatpak instructions to the initial comment: #1457 (comment)

@Hakansv
Copy link
Contributor

Hakansv commented Sep 24, 2019

@leamas
Win10. First test built by VS2017. Builds fine for Debug, Release and Package and works so far.
Installed and later uninstalled squiddio wo trouble.
I suppose next step is a proper data base with latest available plugins. Would that be on opencpn.org or respective plugin Github?
bild
The "Update" button for radar_pi and oeSENC are grayed out and strange. Is that due to newer version already installed or?
bild
(Nothing yet built on radar_pi)
bild

@leamas
Copy link
Collaborator Author

leamas commented Sep 24, 2019

Thanks for taking time to test!

I suppose next step is a proper data base with latest available plugins

It's already installed -- it's the file ocpn-plugins.xml, in the same directory as the opencpn executable. As you can see the install window lists the four supported plugins which looks ok.

It is possible to regenerate ocpn-plugins.xml, see plugins/README.md.

The "Update" button for radar_pi and oeSENC are grayed out and strange. Is that due
to newer version already installed or?

This should reflect that you have already installed these plugins as packages, right? In this case the installer treats them as read-only entities which cannot be changed.

TL;DR If you already have installed oesenc_pi and radar_pi as regular packages everything looks OK.

@taruti
Copy link
Contributor

taruti commented Sep 25, 2019

Installed binary from Flatpak on Arch Linux - success! (needed to tweak GPG things first, since missing the correct keys for the repo.

OpenCPN runs and installing & reinstalling oesenc 1.2.0 and squiddio 0.7.1 worked trivially. Reinstall and Uinstall all work fine!

Wishes/notes on the GUI - these are ideas for future:

  • After clicking "Install" on a plugin the Downloading window opens but is black for one second until it starts working fine.
  • Just having a textarea where the install progress is displayed rather than dialogs might be friendlier if I want to install half a dozen plugins over a slow network?
  • Should the "Install new plugins" window display already installed plugins like it does now?
  • Having Uninstall under the plugin and Reinstall in the "Install new plugins" window was confusing.
  • Perhaps we should look at few plugin installers from other applications on how to polish the GUI in future.

But this will using plugins much easier! This will help especially on mobile. Thank you for the great work :)

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

needed to tweak GPG things first, since missing the correct keys for the repo.

oops... will update flatpak instructions in initial comment.

Should the "Install new plugins" window display already installed plugins like it does now?

Yes, I think so since other alternatives includes Upgrade and Downgrade. Perhaps it's just a confusing window title?

Having Uninstall under the plugin and Reinstall in the "Install new plugins" window was confusing.

Yes... But then again, what should the new title of the Install new plugins window be?

EDIT: And there is actually some sound logic here: The original window operates on the list of installed plugins (hence Uninstall), the new window operates on the available list of downloads representing Install, Upgrade or Downgrade depending on what's actually installed.

@taruti
Copy link
Contributor

taruti commented Sep 25, 2019

Perhaps:

  • "Manage plugins"
  • bold for plugin names there.
  • Plugin icon moves ~10pixels to right when clicking "More" and moves back to left when clicking "Less".
  • Install new plugins window lacks a scrollbar (expand all descriptions, cannot scroll to plugins in the bottom).

But this is all trivial fixes. It might make sense to get the large part merged and then work on the smaller tidying up in separate PRs.

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

"Manage plugins"

Yes! Making a note in initial comment.

It might make sense to get the large part merged and then work on the smaller tidying up

So far, there is no input whatsoever from the maintainers. I plan to do at least one more clean-up cycle, trying to play it safe. But it really depends if testing reveals something requiring larger fixes.

Thanks again for your time & efforts!

@Hakansv
Copy link
Contributor

Hakansv commented Sep 25, 2019

@leamas
Although I've read plugins/README.md it's still not clear how an average user can get a updated ocpn-plugins.xml or a local variant without a OCPN update/reinstall. Plugins are changed or added more or less frequently. Should the "+" button try to update ocpn-plugins.xml before presentation of the available list of plugins? If so would there be a method to read last release for "all" plugins, e.g. "https://github.com/opencpn-radar-pi/radar_pi/releases"
And BTW
The present ocpn-plugins.xml needs some typo tweak:
Row 459, 481, 503, 525, 547 and 569 refers to "github.com/opencpn-radar-pi/radar_pi" although it's about squiddio.
Håkan

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

Although I've read plugins/README.md it's still not clear how an average user can get a
updated ocpn-plugins.xml or a local variant without a OCPN update/reinstall.

yes... I've been thinking in similar paths. Basically, the first step would be to publish new versions of ocpn-plugins.xml and provide means to install it. That should be easy to use and not that hard to maintain.

Besides this there are two issues: how plugins are created/built and how they becomes available in opencpn.

As for the first part, plugins are not available until there is a xxx-plugin.xml available which also means that the corresponding tarball is available on the web. As of now, this is only true for my four forks. I have submitted PR:s to the plugin upstreams, some have made some progress but there is still none ready to provide usable plugins in this context. I cannot see any way around this, it's work to be done in each plugin and it will take some time.

As for the second part it falls into two directions. The first is to make it easier to regenerate ocpn-plugins.xml. This requires that we distribute plugins/metadata and plugins/icons with the program and also adds scripts which supports the process better, not least on windows.

Another direction would be to drop the idea of a single xml file and instead just have a number of xml files, more or less the same as plugins/metadata. If so, users could just modify or drop a new file into the directory to make it available. It's not a technical problem, it's more how we want things to work -- Dave has expressed some interest in having a degree of control of the "approved" set of plugins so to speak.

Thoughts?

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

The present ocpn-plugins.xml needs some typo tweak

Will add to initial comment "open issues". Probably a single copy-paste error in squiddio-plugin.xml.in. Good catch!

@rgleason
Copy link
Collaborator

rgleason commented Sep 25, 2019

Should the "+" button try to update ocpn-plugins.xml before presentation of the available list of plugins?

Drawing from Chartdownloader plugin user experience, it is like updating the "catalogue" but in this case I guess it is small enough to be automatic. Should there be a message that the ocpn-plugins.xml database was updated?

Not only think of adding plugins, but perhaps also removing them for various reasons, as they have not been updated yet during a major upgrade in wxWidgets for example.

Which leads to the desire to keep the old ocpn-plugins.xml for the previous version intact and useable, and to start a new xml file for the new Opencpn Version. Therefore I think there should be an additional designator for ocpn-plugins. ...perhaps Plugin API?

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

@Hakansv: All this said: Does it work for you?

@leamas
Copy link
Collaborator Author

leamas commented Sep 25, 2019

Should the "+" button try to update ocpn-plugins.xml before presentation of the available list of plugins?

It's not that I have any problems with this as such, on the contrary. But to be meaningful such a change needs input from the project maintainers -- it's really about defining the process for the approved set of plugins.

@Hakansv
Copy link
Contributor

Hakansv commented Dec 14, 2019

Very nice and understandable.
For me all are agreed.
I recently met Openplotter ver 2.0.17 and the provided installation tool is what I can understand about the same philosophy.

@leamas
Copy link
Collaborator Author

leamas commented Dec 14, 2019

Rebased to current master

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.

None yet

6 participants