-
Notifications
You must be signed in to change notification settings - Fork 9
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
TP Convert to 'opencpn-libs' submodule #332
Comments
If all plugins are going to use opencpn-libs shouldn't these really be available from the opencpn git repository? It may well be a good idea to do this, as you document above, I am just not convinced it should be in a non-opencpn git. Also, this whole process should allow different versions of the libs to be used depending on which version of OCPN that the plugins are being built for. I have found with the current version of OCPN 5.8.4 on a Pi 3B+ using binary signalK that the resource requirements are on the limit when following a route. The OCPN 5.6 did not have this issue. Trying to use the character version of signalK does not seem to 'turn off' the processing of digital signalK so is worse. I will investigate this when I get back to my dev machines. |
Thank you Jon. I am sure Dave will consider these suggestions. I need to send him this thread. |
Keep in mind that the plugin uses the opencpn-libs that are needed and the opencpn-libs folder that is on github is kind of virtual, or empty, just referring to the leams location. However I am finding that there is a downside in terms of disk usage for devs (about 87mb per plugin). I hope there is a more efficient way or process available. For 20+ plugins that is about 1.8gb for just one or two plugins it is not so bad. I could simply delete the local repos and git clone it when needed, but that takes some time and bandwidth. Alternatively I could just ignore the problem and continue as I have. Dave wrote:
Since I will have about 77mb of opencpn-libs x 20+ plugins on my local machine, one way to save space is to
to recreate opencpn-libs. Of course this will require downloads of 77mb. I also asked Dave if he knew if I could just copy the opencpn-libs directory to the next plugin. |
I've just checked my source/shipdriver_pi, vdr_pi and logbook_pi local repositories and they simply have a directory called "libs" with the various sub-directories for what I think is only the libs that are needed by the plugin. On the other hand, I think I need to implement the opencpn-libs folder in Vdr_pi and LogbookKonni_pi because they are still using the older "libs" directory. ..also as Dave notes, they might have "private" libs, so beware when making this change. Also I see that my remote has opencpn-libs submodule https://github.com/rgleason/shipdriver_pi I wonder if this is the way the TP plugins are supposed to work? Or should I be updating my local Shipdriver plugins??? It might be better if the plugin just downloaded what was needed in the way of libs or opencpn-libs but that may not be possible. |
"they simply have a directory called "libs" with the various sub-directories for what I think is only the libs that are needed by the plugin." Hmmm. My shipdriver_pi contains only submodule reference. I do not see libs in Rasbat's git repo. Must be some old cruft ? However, if present, these "libs" directories might contain extra source files that are private to the plugin build, That is, they are not used by any other plugin, so don't really belong in the common opencpn-libs. |
@jongough Not sure I understand your concern here. |
Dave, I asked mike about the local version having libs directories. He says that is an old clone, delete the local dir and clone it again. Rasbats/shipdriver_pi#560 (comment) So my question re shipdriver plugins was answered. |
Prevent Appveyor from building. I believe there are a number of ways to do this. Plugin github > settings > webhooks > Delete then Save --- Works. but but then it has to be setup again. |
Examples of typical fixesFindit_pi Correct Android Build Errors Autopilot_route_pi Correct Debian Build Errors Plots_pi Fixes macos, debian, flatpak ubuntu about 10 builds failing First do this in cmakelists.txt
First # try it, then do static
Celestial_navigation Correct Android and MacOS builds #18 Deviation Correct Android build Deviation Correct all builds Pypilot More good examples Enable fPIC, Add wxServDisc to main build dir avoiding linkage problems |
There is now a disconnect between WeatherRouting_pi on github with opencpn-libs and what is actually in opencpn-libs. The version within WeatherRouting_pi has opencpn-libs/odapi but in the real opencpn-libs it apears to be opencpn-libs/ODAPI. This may not matter on windows, but on linux it is case sensitive. |
I seem to be fighting this process as the opencpn-libs stuff seems to be under active development which is invalidating some of my pushes. I would have hoped there would only be additions now not name/case changes. |
ODraw has an updated version of pidc with more methods for handling the stuff that OD needs. Is it worth picking this version up and making it the pidc version? The OD version is based on the pidc one originally. |
Jon, Dave is working on this now!!! |
Email Dave about this, he is pretty busy.
|
Closing, moved to Process and Examples. |
"ODraw has an updated version of pidc with more methods for handling the stuff that OD needs. Is it worth picking this version up and making it the pidc version? The OD version is based on the pidc one originally." I was waiting for your return to address this I do think it would be a good idea to pick up your extended pidc class in opencpn-libs. As mentioned elsewhere, you may now make this change yourself in the opencpn-libs "devel" branch, test it, and request a merge to branch "main" when ready, thus bringing it into production. Now would be an ideal time to make that update. |
@jongough I hope you saw bdbcat's post as I had closed this! |
Yep, working on the OpenCPN/opencpn-libs now. |
Yes.
to
|
Dave, I can do that today.
There is some overlap in that change ODAPI to lower case, but should I also make the other changes as done in the commit? It would help to have a clarification before starting this process. Also, should I push the changes up to my github repos sublibs? or just keep the commit local for the time being? |
The update for odapi is good,
This seems cleaner to me, since it becomes part of the common template instead of repeated for every plugin CMakeLists.txt. I would keep this all local for the time being. If you push them, the CCI builds will fail. No need for that confusion. |
Which testplugin repository did you make this change too? I have pulled Ricks version and applied it to mine to get it up to date. There is one change that is needed to make the built macos wx315 work with the xml, which is to not put the wx version into the target name. A one line change to PluginSetup.cmake. |
I did not push it anywhere. |
I believe my testplugin_pi sublibs branch is now up to date. |
Yes, I've just finished this list of changes " Last steps: before final submodule reload #343 " which is in Watchdog sublibs branch Msvc-wx32 did not build https://app.circleci.com/pipelines/github/rgleason/watchdog_pi/413/workflows/6e3293a0-b161-4ad1-84ec-9777264665ea/jobs/5442 Msvc did not build https://app.circleci.com/pipelines/github/rgleason/watchdog_pi/413/workflows/6e3293a0-b161-4ad1-84ec-9777264665ea/jobs/5441 I want to be sure I have this right before completing the change. In particular the changes to windowsheaders. Dave can you check the changes I made? Do these commit changes look right? |
Jon spotted this possible fix for Apple Dave, should I make that change too? |
Rick...
what I said was:
|
I put the Have made Jon's change. |
OK, great. |
Dave, now getting a number of these errors
See this recent push https://github.com/rgleason/watchdog_pi/commits/sublibs What did I do wrong here? as compared to what Jon had f9f45b5 |
Oh it is |
Yep. |
Probably my bad as I commented out the 'APPLE' and added the 'FALSE' as I was testing at the time to try and workout the issue with macos builds. |
It seems to be working now. Thanks all! |
I have a question about Climatology_pi CMakeLists.txt Can this be simplified? and maybe there is a better way to express the private zlib code? I haven't pushed these changes up, but the current commit in rgleason/climatology_pi Thanks |
These "final TP chngs" have been completed #177 |
Rick...
So these kinds of "on-off" dependencies belong in the CMakeLists.txt file, however complicated that appears. Dave |
Thanks Dave. Everything is so clean, I thought perhaps there was a way, but this is perfectly ok as it is. |
Rick... For each plugin, do this: A. Remove the current opencpn-libs submodule linkage.
B. Add the new linkage
C. Push all Let's do one plugin fully, and verify the process. Maybe testplugin_pi? |
Made a batch file link-rm.bat
Made link-add.bat and (at bottom of link-rm.bat)
|
After using
When I
Haven't I already removed it with
|
Isn't this a less brutal way to simply remove submodule.opencpn-libs?
|
Dave I've done testplugin and pushed it. I've gone through these commands and think one of them is redundant.
|
link-rm.bat
link-add.bat
|
OK, I think I've got this now, watchdog has been done too. |
I checked testplugin_pi, reading the logs. Let us carry on. |
Had to reload submodule "data" in Climatology_pi |
Convert to opencpn-libs submodule
Create a new branch called "sublibs" (locally)
In root directory of plugin, do:
1.1. This adds the "devel" branch of leamas' opencpn-libs repo to the plugin project as a submodule.
1.2. It also immediately downloads opencpn-libs, and places it in the plugin project root directory.
1.3 This needs only to be done ONCE.
1.4. Also, DO NOT add the raw contents of opencpn-libs to the plugin project git tree.
1.5. Submodules use only a reference to the github resident version of the submodule.
Edit CMakeLists.txt in plugin project to change all instances of "libs/x" to "opencpn-libs/x"
Use the syntax shown in watchdog_pi for adding directories from the opencpn-libs submodule.
You may test build locally from the plugin project root as normal.
When satisfies with local builds, got to Old src structure -- update to new? #3.
Review the CMakeLists.txt changes made in Watchdog Weatherfax, Weather_routing, Testplugin for submodule changes including:
Removal of set(libsrc, set(libhdrs,
Change the set(src collection line to remove libsrc
Remove include directories BEFORE (not the include directories BEFORE Project_Source include.
Change add sub_directory to use ocpn-libs.
Review weatherfax, weather_routing, testplugin changes too.
Reference Commit rgleason/watchdog_pi@d0ce572
This adds the required sublibrary command to all build scripts.
Build Locally
For your local builds, after doing:
This is exactly what the CI process does. It will bring in a fresh copy of opencpn-libs from github.
It does not change the git repo contents at all, since you already have the git reference to opencpn-libs.
You generally need to only do this once (for each plugin), unless opencpn-libs is changed by me or Alec.
We will let you know if that happens.
Remember: treat opencpn-libs as a read-only library. Do not edit. Do not commit contents.
Dave
More rules about submodules
git submodule update --init opencpn-libs
If you follow these rules locally, then you will be following exactly the same process as CCI does in its scripts.
Remember, CCI is the "golden build", from which production plugins are released.
I'm sure you know all this, so just reminders.
Regarding Watchdog opencpn-libs Issue
from Dave
More research, working on the flatpak builds. You need to do this for each plugin:
In ci directory, add this line to build scripts after each submodule update.
git submodule update --init opencpn-libs
git submodule update --remote --merge opencpn-libs <-- ADD THIS.
Then commit and push the results. This will fix all builds except flatpak.
To fix failing flatpak builds, for each plugin that you have IN PROCESS that fails flatpak on CCI, from source root, do this:
$ cd opencpn-libs
$ git checkout devel
$ git pull origin devel
$ cd ..
$ git add opencpn-libs
$ git commit -m "Updating the submodule 'opencpn-libs' to the latest version"
$ git push origin sublibs
For plugin that you have not started yet, you only need to do this, as described in the adaptation guide.:
Lets confirm this on watchdog, and then move on.
Changes to opencpn-libs (notice from Dave or Alec)
When the change lands in opencpn-libs main branch, you will need to (for these two plugins only):
$ git submodule update --remote --merge opencpn-libs
$ git add opencpn-libs
$ git commit -m "Update opencpn-libs submodule"
$ git push origin master (or other branch as you decide)
The text was updated successfully, but these errors were encountered: