Allow installing qt5 and qt6 versions in parallel

[DarthGandalf]

Why did you decide to open the library at runtime instead of linking to it at build time? The code would be much simpler then

[10110111]

Why did you decide to open the library at runtime instead of linking to it at build time?

This library was conceived as an alternative atmosphere model for Stellarium. And there the point was that if something goes very wrong, it'll be possible for the user to simply delete the file and use the legacy model. With direct linking it wouldn't be possible.

Besides, this lets me be less strict about binary compatibility, at least at this stage before version 1.0. The app can just check its ABI against the library ABI and give a sensible error message if it's not compatible.

[DarthGandalf]

And there the point was that if something goes very wrong, it'll be possible for the user to simply delete the file and use the legacy model

I see, makes sense.

The app can just check its ABI against the library ABI and give a sensible error message if it's not compatible.

This is not enough, unfortunately. Because you use std::function etc in the API, it's possible to e.g. build ShowMySky with libc++, but Stellarium with libstdc++ (or a different version of the same library/different compiler settings), and there still will be ABI issues.

I'm trying to make it possible to use the library without installation, instead building as part of build process of Stellarium, which makes that failure impossible. Of course, without breaking the other requirement

[10110111]

Because you use std::function etc in the API, it's possible to e.g. build ShowMySky with libc++, but Stellarium with libstdc++

Indeed it's possible. But for now the most problematic has been Qt, because Stellarium aims at supporting both 5 and 6, and apparently this is going to continue for quite long.

I'm trying to make it possible to use the library without installation

What exactly do you mean by installation — make install? Or something further like dpkg -i showmysky.deb? And how should the renaming into ShowMySky-Qt{5,6} help with this?

[DarthGandalf]

I mean `add_subdirectory(showmysky)` in cmake project which uses it. Renaming is independent from that change, and this PR is ready as is already. Renaming is literally to be able to have both builds simultaneously. I'm packaging Stellarium for Gentoo, and with the new version 1.0 trying to package the new dependencies as well. Gentoo allows installing qt5 and qt6 in parallel. Probably other distros allow that too, I haven't checked. But depending on whether Stellarium is built with qt5 or 6, it needs different builds of showmysky. And as it's a different package, the choice for Stellarium can't affect choice for this library. Or if you imagine a situation where some other client of it (non-stellarium) required qt6 build of showmysky specifically, we shouldn't force qt6 onto Stellarium too ср, 5 окт. 2022 г., 10:51 Ruslan Kabatsayev ***@***.***>:

…

Because you use std::function etc in the API, it's possible to e.g. build ShowMySky with libc++, but Stellarium with libstdc++ Indeed it's possible. But for now the most problematic has been Qt, because Stellarium aims at supporting both 5 and 6, and apparently this is going to continue for quite long. I'm trying to make it possible to use the library without installation What exactly do you mean by installation — make install? Or something further like dpkg -i showmysky.deb? And how should the renaming into ShowMySky-Qt{5,6} help with this? — Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPLZHESUBNLJLWXOLN5OLWBVFQTANCNFSM6AAAAAAQ5AI7DE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[10110111]

How would parallel installation of qt5 and qt6 builds of Stellarium work in Gentoo? Or is it only supported for libraries? I've never seen such a feature on purely binary distributions like Debian or Fedora.

Yes, I do understand the usefulness of this for libShowMySky, but I don't quite like the renaming solution. Maybe it could be optional, like passing a custom suffix to cmake? Is there any traditional way of doing this (on Gentoo or anywhere)?

[DarthGandalf]

ср, 5 окт. 2022 г., 13:39 Ruslan Kabatsayev ***@***.***>:

How would parallel installation of qt5 and qt6 builds of Stellarium work in Gentoo? Or is it only supported for libraries? I've never seen such a feature on purely binary distributions like Debian or Fedora.

Yeah, this is mostly for libraries. When you execute /usr/bin/foo, that's still a single file, though it is possible to duplicate them too via renaming. I see Ubuntu can install qt6 itself in parallel with qt5: https://packages.ubuntu.com/jammy/amd64/libqt6core6/filelist

Yes, I do understand the usefulness of this for libShowMySky, but I don't quite like the renaming solution. Maybe it could be optional, like passing a custom suffix to cmake? Is there any traditional way of doing this (on Gentoo or anywhere)?

The client applications would still need to know how to find the library. Eg what to pass to cmake find_package, or what package to ask from pkg-config, etc. I mean, this should be synchronised between library and app using it. So I was planning to send the patch to Stellarium for the new name in find_package after this PR. What do you suggest instead of renaming?

