sailfish-keyboard / sailfishos-presage-predictor

Presage based input predictor for the Sailfish OS
https://openrepos.net/content/sailfishkeyboard/maliit-plugin-presage
GNU General Public License v3.0
7 stars 4 forks source link

update rpm spec to include license and url #15

Closed rinigus closed 6 years ago

rinigus commented 6 years ago

Small cleanup, addition of license and URL as well as carrying over your changelog. Tested and all builds at OBS

rinigus commented 6 years ago

Looks like I am still missing something in the build env. I will have to tune it a bit further. If this message arrives before merge, then please wait. If not, I'll review all tomorrow.

Right now, macros are jumping when spec is regenerating and sqlite link is missing

rinigus commented 6 years ago

OK, looks I am getting there. I'll update tomorrow evening the build scripts with sqlite linking added

rinigus commented 6 years ago

I think that the issue with SQLite linking is resolved and the compiled presage works now when using the scripts from this PR.

SQLite linking has to be explicit since we use static linking and libpresage dependencies are not resolved automatically by ldd. So, any dependency introduced in libpresage needs to be specified in the plugin .pro and .yaml.

The second issue which remains is the %post and %postun scripts. When having them in macros section, YAML preprocessor and sb2/mb2 as well as OBS building engines get confused and refuse to compile the package. I would suggest to skip these scripts for now. Later, we can convert all build into plain SPEC and then specify them. Alternative would be to make the conversion now and drop YAML. How would OBS react to that, I don't know yet.

So, unless you want to keep %post and %postun, this PR is ready for merge.

martonmiklos commented 6 years ago

I have added the maliit restart because without it the keyboard with predictor does not show up after installation and this is not something what I call user experience in 2018 ;).

rinigus commented 6 years ago

:) true. so, should I then look into how to remove YAML and move all build to SPEC? That's probably not the best supported option by QtCreator though. We can also ask at sfos-dev list, maybe someone knows the trick to enable it nicely.

martonmiklos commented 6 years ago

By reading through the spectacle docs: https://github.com/mer-tools/spectacle It seems that I have used the wrong section: instead of macros it should be install pre/install post. If you could give it a try that would be great, I can give it a try at the evening.

rinigus commented 6 years ago

I did try it in "install post" yesterday and it wasn't happy about it either. But looks from the spectacle spec, I can add << post which should work. I can test later tonigh

rinigus commented 6 years ago

Thanks for digging up spectacle specs! I have added the scripts. Small modification due to the fact that the scripts are run as root: had to use su - nemo to make them restart user's keyboard (its running under nemo systemd services).

Tested on phone and got maliit service reloaded on install.

From my side, its ready now for PR with all functionality that we wanted in 2018 :)

martonmiklos commented 6 years ago

Many thanks for figuring this out! I do not have time to test atm, but I trust the fact that this is working.

rinigus commented 6 years ago

It is :) . No problem, you have spent quite some time on this earlier :)

rinigus commented 6 years ago

I'd like to submit a new PR on the top of the changes in this one. If its fine, would you mind to merge this PR so I can submit the next one for review?

martonmiklos commented 6 years ago

Ah it seems I have forgot to push the merge button.