search

openstreetmap / merkaartor

Home of Merkaartor, an openstreetmap mapping program.
http://merkaartor.be/
Other
290 stars 79 forks source link

Add option to use system-wide qtsingleapplication #292

Open eclipseo opened 10 months ago

eclipseo commented 10 months ago

Only tested on Linux.

The goal is to use system-wide qtsingleapplication.

Krakonos commented 6 months ago

Thanks for the PR. Unfortunately, I merged the PR #294 this now has a conflict (easy to fix). Also, what distro actually has qtsingleapplication system-wide? Anyway, I think the correct way to do this is to build the internally bundled qtsingleapplication as a static library and then link it or the system one using the same mechanism.

gbschenkel commented 4 months ago

QtSingleApplication is deprecated for more then 10 years, is from Qt4, it doesn't "exist" on Qt5 and Qt6. This guy has ported the concept to Qt5 and Qt6. https://github.com/itay-grudev/SingleApplication

eclipseo commented 3 months ago

Built in https://copr.fedorainfracloud.org/coprs/eclipseo/exiv2-0.28.2/build/7493845/

Works with Qt6.

Krakonos commented 3 days ago

@kkofler Thanks for looking into it.

@eclipseo However, I'm in doubts if it makes sense to do this work. Is there any benefit to using the system-wide lib? It's reasonably small, it's already included in the sources and necessary for other platforms. Adding a bunch of conditional compilations introduces possible complexity and possible bugs.

kkofler commented 3 days ago

For some context: I am the primary point of contact for the Fedora package of Merkaartor, for which @eclipseo has written this patch. I have built Merkaartor 0.20.0 updates for Fedora yesterday: https://bodhi.fedoraproject.org/updates/?packages=merkaartor

In Fedora, we try to build against system libraries whenever possible, see:

That is why the Fedora package carries this patch.

How I have noticed the fault that I have commented on (the line that needs to be removed) is that I have made the system QtSingleApplication usage optional in the specfile in order to be able to support a Qt 6 build of Merkaartor also on older Fedora releases where the system QtSingleApplication is only built for Qt 4 and 5, in particular, the "old stable" release Fedora 39 that has about 2 months of update support left. (The current Fedora 40 has a system qtsingleapplication-qt6 available.) I could have just applied the patch conditionally, but since the patch is supposed to allow building both with and without system QtSingleApplication, I did just that, which is how I noticed that the build fails without system QtSingleApplication because of a bogus -lQtSingleApplication that is not available.

Also note that QtSingleApplication itself bundled QtLockedFile, which is also packaged separately in Fedora (and the system QtSingleApplication is built against the system QtLockedFile), so we are technically speaking about two bundled libraries, not one.

Krakonos commented 2 days ago

@kkofler Thanks for the explanation. I understand the motivation, but though in case of unmaintained libraries, I don't think it's reasonable to ask users/packagers to keep them in the distribution.

In any case, I'll review in more detail and see what needs to be done. I don't fancy adding more conditional compilation here, so I'd rather just create a subproject of the singleapplication and link one or the other.