Various CMake modernizations by LecrisUT · Pull Request #40 · Lecrapouille/zipper

LecrisUT · 2025-07-18T22:08:43Z

A lot of points being addressed here:

Fixup the FetchContent support in line with the GTest usage (except additional USE_MODULE support)
Reorganize the project to follow some modern CMake practices
Had to bump minimum CMake to 3.13 in order to move the FetchContent parts to its own subdirectory
Scope-out the third-party variables (variables defined there don't propagate outwards, but the variables here propagate inwards)
Try to use consistent targets to support both find_package and FetchContent workflows

Closes #34

Simplified cleaning rules, fix installed folders for data and doc, allow creating RPM

* Add support for using packaged GTest Support is available only in CMake 3.24 and higer * Move enable_testing This needs to be present before any add_tests, otherwise the tests are not recorded properly

Useful for prototyping by pointing a dependent's `zipper_ROOT` to a build directory

LecrisUT · 2025-07-18T22:11:08Z

src/Unzipper.cpp

+#include <minizip/ioapi_mem.h>
+#include <minizip/minishared.h>
+#include <minizip/unzip.h>
+#include <minizip/zip.h>


Well, potatoes. Can you fix the Makefile part to account for this change? This change would be needed to support using system packages.

This is not an issue for me with Makefile: it's just fixing a -I piece of cake. I find sad that the hard part is always coming from CMake. On the original project 99% of issues come from CMake that is why I used my Makefile system. I'm not even able to compile with your changes. The nightmare is coming back again ;(

-- The following OPTIONAL packages have not been found:

minizip

zlib-ng

-- Configuring done (10.5s)
CMake Error: install(EXPORT "zipperTargets" ...) includes target "zipper" which requires target "zlib" that is not in any export set.
CMake Error in CMakeLists.txt:
export called with target "zipper" which requires target "minizip" that is
not in any export set.

CMake Error in CMakeLists.txt:
export called with target "zipper" which requires target "zlib" that is not
in any export set.

-- Generating done (0.0s)
CMake Generate step failed. Build files cannot be regenerated correctly.

There are not optional packages. They either comes from initially with make download-external-libs then I added git submodules for Red-Hat

-- Configuring done (10.5s)
CMake Error: install(EXPORT "zipperTargets" ...) includes target "zipper" which requires target "zlib" that is not in any export set.

This is a complex issue about static libraries requiring to have all dependencies installed. Iirc, that is an existing issue.

I can look into that afterwards, but for now, try building with -DZIPPER_SHARED_LIBS=ON

@LecrisUT why not letting people doing git clone ... --recurse then either our CMakeLists includes directly the CMake inside the external/minizip/CMakeLists.txt our we compile the C++ files directly ? I can modify headers for you now

@LecrisUT I have commited the includes. I'm not sure to switch "" to <> because, if I remember well the compiler will search for OS files first

I'm not sure to switch "" to <> because, if I remember well the compiler will search for OS files first

Yes, but that is desired. You are signalling that this is a third-party library. The -I flags will make sure that the system dependency is picked up.

@LecrisUT why not letting people doing git clone ... --recurse then either our CMakeLists includes directly the CMake inside the external/minizip/CMakeLists.txt our we compile the C++ files directly ? I can modify headers for you now

The reason for this change is to make way to allow building against a system minizip which exports it as #include <minizip/*>. This will make it easier to pull in minizip-ng-compat-devel make it compatible with that, and move the bundled minizip to the upstream version.

LecrisUT · 2025-07-18T22:15:37Z

ZIPPER_BUILD_DEMOS should go away. Instead use a pattern like the one I am introducing here.

Lecrapouille · 2025-07-26T07:49:02Z

@LecrisUT

I had to git push force the master branch to fix zipper versions in cmake to match my tags.
I moved your commits into the dev-modern-cmake branch

LecrisUT · 2025-07-26T08:09:06Z

I had to git push force the master branch to fix zipper versions in cmake to match my tags.

Please refrain from doing that if this ends up in downstream packaging. Changing the checksum of a tag will break almost all downstream packages. Instead you can remove a release and get a newer tag that fixes the issue.

I am confused by a lot of the commits being made. To be blunt I suspect that they were done through vibe-coding. If you have people willing to help you with the project and willing to share their expertise, please allow them provide feedback before accepting the AI input at face value.

Lecrapouille · 2025-07-26T08:59:23Z

@LecrisUT yes for tags, but it's ok since people do not use this repo and mismatching versions with tags is more confusing.

I am confused by a lot of the commits being made.

Concerning which parts ? I used IA for CMake because I do not know how the syntax. I used once for fixing big slow down issue and for refactoring unit tests when I modified the API.

I just made a branch of your commits temporarily to be merged into the main branch later when I'll have time to investigate on CMake

Lecrapouille and others added 26 commits June 17, 2025 02:05

Update to version 3.0.0

413d38e

Authors

aaf7ac3

Require C++14 to fix build errors (tested on Ubuntu 24 + GCC 13 x86) (L…

4e7f20f

…ecrapouille#29)

Update README.md: Replace C++11 to C++14

087c681

CI: add Ubuntu + CMake

d779ccd

Disable temporary a test for Windows Lecrapouille#30

5f482e3

Lecrapouille#30

Include zipper from CMake Lecrapouille#31

237c8f2

Fix English

7afe417

Update CI: CMakeHelloWorld Lecrapouille#31

7fa84fa

Update to version 3.0.1

8d6da92

Update Makefile Lecrapouille#33

6c423cc

Simplified cleaning rules, fix installed folders for data and doc, allow creating RPM

Update licenses Lecrapouille#38

07b4903

Update minizip

3938d0a

Add support for using packaged GTest (Lecrapouille#37)

f859409

* Add support for using packaged GTest Support is available only in CMake 3.24 and higer * Move enable_testing This needs to be present before any add_tests, otherwise the tests are not recorded properly

Update minizip Lecrapouille#35

f5f85e1

CMake: fetch minizip and zlib Lecrapouille#34

157bb70

Missing update version in CMake Lecrapouille#36

0f0bce3

Set tags to third parties

e1f9805

Misc changes Makefile

34323b0

Export build-tree targets

11b0a2d

Useful for prototyping by pointing a dependent's `zipper_ROOT` to a build directory

Simplify the Config.cmake export

2821036

Move and rewrite third-party dependencies

17ebaec

Use namepsaced options

1bffa4f

Use a more modern design for the project

e3c0289

Need to bump minimum CMake for CMP0079

247b888

Fix minizip include path

05a443c

LecrisUT commented Jul 18, 2025

View reviewed changes

Lecrapouille force-pushed the master branch 2 times, most recently from 4de84a5 to 0351dc7 Compare July 26, 2025 07:33

Merge branch 'master' into cmake/modern

db578ad

Lecrapouille merged commit 7c5d3f4 into Lecrapouille:master Jul 26, 2025
0 of 4 checks passed

Lecrapouille pushed a commit that referenced this pull request Jul 26, 2025

Various CMake modernizations (#40)

56f56bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various CMake modernizations#40

Various CMake modernizations#40
Lecrapouille merged 27 commits intoLecrapouille:masterfrom
LecrisUT:cmake/modern

LecrisUT commented Jul 18, 2025 •

edited

Loading

Uh oh!

LecrisUT Jul 18, 2025

Uh oh!

Lecrapouille Jul 20, 2025

Uh oh!

Lecrapouille Jul 20, 2025

Uh oh!

Lecrapouille Jul 20, 2025

Uh oh!

LecrisUT Jul 20, 2025

Uh oh!

Lecrapouille Jul 20, 2025

Uh oh!

Lecrapouille Jul 20, 2025

Uh oh!

LecrisUT Jul 20, 2025

Uh oh!

LecrisUT commented Jul 18, 2025

Uh oh!

Uh oh!

Lecrapouille commented Jul 26, 2025

Uh oh!

LecrisUT commented Jul 26, 2025

Uh oh!

Lecrapouille commented Jul 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

LecrisUT commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LecrisUT commented Jul 18, 2025

Uh oh!

Uh oh!

Lecrapouille commented Jul 26, 2025

Uh oh!

LecrisUT commented Jul 26, 2025

Uh oh!

Lecrapouille commented Jul 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LecrisUT commented Jul 18, 2025 •

edited

Loading