Support minizip-ng build #574
Conversation
CMakeLists.txt
Outdated
| @@ -48,7 +48,7 @@ find_package(ZLIB REQUIRED) | |||
| find_package(MiniZip 1 QUIET) # version range (0...<2.0.0) requires CMake>=3.19 | |||
| if(UNIX AND NOT APPLE) | |||
| find_package(PkgConfig) | |||
| pkg_check_modules(MINIZIP minizip IMPORTED_TARGET minizip<2.0.0) | |||
| pkg_check_modules(MINIZIP minizip IMPORTED_TARGET minizip<5.0.0) | |||
There was a problem hiding this comment.
Why 5? do we need version restriction?
There was a problem hiding this comment.
It was there before, and I wasn't sure if it served some purpose, so I left it there
There was a problem hiding this comment.
I think it was there to avoid pickup minizip-ng. My question is why version 5 or can we get rid of this condition?
Or are all minizip-ng versions compatible?
There was a problem hiding this comment.
From what I could test, version 3 is compatible after these two changes. I can test version 4 as well, but I cannot guarantee the upcoming ones.
There was a problem hiding this comment.
@metsma I've tested it on version 4 and it's building, so I would leave it this way. Can you merge this?
There was a problem hiding this comment.
There was a problem hiding this comment.
Why not remove then the <5 condition? If new release is with major version number 5 then we need change code again
CMakeLists.txt
Outdated
| @@ -48,7 +48,7 @@ find_package(ZLIB REQUIRED) | |||
| find_package(MiniZip 1 QUIET) # version range (0...<2.0.0) requires CMake>=3.19 | |||
| if(UNIX AND NOT APPLE) | |||
| find_package(PkgConfig) | |||
| pkg_check_modules(MINIZIP minizip IMPORTED_TARGET minizip<2.0.0) | |||
| pkg_check_modules(MINIZIP minizip IMPORTED_TARGET minizip<5.0.0) | |||
There was a problem hiding this comment.
Why not remove then the <5 condition? If new release is with major version number 5 then we need change code again
|
Changed to not check the version at all |
|
Hi, can this MR be merged? |
|
We have to test it before we can merge it. |
|
Can you please rebase to latest master to fix github actions and provide fedora builds with minizip-ng (needs probably installing minizip-ng-devel package?) |
Up until now, the minizip was including this header file, but minizip-ng does not and it causing the build to fail. If the file uses structures defined in the zlib.h it should explicitly include it.
minizip-ng versions are 3 and 4, so support it in CMakeList as well
|
Rebased. It requires both |
|
I was reffering to this project github action builds https://github.com/open-eid/libdigidocpp/blob/master/.github/workflows/build.yml#L87 |
|
I've just added a new commit fixing the deps in Fedora. I used the old names because the new names work only since Fedora 40. However, the old names are working in Fedora 40 so don't worry |
Ideally we would like to include 'rawhide' in the loop as well, which would allow spotting issues like open-eid#574 early on, but official docker image only supports stable/rawhide which at current point of time correspond to F40 and F42 version, thus we still miss 41.
Fedora 41 will be the latest stable version to be released in upcoming weeks and thus will render 39 as EOL. Also bring in 'rawhide' so we can detect issues like open-eid#574/open-eid#600 earlier.
Fedora 41 will be the latest stable version to be released in upcoming weeks and thus will render 39 as EOL. Also bring in 'rawhide' so we can detect issues like open-eid#574/open-eid#600 earlier. Signed-off-by: Priit Laes <plaes@plaes.org>
Fedora 39 drops upstream support from 12 Nov 2024. Also bring in 'rawhide' so we can detect issues like open-eid#574/open-eid#600 earlier. Signed-off-by: Priit Laes <plaes@plaes.org>
* Add include to zlib.h as it uses the structures defined in the zlib.h Up until now, the minizip was including this header file, but minizip-ng does not and it causing the build to fail. If the file uses structures defined in the zlib.h it should explicitly include it. * Support minizip version 3 and 4 as well minizip-ng versions are 3 and 4, so support it in CMakeList as well * Fix install deps in Fedora Github action
Fedora has switched to
minizip-ng(with the compat mode) and these patches fixed the failed build oflibdigidocpp.You can see more here: https://src.fedoraproject.org/rpms/libdigidocpp/pull-request/4
Signed-off-by: Lukas Javorsky ljavorsk@redhat.com