Giter Club home page Giter Club logo

Comments (58)

a17r avatar a17r commented on August 18, 2024 1

My broad outline:

  • Get CMake done right. It is a well established standard among Qt5 projects.
  • Ditch qmake completely. Dual work for no obvious gain and potential for getting out of sync as you have seen in the past.
  • I would not make the move to yet another build system just because some IDE has support for it.

from quazip.

stachenov avatar stachenov commented on August 18, 2024 1

Maybe QuaZip should not set any defaults at all and leave it up to the user. That would make perfect sense for me, if I were a user.

from quazip.

lgbaldoni avatar lgbaldoni commented on August 18, 2024

Just brainstorming: since Qt Creator now (supposedly) supports meson, how about moving to it instead?

from quazip.

stachenov avatar stachenov commented on August 18, 2024

It's the first time I even hear about it. But since Qt6 is supposedly support CMake as the main build tool, the idea of using CMake plus qmake as a fall-back option seems reasonable enough.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

see #76 for a proof of concept CMake build

re naming:
We kinda arrived at something before. Everything should be inferable from the info in the PR (i didn’t list that explicitly separately)
The only non-intuitive/unexplained thing there is the libraries without ABI or Qt version (libquazip.so)

re CMake version:
It’s your decision in the end. It’s certainly not a good idea to start with an old version of CMake, might as well not do a CMake build system then.
But as i said before, stick to a target OS you want to support and then necessary boilerplate can be added to backport it to the version of CMake version there. I just like to start with the newest and best (since that’s the system i’m working on).

from quazip.

a17r avatar a17r commented on August 18, 2024
  1. We need to decide with CMake version. Modern CMake seems nice, but many distros still ship with retared CMake versions like 2.7. RHEL/CentOS 7 comes to mind as an example.

...so that's 2, but really 1, and that's probably about it. Don't chastise yourself to the CMake version shipped by the biggest slacker distribution. We know they exist; we also know they are typically not the ones who will ever think of packaging a version 1.0.0 of quazip many years after their release.

It is fairly easy to get an overview: https://repology.org/project/cmake/versions

An overwhelming majority of distributions are well into the 3.x series; Let's consider Ubuntu 14.04 as an odd but still valid choice for a desktop system in use today; it provides CMake-3.5.1 already. Which quazip version does it package? 0.6.2. They won't bother with your next release either. Developers using such a distribution are already going through great pain having to maintain a big dev environment that quite likely also contains a modern version of CMake.

KDE Frameworks 5.72.0 currently has a minimum CMake version of 3.5.
nomacs, an image viewer using quazip: 3.0

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Fully agree about getting it right. But we need to come to some agreement on what right is. This is exactly what this issue is for.

Qmake support is intended for legacy systems. Pretty much the only reason QuaZip exists at all is because I needed it for my work. I'm not sure I can migrate to CMake at work because of the reasons mentioned above. With many different distros and no Internet access... much easier to just continue to use CMake. And the only reason CMake was getting out of sync (or was never in sync, to put it right) is that all CMake stuff was contributed. I didn't understand a thing about it, and I'm still in process of learning it. Once I figure out how it works, I'm going to keep qmake in sync with CMake. No problems here.

I tend to agree that going CMake 3.x is generally a good idea. I'm going to make some research on how easy it is to install it on a retarded distro (especially with no Internet connection) and what CMake versions are shipped with other distros I have to work with. Since we use a lot of different and mostly very old distros, I think it will be a good assessment of what people might need.

And I still need to learn CMake. Otherwise, I won't even understand most PRs and subtle details being discussed.

Re: naming, I think we've more or less decided on libquazip1-qt5.so, libquazip2-qt6.so and so on. But what about include paths, for example? I proposed this scheme:
include/quazip/quazip — Qt5/1.0
include/quazip-qt4/quazip — Qt4/1.0
include/quazip-qt6/quazip — Qt6/1.0
include/quazip2/quazip — Qt5/2.0
include/quazip2-qt6/quazip — Qt6/2.0
...but maybe it's not a good idea after all? Maybe make Qt5 explicit: quazip-qt5? Or maybe there's no need to put includes for different Qt versions into different directories because they are source-compatible anyway? That may cause packaging problems, though: instead of quazip-qt5 and quazip-qt5-dev we'll have quazip-dev that is common for both quazip-qt5 and quazip-qt6. Or is it not a problem at all?

from quazip.

a17r avatar a17r commented on August 18, 2024

see #76 for a proof of concept CMake build

In general I like what I see, but I don't see what CMake 3.14 is going to add in terms of # default install paths with GNUInstallDirs, over, say, 3.5. While my packaging would not mind at all such a minimum version, a library will in general always have more conservative requirements than leaf packages, see for example KDE Frameworks, which albeit having ditched pre-Qt 5.12 support already, still remains at CMake 3.5.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

