mauroc / squiddio_pi

squiddio_pi
3 stars 13 forks source link

flatpak builds broken #73

Closed leamas closed 4 years ago

leamas commented 4 years ago

After merging #63 the flatpak builds are broken. I would appreciate if @jongough could take a look at what's going on. It fails in configuration, in cmake/PluginConfigure which somehow doesn't seem to work in the flatpak environment.

jongough commented 4 years ago

Why are you moving packaging back into the main CMakeLists.txt file? This should really be in the PluginPackage.cmake file as it is dealing with packaging. If it does not work then we need to fix it there not clutter up the main file.

leamas commented 4 years ago

I have added a comment to CMakeLists.txt. In short, I understand your concerns.

I had to sleep on it before I recalled why it's so early. The problem is that on a flatpak build, libraries such as OpenGL and wxWidgets are only available in the flatpak sandbox, not in the host. This means that as soon as there is something like FindLibrary() or FindWxWidgets() the build will fail. This is what happens in the build of the #63 PR at https://app.circleci.com/jobs/github/mauroc/squiddio_pi/102

The solution is to run the flatpak build before any FindLibrary etc. This means that it must run before PluginConfigure (or perhaps at top of it).

Note that current code just don't build and shouldn't really have been merged for that reason. That said, I could happily revise this according to your instructions as long as it builds. I'm perfectly aware that you have the overall view at this point.

jongough commented 4 years ago

Isn't that rather like the macos docker builds? You just need to download and install the requisite packages into the docker environment before trying to do the build. The script you have should be able to do that prior to the cmake process being called.

I am investigating setting up a virtual docker environment with the flatpak-builder environment available to see if I can see what is happening. Not sure how I will go as this is all new to me.

leamas commented 4 years ago

No, it's not. The build is done without any libraries in the host environment at all. The libraries comes mostly from the flatpak SDK and the the opencpn flatpak runtime.

The key issue is that flatpak is a sandbox separated from the host environment both at build time and runtime.

jongough commented 4 years ago

That is the way travis builds macos stuff. That is why there is such an install process needed to get it all working. There really is nothing in the travis environment for mac. There should be no need to rely on anything that is installed in the host as the build environment is self contained and goes away at the end of the build. So what is the problem in installing all that is needed into the docker image at build time? If you cannot customise the docker environment for builds then the flatpak process is rather limited. The fact that it is a sandbox does not matter, in fact it should allow you to install whatever you like without impacting the hosting system.

I have only just started to look at the flatpak process because you have said it is broken, so I have quite a bit of learning to do.

leamas commented 4 years ago

The basic problem:

Note that the simple solution to run the flatpak build before PluginConfigure.cmake Just Works (tm). The alternative would be to conditionalize PluginConfigure to not run in the flatpak case. IMHO, it's not worth it.

leamas commented 4 years ago

I have only just started to look at the flatpak process because you have said it is broken,

It's not broken. On the contrary, the block guarded byif (OCPN_FLATPAK) happily builds flatpak as obvious from this PR. It is more or less self-contained besides the variables from PluginSetup and the new generated file pkg_version.sh

What's broken is the attempt to make a "normal" package which is made if the code in PluginConfigure is run (possible also later stuff) in the flatpak case. There is just a need to make sure that this isn't run in this case. And that's it.

jongough commented 4 years ago

I must be missing something here. The flatpak stuff that I have moved is in the cmake files and to run cmake you need a development environment available. So flatpak has a compiler, cmake, etc. but does not allow you to install anything extra into the flatpak environment.

Anyway, looking at the flatpak code in cmake it has a return() statement at the end of it which causes the process to return to the parent process. In the case of calling from CMakeLists.txt it will return to the caller. If the processing of cmake is the issue then we can put an if statement in that will bypass all the other processing and end up in the PluginPackage.cmake file, which will then return as soon as the flatpak stuff has been done. If we just need to run the flatpak stuff and not everything else we can put if statements at the start of each included cmake file to return immediately if it is a flatpak process.

The idea is to 'hide' as much of the workings as possible from plugin developers and to be able to copy most of the stuff from one plugin to the next modifying only a couple of files, i.e. .travis.yml. CMakeLists.txt, appveyor.yml, etc.

jongough commented 4 years ago

I have just created a pull request for a change that will only run the PluginPackage.cmake and PluginSetup.cmake files when it is a flatpak build. This should then allow it to work.

Is there an easy way to check out if it does work?

leamas commented 4 years ago

You should setup circleci builds from you forked repo. Get a circleci account and start building your repo. It won't deploy without a cloudsmith setup, but it will build.

rgleason commented 4 years ago

https://github.com/mauroc/squiddio_pi/pull/76/commits/6fc71e0f54e09b94805339fa70dd4b40078c9ed7

jongough commented 4 years ago

I am not going to make any more changes as I am playing catch up and it would appear my changes are not wanted/required.

rgleason commented 4 years ago

Jon, I will do a test, I have gotten deploy to Cloudsmith working. We need your work.

Mauro we need you to be the semaphore guy, handing the batton to Jon or Alec after you get it back.

Fortunately I just posted information on how to setup Cloudsmith and Circleci accounts https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:developer_manual:plugins:plugin_installer

This needs some editing and there is some duplicate info, but it is not hard to do, it is just difficult when it is totally new environment. Any comments suggestions would be appreciated.

rgleason commented 4 years ago

Jon
Please see https://github.com/mauroc/squiddio_pi/pull/76#issuecomment-545213330 https://github.com/rgleason/squiddio_pi/runs/270822864 Maybe you can see where it failed from the report.

Mauro, please hold on merges while this gets sorted out.

rgleason commented 4 years ago

Comparing failed flatpak build https://circleci.com/gh/rgleason/squiddio_pi/110?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link to successful flatpak build https://circleci.com/gh/mauroc/squiddio_pi/110?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

It is hard to determine any difference! all the way to the bottom.

rgleason commented 4 years ago

I've created two text files, one from the failed flatpak and one from a mauro master successful flatpak that are useful when compared in notepad++ I don't know enough to figure out what the problem is. They are attached below.

flatpak-fail.txt flatpak-succeed.txt

leamas commented 4 years ago

I am not going to make any more changes as I am playing catch up

Don't worry, I'm on my way out ;) My primary concern is to get the installer running. I have become aware that there are deployment issues linked to this, though.

So, don't expect any big changes from me any more. I might take part in the deployment and ci issues, but foresee nothing but that.

Also note that this would not have happened if your last PR just didn't build. When things don't build it's quite common to make a short-term fix. I was aware that there was a need to polish things , this was acknowledged in the PR.

Things could have been easier for you if @mauroc had merged my PR some day earlier, but he perhaps has a life... who knows ;)

Also note that this happens all the time when doing open-source work. For example, there are now some 38 PRs pending for opencpn. Several of them will need to be rebased depending on in which order they are applied. For a more interesting case, consider the linux kernel. PRs are always targeted against a moving target, it's a simple fact of life.

Also, it doesn't mean that someone is stating that your work is not welcome. I for example see the value in it, but still think @mauroc has handled it properly as upstream maintainer.

So, as they say: Keep calm and carry on! Or, in a perhaps more modern wording: Keep up your good work!

leamas commented 4 years ago

Now, this issue is fixed by PR #75. Closing.

jongough commented 4 years ago

I understand that some changes made to git have to be redone, it happened quite a lot when doing OCPN 5.0. The difference here was that code changes were being made to exactly the same lines of code and there were collisions that could not be resolved. I did it a few times, but I have better things to do that keep on fixing up my changes. I may look at doing further work when I understand docker and flatpak better AND I see that the change rate has reduced.