…

— Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPLZHMBPTKABV5FV7ONALWBVZG3ANCNFSM6AAAAAAQ5AI7DE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[10110111]

What do you suggest instead of renaming?

This is the problem: I've never seen a precedent*, so I don't know how it can or should be done. I'd like to learn how similar cases are typically solved for other libraries. Usually a single system uses a single toolchain for an application and its dependencies.

^{* Well, there's Poppler, which has bindings for Qt{3,4,5,6}, glib, and just C++, but it also has a common library libpoppler.so, so this seems not to be an example.}

[DarthGandalf]

I found https://github.com/stachenov/quazip/blob/master/QuaZip-1.x-migration.md as a precedent, though they renamed library and include, and even what you pass to find_package, but didn't rename the imported cmake target. Here I didn't rename include, because it's the same for both, but renamed the cmake target to be able to import both in the same cmake project; not sure if it's ever useful though. Ubuntu still package only the old version of QuaZip, but Fedora has it https://src.fedoraproject.org/rpms/quazip/blob/rawhide/f/quazip.spec Tbh, most Qt projects I know are not libraries, so they don't have this concern at all. If you don't like this change, I guess I'm fine with applying something like this patch locally for Gentoo for now. ср, 5 окт. 2022 г., 14:45 Ruslan Kabatsayev ***@***.***>:

…

What do you suggest instead of renaming? This is the problem: I've never seen a precedent*, so I don't know how it can or should be done. I'd like to learn how similar cases are typically solved for other libraries. Usually a single system uses a single toolchain for an application and its dependencies. * Well, there's Poppler, which has bindings for Qt{3,4,5,6}, glib, and just C++, but it also has a common library libpoppler.so, so this seems not to be an example. — Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPLZHDP2JWFH4GTBGROZDWBWBAJANCNFSM6AAAAAAQ5AI7DE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[10110111]

If you don't like this change, I guess I'm fine with applying something
like this patch locally for Gentoo for now.

I'm leaning towards accepting it, but need a few days more to think of better ways. In particular, I'd move the library name #ifs into the header rather than inserting it into the user code. E.g. put the following next to the ShowMySky_ABI_version definition in ShowMySky/api/AtmosphereRenderer.hpp.

#if QT_VERSION_MAJOR == 5
# define SHOWMYSKY_LIB_NAME "ShowMySky-Qt5"
#else
# define SHOWMYSKY_LIB_NAME "ShowMySky-Qt6"
#endif

[DarthGandalf]

E.g. put the following next to the ShowMySky_ABI_version definition

Good idea. Done.

[10110111]

Is it feasible to make the target to query properties from be called ShowMySky::ShowMySky instead of ShowMySky::ShowMySky-QtX, while the project remains ShowMySky-QtX? Otherwise it seems like unnecessary verbosity in application's CMakeLists to list -QtX everywhere. It's quite unlikely that a single application will simultaneously link against both Qt5 and Qt6, so ShowMySky::ShowMySky seems quite unambiguous already.

[10110111]

My variant that satisfies the need to have Qt5 and Qt6 versions can be seen in my branch DarthGandalf-qtver. It includes your patch and a few tweaks. If you are OK with it, let me know and I'll merge it.

[10110111]

In fact, it would be the best if you updated your branch to my version so that I could merge it from GitHub UI, otherwise it'll likely not recognize the PR as merged due to the conflict resolution.

[DarthGandalf]

The branch does work. The difference in functionality is that the target added by find_package(ShowMySky-Qt5) is now called ShowMySky::ShowMySky. Which means that it's impossible to have a cmake project which creates 2 executables: one with ShowMySky-Qt5 and another with ShowMySky-Qt6. I don't know whether it's a big concern though.

[DarthGandalf]

otherwise it'll likely not recognize the PR as merged due to the conflict resolution.

I really don't care. You can close it then by hand. Deciding whether that issue is bad enough or doesn't matter is more important than how exactly the PR is closed :)

[10110111]

Which means that it's impossible to have a cmake project which creates 2 executables: one with ShowMySky-Qt5 and another with ShowMySky-Qt6.

Well, this would be a very strange use case, which can be solved by running two separate builds. Having two Qt versions in the same build is a truly rare case.

OK, so I'm merging it.

[10110111]

Merged.

[DarthGandalf]

Yep, that's hopefully a rare case.