My PR uses install/include/quazip1-qt5/quazip, assuming that anyone who doesn’t use CMake targets can figure out that it’s not simply include/quazip. It just makes it consistent.

Imo putting only headers in a separate, common, package is more annoying than it helps anything. Especially if e.g. Qt4 support get’s discontinued and the versions get out of sync.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Right... so:

  1. libquazip1-qt5.so.1.0.0, libquazip1-qt6.so.1.0.0, libquazip2-qt5.so.2.0.0 and so on.
  2. include/quazip1-qt5/quazip with both include/quazip1-qt5 and include/quazip1-qt5/quazip in INCLUDEPATH to allow for both #include <quazip.h> and #include<quazip/quazip.h>.
  3. Use CMake 3.x on as-needed basis. If a feature is useful, just bump the required version, if not, just keep it reasonable. Gotta read that Modern CMake book first, though.

Anything else?

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

@a17r default locations for the install(TARGETS command based on the GNUInstallDirs package were only added in 3.14

Previously every script coded this manually. I’m happy this boilerplate is a default now, although i can understand that that’s not a good enough reason for some.
I actually don’t know the version requirement for any of the other stuff, that’s just the thing that was added most recently, which is why i put it in a comment next to its minimum version.

from quazip.

a17r avatar a17r commented on August 18, 2024

include/quazip/quazip — Qt5/1.0
include/quazip-qt4/quazip — Qt4/1.0
include/quazip-qt6/quazip — Qt6/1.0
include/quazip2/quazip — Qt5/2.0
include/quazip2-qt6/quazip — Qt6/2.0

That's being overly generous to distributions. Not that I'm opposed to it, but is it worth the added complexity over simply making -qt4, -qt5 and -qt6 coinstallable?

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

It works fine like that. Where is the complexity (putting the ABI version into the include path, i assume) a problem?

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

@stachenov if you continue to support the qmake build and you simply build a libquazip.so there, then that one could be removed from the CMake build. It is included atm, but i think it’s rather odd to have.

And i think it’s pretty crucial to just choose/try to find a baseline distro now. I have at least Ubuntu LTS VMs at hand, but as i said i’m not gonna blindly start with the oldest.

from quazip.

a17r avatar a17r commented on August 18, 2024

It works fine like that. Where is the complexity (putting the ABI version into the include path, i assume) a problem?

You will also have to reflect that in the pkgconfig (and cmake) files needing to be coinstallable. It just does not have all that much precedence, combined with supporting multiple Qt implementations at the same time. As a packager I'm used to the regular fallout on ABI breaks (think openssl, poppler, icu...) and getting all consumers fixed. Certainly not bad not having to deal with that.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I'm against building libquazip.so at all at this moment. I'd rather reserve that name to 0.9.x only. As well as the include/quazip dir. This way 0.9.x can happily coexist with 1.x and everyone using 0.9.x will continue to use it until they decide it's time to upgrade to 1.x.

And I don't see any complexity with these include paths. On the contrary, it'll be easier to me to keep things separate. Packaging aside, I'm thinking on how I can use it at work. Suppose I have two programs using QuaZip on the same server. One uses QuaZip 1.0/Qt5 and the other (newer) one uses QuaZip 1.0/Qt6. The newer one is under active development, so I decide to upgrade QuaZip 1.0/Qt6 to QuaZip 1.1/Qt6. If I keep everything separate, I can do it without even risking to break anything related to QuaZip 1.0/Qt5. I can keep both 1.0/Qt5 and 1.1/Qt6 on the same machine. That's a huge plus for me.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

hm, actually, atm the package is only coinstallable from a CMake POV, but still intended to be 1 (meta) package, since the CMake configs (are intended to) collide because it’s a selector for the different configurations.

pkg-config is something i haven’t really touched yet, that’s why it’s listed as todo in the PR, the main thing there is imo how far down to segment the names. If the name is more generic i think the build system would have to decide what it represents (which, in the PR, is just the newest version). But i just don’t know anything about pkg-config, so maybe it’s possible to implement some sort of selection mechanism.

from quazip.

a17r avatar a17r commented on August 18, 2024

As I said, you also need to manage all that from the pkgconfig and cmake config side, and if you even separate micro soversion bumps it recreates all that micro management on build systems of consumers of your library as well.

from quazip.

a17r avatar a17r commented on August 18, 2024

I can keep both 1.0/Qt5 and 1.1/Qt6 on the same machine. That's a huge plus for me.

But you can do that with only separating qt5 from qt6 already.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

As I said, you also need to manage all that from the pkgconfig and cmake config side

What is there to manage? If there is (binary) software that needs the old ABI, you can build the old package without installing the CMake and pkg-config files, those are reserved for the most recent version of the library, or available under a different prefix.

and if you even separate micro soversion bumps it recreates all that micro management on build systems of consumers of your library as well.

no. The point is that source consumers don’t select based on ABI version but simply by the name QuaZip + the Qt version.
Granted, for binary dependencies you don’t need headers.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Nobody wants to separate minor SOVERSION bumps, but major SOVERSION bumps will be separated naturally since QuaZip by design never breaks ABI compatibility without bumping the major version number of the project itself.

Speaking about coinstalling CMake files and naming... We also need to decide on QuaZip target names. As far as I remember, there was a choice between QuaZip::QuaZip for all versions, and QuaZip::QuaZip1 for 1.x and so on. What about CMake file names?

I'd rather go with QuaZip::QuaZip1 and FindQuaZip1.cmake. That way I can do whatever I want with QuaZip 2.x without worrying about breaking anything 1.x-related. And this way I don't have to deal with the current (messy) 0.9.x files.

But you can do that with only separating qt5 from qt6 already.

Right, and that's exactly what I was talking about. The first part, as in quazip1- or quazip2- is needed because major versions may be even source-incompatible, much like it is with Qt. So when (if) QuaZip 2.0 is released, it should never ever conflict with anything 1.x-related. It just installs in parallel and simply works independently.

from quazip.

a17r avatar a17r commented on August 18, 2024

Granted, for binary dependencies you don’t need headers.

That's what I was getting at - binaries will be fine. I don't see the application for header coinstability between soversions, the moment I see that I will expect being able to address them from pkgconfig or cmake. You do that if there is a major break and you expect your consumers to take a very long time to adapt, but even such a major package as openssl didn't do that.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

@stachenov atm the target name is QuaZip::Qt5

and with CMake you can specify compatibility to be SameMajorVersion while retaining a name without the version included

I don’t think that forces users to set the version though, maybe that has to be asserted manually if desired.

@a17r i think the idea of @stachenov is that major updates are intended to be very explicit for the user, i.e. you never get the new major version unless you set it somewhere. It’s less about ABI but more about API version, although they are combined here for simplicity.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

atm the SOVERSION is also only added to the library (and include) name on Windows

from quazip.

stachenov avatar stachenov commented on August 18, 2024

QuaZip policy is that major version updates are either ABI or API incompatible. I definitely don't want QuaZip 2.0 to be picked up automatically by the users. QuaZip is more or less in the maintenance stage, I don't update it much. The only big update that is planned is Minizip update, but that's not going to be any time soon.

So it's not about SOVERSION. However, if ABI breaks, I'll bump the major version too, maybe keeping source-level compatibility too. I don't see any problems with that either. If the user wants to update, they just replace QuaZip1 with QuaZip2 in their CMake files and there they go. But at least they don't have to deal with this kind of DLL hell when they simply recompile a part of their project and then it turns out that different libraries use different binary versions of QuaZip and that causes all kinds of trouble.

And I'd like to have a great degree of freedom when doing major updates. Surely, I'll try to keep things as easy to the user as possible, source-level. But that is one thing, and having to deal with 1.x legacy when releasing 2.0 is another. If QuaZip 1.x uses some CMake 3.33 by the time it reaches the end of its life, I'd like to have an option to ditch CMake files and rewrite them using CMake 6.66 or whatever version will be the thing those days. So QuaZip 1.x and 2.x can happily coexist without interfering with each other in any way.

Given all that, maybe the target name should be QuaZip1::QuaZip, not QuaZip::QuaZip1. Definitely not QuaZip::Qt5, that just doesn't make any sense.

from quazip.

a17r avatar a17r commented on August 18, 2024

Yet another data point for minimum CMake versions, since I was building it just now: Qt6 indeed has 3.15.0 there.

from quazip.

lgbaldoni avatar lgbaldoni commented on August 18, 2024

@a17r

I would not make the move to yet another build system just because some IDE has support for it.

I made that comment because I assumed @stachenov used that IDE to develop quazip.
In my limited experience, meson tends to produce less headaches than cmake, unless cross-compilation is involved.

from quazip.

a17r avatar a17r commented on August 18, 2024

That's not my experience. Yes, as a packager I have seen way too many badly written cmake, qmake and autotools build systems (deliberately ignoring waf and scons here, they shall not even be talked about), because they are treated as an afterthought, and I'm sure you can write a bad meson build system as well. Talking about [build systems] upstream, there have been breaking changes once in a while in cmake, but usually they are preceded by extended deprecation. On the other hand, the breakages introduced by every other meson release are indicative of it being a very young project needing time to settle.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I've finished reading Modern CMake. The good news is that CMake is very easy to install on pretty much any Linux machine, even by just copying the binary tarball and extracting it somewhere. So there's no point trying to restrict everything to the bundled CMake. I'm thinking about CMake 3.15 at this moment.

The bad news is that CMake is the worst build system I've ever seen. And I'm talking about modern CMake here. Really, install(...INCLUDES...) doesn't really install includes, install(...PUBLIC_HEADER...) (singular!) should be used for that... VERSION that goes to many places because project version and target version are apparently different things, and omitting the latter is not even a warning... and so on, and on, and on... And yet, it seems to be the most powerful, popular and de-facto standard build system. Guess I'll just have to bear with it.

One thing I'm not getting at this moment is how to separate CMakeLists.txt (and do I even need to do it). Right now it's a mess, so I'm not even looking at it. But how should it look? Do I just keep a minimum, like CMake version required and project name in the top-level file and leave target-specific stuff to local files?

Another thing is how to coinstall all those QuaZip1-Qt5 and QuaZip1-Qt6 together if CMake files are common. Do I just install copies to lib/cmake/QuaZip1-Qt5 and lib/cmake/QuaZip1-Qt6, perhaps with different config adjustments (when converting QuaZip1Config.cmake.in to QuaZip1Config.cmake)?

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

So, in addition to the name QuaZip, there is the API/ABI version as well as the Qt version.
In the PR they are currently implemented via the version as well as the components options of find_package instead of encoding it in the package name and target namespace (leaving that cleanly as QuaZip::).

find_package(QuaZip 1.0 REQUIRED COMPONENTS Qt5)

If the version is omitted it defaults to 0.0.0.0 so it should work if you try to use it with the 0.9 versions now (due to SameMajorVersion) but error once the version is 1.0 or later.
Imo this is better than putting a version in the namespace or component name.

re splitting: Since there is the library and test components it lends itself to separate those. I think commonly there is a CMakeLists.txt per directory which handles the folder’s contents, but it doesn’t have to be that strict. If there are just several subfolders to segment sources and there would be no logic required because there is no target there is perhaps no point in littering it with additional files.

re common files: In the PR lib/cmake/QuaZip/QuaZipConfig.cmake works as a dispatcher between the Qt version components and the static and shared builds that is installed for each flavor, so it would conflict if each flavor would to be a separate independent package (no surprise). The individual flavor files don’t conflict as they carry all distinguishing information in the filename e.g. QuaZipSharedQt5Targets.cmake and QuaZipSharedQt5Targets-release.cmake (that could carry the ABI/API version as well, but currently doesn’t – that would also require additional logic in the dispatcher).

PS yes CMake isn’t all that great…

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Dispatching works fine as long as both flavors are more or less the same version. If we have QuaZip 2.8 released, say, in 2030, which uses CMake 3.28 or something, and then by 2040 CMake 3.28 is rightfully thought of as a terrible mess, and CMake 5.2 is the thing now, then I'd definitely want to use CMake 5.2 syntax for QuaZip 3.0 and just install it in parallel with QuaZip 2.8, no common files needed.

Is it possible to resolve it with something like lib/cmake/QuaZip1/QuaZipConfig.cmake and lib/cmake/QuaZip2/QuaZipConfig.cmake, both of which provide QuaZip::QuaZip, but of different versions? Won't such a name clash break anything?

Re: Qt versions, then it would be impossible to install QuaZip 1.0/Qt5 and QuaZip 1.1/Qt6 in parallel? I don't really like it... I'd rather have two config files for different Qt flavors. But again, I'm not sure that's possible with CMake, to have two config files for the same package, but different build-time configurations.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

The dispatcher would always come from the newest build and could support loading older versions, but that’s something that would have to be implemented manually.
If you want different versions you can always install them to a different prefix. I’m guessing that’s how CMake intends it.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

I'd rather have two config files for different Qt flavors. But again, I'm not sure that's possible with CMake, to have two config files for the same package, but different build-time configurations.

that’s exactly what it is…

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

You can certainly circumvent using CMake features by encoding stuff in names instead (which is what the individual config files already have to do – but that’s not user facing), i’m just not a fan of it. And that’s how a lot of CMake goes: there is little actual rules and a lot of (different) »common« conventions…

Is it possible to resolve it with something like lib/cmake/QuaZip1/QuaZipConfig.cmake and lib/cmake/QuaZip2/QuaZipConfig.cmake, both of which provide QuaZip::QuaZip, but of different versions? Won't such a name clash break anything?

There is no ODR in CMake, so there is nothing inherently in the way.

If you really want to differentiate the major versions that hard you might as well append it to the project name (in CMake that is) which would make the next major version basically a different library altogether. I don’t think that’s great.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

that’s exactly what it is…

How so? There's one config file. I still don't get how to separate different major versions. find_package apparently uses the package name to construct both the path and the config file name: .../<PackageName>/<PackageName>Config.cmake. How to make sure they don't conflict at all on the file system level if it's the same file?

And one would think that installing two incompatible versions of the same library is such a common task that even the most retarded build system would offer a solution right out of the box...

I was thinking about just appending the major version to the project name. That isn't very great by itself and it doesn't solve the Qt5/Qt6 problem either (allow for upgrading one without even touching the other).

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Oh, wait. That's .../<name>*/, not .../<name>/. Hmm, maybe ...lib/cmake/QuaZip1-Qt5/QuaZipConfig.cmake is the answer then. And just use QuaZip::QuaZip without that ugly numbers in the names...

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I've just tried this on a simple non-Qt Hello World project. Worked like a charm. The 1 was only in two paths: .../include/HelloCMake1/hello-cmake/*.h and .../lib/cmake/HelloCMake1/HelloCMakeConfig.cmake. Then I did same thing with a 2 and wrote a simple app using one of the libraries. It happily used whatever version I asked for, no conflicts whatsoever.

Now for the Qt part... I think it should be fine too, but better make sure.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I couldn't get REQUIRED COMPONENTS to work with two packages installed in parallel. If I try to do it on the *Config.cmake level, then it fails because CMake only tries to load one config—the first that has a matching version. If I try to do it on the *ConfigVersion.cmake level, it fails because the *_FIND_COMPONENTS variable isn't even set at this stage.

So now I'm thinking about providing QuaZip::QuaZip-Qt5, QuaZip::QuaZip-Qt6 from separate CMake packages (built from the same source, of course).

from quazip.

stachenov avatar stachenov commented on August 18, 2024

All right, I got this part working. Had to change .../lib/cmake/HelloCMake1-Qt5/HelloCMake-Qt5.config to .../lib/cmake/HelloCMake-Qt5-v1/HelloCMake-Qt5.config because HelloCMake1-Qt5 didn't match HelloCMake-Qt5*, which is what find_package looks for. Otherwise, it works fine. Qt4 and Qt5 builds are installed as completely separate packages, and the only disadvantage at the user side is having to specify Qt5 twice: in find_package(QuaZip-Qt5) and in target_link_libraries(... QuaZip::QuaZip-Qt5). But that's a minor inconvenience well compensated by the total freedom of upgrades.

Now I just have to figure out how testing works and how to actually apply all this to a real-life project.

from quazip.

Optiligence avatar Optiligence commented on August 18, 2024

Now I just have to figure out how testing works

one way to do this would be to check out the PR… (starting to wonder why i even spend time discussing that before and creating it)

There's one config file.

If you don’t treat the flavors like separate projects (which is a little odd) there has to be at least 1 common/meta file to handle the project.
But the dispatcher works with any of the flavors (their config files) present or not.

.../<PackageName>/<PackageName>Config.cmake. How to make sure they don't conflict at all on the file system level if it's the same file?

It doesn’t matter much if they conflict, because the dispatcher is exactly the same in every flavor.

if it’s gonna be find_package(QuaZip-Qt5) the namespace should have the same name, i.e. QuaZip-Qt5::QuaZip-Qt5.


I couldn't get REQUIRED COMPONENTS to work with two packages installed in parallel

The package configuration file may set _FOUND to false to tell find_package that component requirements are not satisfied.

Therefore the files can also be installed in a way that allows them to be used like proposed in the PR while being duplicated in different directories.
Encoding any options as suffixes in package or target (namespace) names should be avoided if there is another way.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I've been reading the PR for quite a while now. But since I'm absolutely new to CMake some things are not so easy to understand. It's very helpful nevertheless, but I want to be sure I understand everything this time. Blindly accepting PRs got us where we are now with 0.9.x, which isn't great.

It doesn’t matter much if they conflict, because the dispatcher is exactly the same in every flavor.

Assuming the dispatcher never changes. If it's different in different versions of QuaZip and someone wants to install QuaZip 1.0/Qt5 and QuaZip 1.1/Qt6, we're going to have a problem.

if it’s gonna be find_package(QuaZip-Qt5) the namespace should have the same name, i.e. QuaZip-Qt5::QuaZip-Qt5.

Why? What's wrong with different packages having the same namespace? It worked just fine with my toy project.

Therefore the files can also be installed in a way that allows them to be used like proposed in the PR while being duplicated in different directories.
Encoding any options as suffixes in package or target (namespace) names should be avoided if there is another way.

Absolutely. But I don't see any other way. The PR installs one set of CMake files. If I install another QuaZip version for another Qt major version, it will, what, get overwritten?

The package configuration file may set _FOUND to false to tell find_package that component requirements are not satisfied.

Therefore the files can also be installed in a way that allows them to be used like proposed in the PR while being duplicated in different directories.

Nope, that doesn't work. CMake just stops on the first file. As far as I understand, it works like this:

  1. Find some config file.
  2. Check whether the version requirements are satisfied.
    3a. If so, proceed to loading this config.
    3b. If not, goto 1.

If any config file satisfied the version requirements, it just tries to load it. If it loads fine, then it is used. If not (e. g. *_FOUND is set to false), then another config file is not looked up. The search procedure just aborts. Here.

Therefore, if there are two QuaZipConfig.cmake files of different QuaZip 1.x versions in different directories, than one is chosen randomly and if it isn't loaded successfully, then QuaZip is treated as not found at all.

That is, unless I miss something. This CMake stuff is quite complicated and I'm only a noob here.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I've created the cmake branch, merged PR #76 there and done some work on it. I still don't understand all thinks CMake completely, so there were some cargo cult programming involved, but I did manage to get it working for Qt 5.11 / MinGW, Qt 5.9 / CentOS 7 and Qt 4.8 / CentOS 7. The last two were installed in parallel and I had no problem switching between them in a sample application.

This is by no means complete yet, but at least I got it working and in accordance with my current ideas of how it should work with modern CMake.

In the end, I settled with installation directories like QuaZip-Qt5-1.0 for both includes and CMake files, and named CMake packages like QuaZip-Qt4 and QuaZip-Qt5. But I left the target name as QuaZip::QuaZip because I realized it has nothing to do with file names and locations (these are related only to the package name, as passed into the find_package function). The downstream usage is now this:

find_package(QuaZip-Qt5)
target_link_libraries(... QuaZip::QuaZip)

Could anyone check out the cmake branch and proofread it for stupid mistakes, convention violations, portability (especially for MacOS)...?

And now I am seriously thinking about ditching qmake, if only to avoid confusion and unambiguous use with IDEs, so that one doesn't even have to wonder whether Qt Creator version X.Y tries to open it as a qmake project or a CMake project. Given that installing CMake is as simple as download-and-unpack, I see no reason to even try to support two build systems.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Merged cmake into master to close #76 and avoid confusion, because people commented on it like it was the latest version, which is not anymore. After all, the current state compiles and works just fine for now.

Current plans:

  • Instead of setting CMP0042, use a more general command to set all policies to 3.15 or something.
  • Sort out shared/static compilation. I don't like the way it is implemented right now. I'd rather make two separate targets with options to disable/enable each, so the user can build either or both. Use a common “objects” target to avoid compiling everything twice. Still not sure what should be built by default, but leaning towards both flavors. It's not like the static target takes too much space or causes any other kind of trouble.
  • Ditch qmake.
  • Anything else?

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Update:

  • Policies are implicitly set by cmake_minimum_required, therefore explicit policies are removed. Version range are set to 3.15...3.18.
  • Compiling once for both shared and static libraries aren't going to work because of imports/exports. At least I know of no way to make it work reliably on all platforms. Therefore, I'm leaving it as it is.
  • Qmake ditched successfully.
  • I'm not sure about release/debug builds. As it is, the build type is selected once and if it's the debug one, d is appended to the resulting binary name (libquazip1-qt5d.so). Everything else is the same, which means it's problematic to install both debug and release in parallel, which kind of makes that d pointless. And I don't like that name either because it looks like d is related to qt5 and not to quazip1. Maybe get rid of the debug postfix altogether? Or provide separate packages (or targets?) for release/debug builds?
  • TODO: update documenation.

Overall, I like how it looks now. Everything makes sense, I've tested with Qt 5.11 on Windows and Qt 4 and 5 on CentOS 7. Tried using both find_package and add_subdirectory, both seem to work just fine. After sorting out the debug/release stuff and updating the docs, I think it's ready for 1.0.

from quazip.

Highlife1911 avatar Highlife1911 commented on August 18, 2024

I have problems with the current dev. I tried to add QuaZip via add_subdirectory, but it yields to strange build failures. The QuaZip target builds perfectly fine but most targets are failing with linker errors. I am using Qt 5.15 and CMake 3.18.2

I will investigate this.

The problem lies in option(BUILD_SHARED_LIBS "" ON), which affects the global scope, even when build as a subdirectory. This results in building of shared libraries even when it is not supported by other targets. So the fix was to set it to off first in the parent project: option(BUILD_SHARED_LIBS "" OFF)

It is a little bit annoying that there is no way to set this property locally, but it is ok as it is working now with this change to the parent project.

from quazip.

jlgranda avatar jlgranda commented on August 18, 2024

Hi @stachenov and everybody. I add quazip for support zip files into (https://github.com/jlgranda/QField), when I try to build APK with NDK, no is posible build quazip, I added the last versión for cmake support and QField too in cmake. I try to generar STATIC or SHARED, but the problem persists.

[19/240] Linking CXX shared library android-build/libs/armeabi-v7a/libquazip1-qt5_armeabi-v7a.so
FAILED: android-build/libs/armeabi-v7a/libquazip1-qt5_armeabi-v7a.so
: && /opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ --target=armv7-none-linux-androideabi16 --gcc-toolchain=/opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fPIC -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -march=armv7-a -mthumb -Wformat -Werror=format-security -Oz -DNDEBUG -Wl,--exclude-libs,libgcc.a -Wl,--exclude-libs,libgcc_real.a -Wl,--exclude-libs,libatomic.a -Wl,--build-id -Wl,--fatal-warnings -Wl,--exclude-libs,libunwind.a -Wl,--no-undefined -Qunused-arguments -shared -Wl,-soname,libquazip1-qt5_armeabi-v7a.so -o android-build/libs/armeabi-v7a/libquazip1-qt5_armeabi-v7a.so 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/QuaZip_autogen/mocs_compilation.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/unzip.c.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/zip.c.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/JlCompress.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/qioapi.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quaadler32.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quachecksum32.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quacrc32.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quagzipfile.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quaziodevice.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quazip.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quazipdir.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quazipfile.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quazipfileinfo.cpp.o 3rdparty/quazip/quazip/CMakeFiles/QuaZip.dir/quazipnewinfo.cpp.o /opt/Qt/5.14.2/android/lib/libQt5Core_armeabi-v7a.so /opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/libz.a -latomic -lm && :
bionic/libc/include/bits/fortify/stdio.h:76: error: undefined reference to '__vsnprintf_chk'
bionic/libc/include/bits/fortify/unistd.h:159: error: undefined reference to '__read_chk'
bionic/libc/include/bits/fortify/unistd.h:174: error: undefined reference to '__write_chk'
bionic/libc/include/bits/fortify/unistd.h:174: error: undefined reference to '__write_chk'

from quazip.

stachenov avatar stachenov commented on August 18, 2024

This looks like a separate issue that doesn't belong here (although I may be wrong if it has something to do with CMake). And it looks like you're using some unusual standard C library implementation that you forgot to link to.

from quazip.

jlgranda avatar jlgranda commented on August 18, 2024

Thank you @stachenov, I solve this problem, finally I can build quazip into QField.

from quazip.

zgyarmati avatar zgyarmati commented on August 18, 2024

I'm not sure about release/debug builds. As it is, the build type is selected once and if it's the debug one, d is appended to the resulting binary name (libquazip1-qt5d.so). Everything else is the same, which means it's problematic to install both
debug and release in parallel, which kind of makes that d pointless. And I don't like that name either because it looks like d is related to qt5 and not to quazip1. Maybe get rid of the debug postfix altogether? Or provide separate packages (or targets?) for release/debug builds?

Yes, it's indeed problematic between multiple platforms:
on Linux this is somewhat simple, as normally the binary packages will strip the debug info and ship in a separated package (for example libquazip5-1-dbgsym_0.9.1-1_amd64.deb in current Debian testing), and the debugger will take the debug info from there. So for Linux it's OK to just skip the -d suffix, and it will work for the developers using the binary package shipped by the distribution. If someone compiles quazip from the source tarball, probably knows what she is doing, and likely install it under /usr/local/..., so it will be under a separated path.

For Windows of course it's different, as there the convention is to suffix the debug DLL-s with d, but AFAIK there is no convention about having these suffixes in include folder names or anyplace else other than the binary name itself.

Therefore i would suggest to make the
set(CMAKE_DEBUG_POSTFIX d)
conditional and only enable on Windows. Of course it still doesn't solve the problem of co-installing debug and release builds on Windows, but this issue is far from being unique to quazip, and I don't think this can be sorted out here. I think that happens most of time that if both debug and release binaries are built and installed, the config which built later will simply overwrite the include headers of the former build, and the 2 dll-s (with and without the -d suffix) are in the same folder, and the linker will pick up the one it needs depending whether it's a debug or release build of the target application. So as long as the built dll-s are compiled from the same quazip source version, it works just fine.
(But i rarely work on Windows, so i might be wrong here, please feel free to comment)

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I tested it on both Windows and Linux, and it seems to work just fine. I sometimes use a separate debug version on Linux as well, so I think I'll leave that option for all platforms. As far as I understand, Linux packages build the release version with debug symbols and ship those separately (so they can be used together with the release library), so setting CMAKE_DEBUG_POSTFIX will have no effect there, and is intended mainly for manual builds specially for debugging. True, those typically go into /usr/local or even $HOME/lib, but having a different binary name for true-debug build is helpful for cases when, for example, $HOME/lib is included in LD_LIBRARY_PATH, and hence putting debug binary there with the same name as the release one will cause every app to use the debug version, whereas putting it there with the d suffix will allow to use the debug build only in the app that actually needs debugging.

I think it's about time to release 1.0. The only open question is whether to remove BUILD_SHARED_LIBS by default. Considering possible complication, I think it's a sensible choice. But that'll force everyone to set it manually if they want a DLL/SO (which most people do, I believe).

from quazip.

zgyarmati avatar zgyarmati commented on August 18, 2024

I tested it on both Windows and Linux, and it seems to work just fine. I sometimes use a separate debug version on Linux as well, so I think I'll leave that option for all platforms. As far as I understand, Linux packages build the release version with debug symbols and ship those separately (so they can be used together with the release library), so setting CMAKE_DEBUG_POSTFIX will have no effect there, and is intended mainly for manual builds specially for debugging.

Yes, thx for clarifying. The point I wanted to make is that most of the users under Linux don't care about debug/release binaries, just take the debug info from the dbgsym package. But yes indeed, the d suffix for real debug build will not mess this up.

True, those typically go into /usr/local or even $HOME/lib, but having a different binary name for true-debug build is helpful for cases when, for example, $HOME/lib is included in LD_LIBRARY_PATH, and hence putting debug binary there with the same name as the release one will cause every app to use the debug version, whereas putting it there with the d suffix will allow to use the debug build only in the app that actually needs debugging.

I agree.

I think it's about time to release 1.0.

I'm planning to test the new build config under Buildroot, which has proven to be an efficient testbed for finding cross-compiling issues. Can you please wait a few days with the release until i report back with some results (and potentially patches)?

The only open question is whether to remove BUILD_SHARED_LIBS by default. Considering possible complication, I think it's a sensible choice. But that'll force everyone to set it manually if they want a DLL/SO (which most people do, I believe).

I understand that it causes some complications, but as the lib is licensed under LGPL, I think it's better to encourage the users to use it as a DLL/SO rather than statically linking into their (potentially proprietary) application.

from quazip.

cen1 avatar cen1 commented on August 18, 2024

My vote for BUILD_SHARED_LIBS to be ON by default. As far as I understand, the mentioned problem is when you add quazip source as a cmake subdirectory and it is built together with the target application, which is a rare scenario as far as I am concerned. In most cases you build it as standalone lib and then link to the prebuilt binary.

from quazip.

stachenov avatar stachenov commented on August 18, 2024

I'm planning to test the new build config under Buildroot, which has proven to be an efficient testbed for finding cross-compiling issues. Can you please wait a few days with the release until i report back with some results (and potentially patches)?

No problem at all. It's not like I'm in a hurry.

I'm also in favor of building shared libs by default, but as I understand it is considered a good practice to make no distinction between using an installed library or a subdir, that's why there are aliases for targets. If shared libs break it... I don't know. Maybe it makes sense that the default option is for standalone builds, and if anyone is willing to build a whole tree of projects, some of which are 3rd party, then it's them who should think beforehand whether they want shared or static libraries in their tree. I'm leaving it on for now.

from quazip.

zgyarmati avatar zgyarmati commented on August 18, 2024

I did some testing on different GCC versions and platforms, all seems to be OK.

from quazip.

zgyarmati avatar zgyarmati commented on August 18, 2024

Looking at this further, i think it would be beneficial if the .pc file would also provide the include path cflags, as right now to include QuaZip the include directives should contain something like <QuaZip-Qt5-1.0/quazip/JlCompress.h>, but it would be better to have it as <quazip/JlCompress.h>. If agreed, I'll send a PR with this. And if we are already at it, i would also move the .pc file generation from the top dir into the quazip/CMakeLists.txt file, as the cmake config file generation logic and the template is also there. Any thoughts on that?

from quazip.

stachenov avatar stachenov commented on August 18, 2024

Both ideas make sense to me. One more thing: two include paths should be added. One to allow <quazip/JlCompress.h> and another one to allow <JlCompress.h>. CMake is currently configured like this, as it was agreed somewhere up this thread or one of the previous ones. The reason is that different 0.x builds (CMake/qmake) used different setups, so it's safest to assume that there is client code that uses both styles, and hence it makes sense to support both. Another rationale is that pkg-config should support the same usage as CMake. If we decide to drop <JlCompress.h> support, we should drop it in both pkg-config and CMake for the sake of consistency. But I don't think it's a good idea.

from quazip.

zgyarmati avatar zgyarmati commented on August 18, 2024

Thx for the feedback, I'll send a PR during the day

from quazip.

stachenov avatar stachenov commented on August 18, 2024

1.0 is finally out! Thanks to everyone for your support with CMake stuff. It does have a weird learning curve, but in the end we have clean and consistent build process.

from quazip.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.