Full support for CMake 3.11+ FetchContent()

[Lucide]

I've landed here because I was unhappy with zlib's ancient CMake.
I see that here you're targeting a much newer version, but it looks like the library is still expecting to be imported with ExternalProject() only.
In the last few years, the FetchContent() approach is caught on, thanks to it's major usage improvements. But, being it basically a "glorified add_subdirectory()", unfortunately it requires all targets to be properly prefixed (zlib_zlib, zlib_example, ...), and the original targets' names must disappear. (the example target, for example, is very unfortunately named, very generic).
The same may apply for install components, if relevant

I don't know this project's backward compatibility requirements (or if there are any already, since it's still in beta), could this be a possible feature?

[mtl1979]

I don't personally think it is viable option to add prefixes to all zlib-ng's target names...

[Lucide]

Ehh fair enough, the pains of global target scoping.

(I've written "full support" because, in the current state, FetchContent() should be fine for new, prefixed, projects, as long as other, older-styled non-prefixed dependencies, don't collide)

[mtl1979]

I just use add_subdirectory() with EXCLUDE_FROM_ALL.

[Lucide]

The EXCLUDE_FROM_ALL argument says that targets defined in the subdirectory will not be included in the ALL target of the parent directory by default. Doesn't change CMake's target scoping rule.

From Craig Scott's "Professional CMake":

CMake target names must be unique across the whole set of projects being combined, so if two external projects define a target with the same name, they cannot both be added via add_subdirectory(). If projects are following a naming convention that uses a project specific prefix or something similar, then this limitation is fairly easy to avoid. The difficulties tend to come from projects which never expected to be used in this way and which use fairly generic names that are likely to be commonplace.

[mtl1979]

@Lucide I have used more than one project that depend on for example gtest on same build tree with no clashes... More recent cmake versions (for example on Ubuntu 20.04.1) will crash hard during configuration phase, if there is duplicate target names...

[Lucide]

I recall it was also a generator issue. Make could support (support?) that, but every other generator didn't.
see here: https://cmake.org/cmake/help/v3.17/prop_gbl/ALLOW_DUPLICATE_CUSTOM_TARGETS.html

Unfortunately, Linux distribution's approach to CMake is often very, very wrong. CMake is not a compiler, it's designed to be updated regularly, like how you allow your IDE to update. cmake_minimum_required() is not there just for show.

[mtl1979]

Visual Studio generators have been broken since early days... cmake developers tried to fix the generator for Visual Studio 2019, but no sane person is using VS2019 to compile production code as it is buggy as hell... There has been a lot of discussion between zlib-ng developers and Microsoft about getting bugs in Visual Studio compiler(s) fixed for all supported architectures...

[Lucide]

Interesting. didn't know about that!

But I think discussing about the legitimacy of CMake logic rules/generators it's beyond the original intent of this issue.

[mtl1979]

As zlib-ng plans to support Visual Studio, it limits what we can do... For some feature that is still quite volatile, it doesn't make sense to change the current system until the feature is stable enough... Otherwise we keep changing same thing over and over as the feature evolves...

We have already increased the minimum cmake version we require from 2.8 to 3.5.1, which is quite a leap...

[Lucide]

Of course! Perhaps I didn't express it very well, but I'm very thankful of the work you're doing here, and I'm more than satisfied of what you're already offering. When the first docs arrive, I'll jump right in.

I was proposing this just for the sake of trying, you might see a lot of similar github issues spawning up here and there. CMake is extremely hard to do right for newcomers like me, and everyone aspiring to learn is redirect to the same book, so some feature gets promoted on large scale.

The current situation doesn't even block FetchContent() completely, just on some potentially bigger, more crowded, projects.

As far as I'm concerned this issue can be closed, if it's already too late to make the switch now, this will become less relevant the more time passes.

[mtl1979]

We're pretty close to first public release, so I think now is not the best time to make this kind of changes in the build process... I'm not sure if later on we will or should revisit the issue...

[nmoinvaz]

If we were to change the zlib-ng's CMake generated project names they would no longer have the same project names as what is generated by zlib's CMake. Not sure if that would be something we would want.

[mtl1979]

I don't think having different project names than zlib is a real issue... I'm more worried about fixing something in zlib-ng that should more likely be fixed elsewhere...

[mtl1979]

Most of the "features" FetchContent() adds can already be accomplished by using git submodules... This also makes sure the download fails at earliest stage as possible and can be retried...

[Lucide]

If elsewhere is on CMake's side, in the terms of a new, third dependency management system, I really doubt anything new is going to appear.
I've seen no suggestion whatsoever about a new, revamped, approach to projects. Perhaps, in the future, prefixed target will be made pseudo-mandatory, or something like that.

That said, the FetchContent()/pure add_subdirectory() approach is still pretty much utopic beyond repair. Just one couple of non prefixed dependencies is almost guaranteed to collide and exclude the entire approach.

Even if you decided to support this, I don't know how much of an improvement this would be. For small, bleeding edge new projects, sure, but for the rest...

(The issue about FetchContent() is the same issue of addSubdirectory(), the downloading part poses no compatibility issues, it can be done with any tool, CMake or Git)

[mtl1979]

In the worst case, the best choice is adding all dependencies separately and use find_package() to get the correct include and library paths/directories... This is kind of preferred way on Unix-like systems where cmake itself has most common scripts for find_package() to find libraries...

[nmoinvaz]

Well if the project names are different between zlib and zlib-ng in CMake it means that it is no longer a drop in replacement for zlib at least for my minizip project. There are other ways to manually specify the .lib but it is less than ideal.

[Lucide]

n the worst case, the best choice is adding all dependencies separately and use find_package() to get the correct include and library paths/directories...

Yes, find_package() is usually used for installed dependencies, which location would likely be outside of the project root.
In zlib's case this can be done, since it's a common package to have preinstalled. But if you also plan to support windows, this becomes not true anymore.

The portable way is to use the ExternalProject() approach. In this case, CMake builds and installs locally the project, like an user would, without knowing anything about what it's actually being done or even what build system the project is using. This kind of freedom is a lot harder to handle, and often requires specialized project structures called superbuilds. This is probably what zlib's CMake implementation aims for.

I've seen that they use project-relative variables instead of global ones, so perhaps the project could be imported as a subdirectory. But everything is so old styled that I wouldn't know what to do with that.

[mtl1979]

find_package() also works on Windows, but it requires Administrator rights to install new packages as Windows doesn't normally allow writing to/under "Program Files" or "Program Files (x86)" directories...

There is junctions (compare. symlinks on Linux) for different SDK versions and compiler versions. Previously there was no centralized place to install development libraries and headers, but that was changed quite a while ago...

For non-development libraries, the default location is still application install directory or under Side-by-Side (SxS) directory...

Mixing libraries between compiler versions on Windows doesn't work as the toolchain version depends on specific build of C++ runtime library. You can compile for older version of toolchain, but not newer...

[nmoinvaz]

Even if you were to do add_subdirectory between two CMake projects that have different libraries you still have to use target_link_libraries to specify the names of the libraries, and if those library project names change between zlib CMake and zlib-ng CMake, it no longer becomes drop-in replacement.

[mtl1979]

@nmoinvaz There is two separate things...

1. being source-wise drop-in replacement
2. being binary-wise drop-in replacement

Like I said earlier, it is easy to use find_package() to find specific library by binary filename and header filename...
We could easily change project/target names for non-compat zlib-ng targets and leave the old ones for compatibility library... This would also allow us to build both versions at the same time...

[nmoinvaz]

I am talking about the project being a CMake drop-in replacement for use with FetchContent or ExternalProject.

[mtl1979]

@nmoinvaz For that to work, the default values for options would need to be compatible with stock zlib... As such minimum is that compatibility mode is enabled by default, which might not be what we want in the long term...

[Lucide]

@mtl1979 ahh, now I understand, you're talking about find_package()'s module mode. Yes, that's a fully CMake-agnostic approach, but should be reserved for non-native CMake projects.
Modern CMake projects export targets and are found with find_package() config mode.
See here https://cmake.org/cmake/help/v3.17/command/find_package.html

Module mode should be excluded in principle. Better to not rely on that to provide binary compatibility.

[mtl1979]

@Lucide I've seen some projects exporting targets and blowing up in developer's face... Generally having target exported doesn't mean something actually works as same target can be exported by multiple projects.

[nmoinvaz]

Right now in minizip I can use FetchContent between zlib CMake and zlib-ng CMake and only add ZLIB_COMPAT=ON. Changing project names will break this unless we keep it the same when ZLIB_COMPAT is enabled.

[mtl1979]

@nmoinvaz My point was that if adding ZLIB_COMPAT=ON is already required, we can safely require more definitions... But if none is required currently, it blows in our face, if we start requiring...

[nmoinvaz]

Yes of course.