Fix: correct urls to ones which are currently available.

[poetaster]

I'm not sure why the newer geoservices plugins aren't in use? The 5.8 branch, for instance, is all LGPLv2. Eg. https://github.com/qt/qtlocation/blob/5.8/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp This being the new 'cardinal approach' ...

This fix is not really an answer because hard coding tile servers was a 'bad idea (tm)'. BUT, it would at least make the QML maps work, afaik.

[vigejolla]

I'm not sure why the newer geoservices plugins aren't in use? The 5.8 branch, for instance, is all LGPLv2. Eg. https://github.com/qt/qtlocation/blob/5.8/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp This being the new 'cardinal approach' ...

The file you are ponting to only allows LGPLv3 or GPLv2, not LGPLv2.

[poetaster]

I'm not sure why the newer geoservices plugins aren't in use? The 5.8 branch, for instance, is all LGPLv2. Eg. https://github.com/qt/qtlocation/blob/5.8/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp This being the new 'cardinal approach' ...

The file you are ponting to only allows LGPLv3 or GPLv2, not LGPLv2.

Sorry, GPLv2 is an 'alternative' in the last stanza. But I'm not sure that matters.

This changeset, 5.5 branch, which was the first step in refactoring the geoservices plugins is from 2015 and allows for setting the urls:
qt/qtlocation@3f697f6

And those snapshots are LGPLv2. This would be a far better move but I'm not certain why 5.4 was chosen in the first place? The 5.5 branch is still LGPLv2.1 . EDIT: looks like 5.6 as well.

[vigejolla]

The file you are ponting to only allows LGPLv3 or GPLv2, not LGPLv2.
Sorry, GPLv2 is an 'alternative' in the last stanza. But I'm not sure that matters.

Yes, there is a big difference between GPL and LGPL.

This changeset, 5.5 branch, which was the first step in refactoring the geoservices plugins is from 2015 and allows for setting the urls: qt/qtlocation@3f697f6

And those snapshots are LGPLv2. This would be a far better move but I'm not certain why 5.4 was chosen in the first place? The 5.5 branch is still LGPLv2.1 . EDIT: looks like 5.6 as well.

If you take a close look at the files modified in the commit you specify, you will notice that some of them have already changed to new license. Sure, some of the files still have the old license. It's a mess really. 5.4 was chosen because it's the last version where all files allowed usage under LGPLv2.

[poetaster]

The file you are ponting to only allows LGPLv3 or GPLv2, not LGPLv2.
Sorry, GPLv2 is an 'alternative' in the last stanza. But I'm not sure that matters.

Yes, there is a big difference between GPL and LGPL.

This changeset, 5.5 branch, which was the first step in refactoring the geoservices plugins is from 2015 and allows for setting the urls: qt/qtlocation@3f697f6

And those snapshots are LGPLv2. This would be a far better move but I'm not certain why 5.4 was chosen in the first place? The 5.5 branch is still LGPLv2.1 . EDIT: looks like 5.6 as well.

If you take a close look at the files modified in the commit you specify, you will notice that some of them have already changed to new license. Sure, some of the files still have the old license. It's a mess really. 5.4 was chosen because it's the last version where all files allowed usage under LGPLv2.

I just checked all the files in the plugin itself and those are LGPLv2. The plugin code from 5.5 can be used under the lesser, version 2, for sure. The include which has a single modification accommodates copyright html display. It does look a bit like a trojan, which is not legal. But, it can easily be implemented by other means, if I'm not mistaken?

[poetaster]

I did the backport work last year already, and just did a meld here:
https://github.com/poetaster/qtlocation/tree/backport5.5

This is untested. I thought I would try building this on obs before I throw it at you.

EDIT: the build fails on:

[  155s] RPM build errors:
[  155s]     line 104: It's not recommended to have unversioned Obsoletes: Obsoletes:  qt5-qtlocation-plugin-geoservices-nokia
[  155s]     Bad exit status from /var/tmp/rpm-tmp.pZ3nQr (%build)

which corresponds to

Obsoletes:  qt5-qtlocation-plugin-geoservices-nokia
Provides:   qt5-qtlocation-plugin-geoservices-here

And, here is a rabbit hole, so to speak.

[  146s] mv -f libqtgeoservices_here.so ../../../../plugins/geoservices/ 
[  146s] make[4]: Leaving directory '/home/abuild/rpmbuild/BUILD/qt5-qtlocation-source-5.4.2/src/plugins/geoservices/here'
[  146s] make[2]: *** [Makefile:69: sub-geoservices-make_first] Error 2
[  146s] make[1]: *** [Makefile:73: sub-plugins-make_first] Error 2

Soooo. For the sake of testing what I actually care to test, I'm thinking of not building here at all. But I'm going to take a breather before wasting more electricity with obs.

[poetaster]

I can just drop this an concentrate on the 5.5 backports ?

[vigejolla]

I can just drop this an concentrate on the 5.5 backports ?

Well, that's your choice of course. But I think this PR is almost ready to be merged - I would just squash the commits and double check to make sure the new URLs work.

Your 5.5 backports branch contains quite a lot of changes, which means that also reviewing those will take considerable time. Especially since we have to be extra careful in order to not break the licensing rules.

[vigejolla]

Your 5.5 backports branch contains quite a lot of changes, which means that also reviewing those will take considerable time. Especially since we have to be extra careful in order to not break the licensing rules.

I guess I'm trying to say that I would prefer many small PRs over a single huge one.

[poetaster]

Your 5.5 backports branch contains quite a lot of changes, which means that also reviewing those will take considerable time. Especially since we have to be extra careful in order to not break the licensing rules.

I guess I'm trying to say that I would prefer many small PRs over a single huge one.

Yes, a big backport would be a pita. I've gone through it and I'm dropping it, since the actual change to have flexible URL specifications didn't actually make it in till 5.8.

As you suggest, after testing how/if this works I'll do incremental bits. Thanks for looking!

[vigejolla]

Tested these, they seem to work. I'm almost ready to merge this. There's just this one small thing, which I unfortunately noticed only now. I should have noticed it before, that's my bad. The issue is with the commit message formatting - we use [foo] format for changelog generation, which means that your [Fix] text there is going to look funny. Perhaps you could modify the commit texts so that they are in the format [qtlocation] commit message. JB#60940 ?

dismissing stale review

[vigejolla]

Discussed with Raine, came to conclusion that the commit message should not matter; I'll merge this as is.

[vigejolla]

Here's some documentation about the commit message format, for future reference: https://docs.sailfishos.org/Tools/Sailfish_SDK/Building_packages/#changelog-generation

Thank you for the contribution!

[poetaster]

Thanks a lot! The one [Fix] was inadvertent. I'm still not consistent but have in fact been trying to adopt the method you apply at Jolla!

[poetaster]

Just a last note here. I started implemented an additional paramter in: qtlocation/src/location/maps/qgeotilespec.* and I also looked a using parameters as in the Here plugin : ../here/maptiles/qgeotilefetcher_here.cpp