Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[new package] zlib-ng 2.1.6 #20844

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ognevny
Copy link
Collaborator

@ognevny ognevny commented May 9, 2024

fixes #20815

@ognevny ognevny added the new-package Pull request which adds new package(s) label May 9, 2024
@ognevny ognevny mentioned this pull request May 9, 2024
6 tasks
@Biswa96
Copy link
Member

Biswa96 commented May 9, 2024

The DLL files are installed twice in /bin and /lib.

@ognevny
Copy link
Collaborator Author

ognevny commented May 9, 2024

The DLL files are installed twice in /bin and /lib.

I see, I'll try to find where it's set

@ognevny
Copy link
Collaborator Author

ognevny commented May 9, 2024

also there is extra lib prefix for all library files

@esator
Copy link

esator commented May 10, 2024

-DWITH_UNALIGNED=OFF is really that needed, is there some issues without this flag?
And as already mentioned, it would be better to have a complete zlib mimic for zlib-ng-compat, without the extra lib prefix, for easier drop-in replacement, because that's what it's intended for

@ognevny
Copy link
Collaborator Author

ognevny commented May 10, 2024

-DWITH_UNALIGNED=OFF is really that needed, is there some issues without this flag?
And as already mentioned, it would be better to have a complete zlib mimic for zlib-ng-compat, without the extra lib prefix, for easier drop-in replacement, because that's what it's intended for

arch disabled it https://gitlab.archlinux.org/archlinux/packaging/packages/zlib-ng/-/blob/main/PKGBUILD?ref_type=heads#L27

@esator
Copy link

esator commented May 10, 2024

And probably zlib-ng-compat also needs provides (replaces?) for zlib, not just conflicts

@ognevny
Copy link
Collaborator Author

ognevny commented May 10, 2024

And probably zlib-ng-compat also needs provides (replaces?) for zlib, not just conflicts

no replaces. I would add provides if there wasn't an issue with lib prefix (it shouldn't be there)

@ognevny
Copy link
Collaborator Author

ognevny commented May 10, 2024

issue with lib prefix (it shouldn't be there)

fixed. now a soname is the same as for zlib package

@esator
Copy link

esator commented May 10, 2024

LICENSE.md for zlib-ng and zlib-ng-compat have the same path and name, probably would be better to use package names
And there are still prefixes for libzlib.dll.a and libzlib-ng.dll.a, for example ZLIB-release.cmake from the official zlib-ng-win-x86-64-compat.zip

#----------------------------------------------------------------
# Generated CMake target import file for configuration "Release".
#----------------------------------------------------------------

# Commands may need to know the format version.
set(CMAKE_IMPORT_FILE_VERSION 1)

# Import target "ZLIB::ZLIB" for configuration "Release"
set_property(TARGET ZLIB::ZLIB APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(ZLIB::ZLIB PROPERTIES
  IMPORTED_IMPLIB_RELEASE "${_IMPORT_PREFIX}/lib/zlib.lib"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/zlib1.dll"
  )

list(APPEND _cmake_import_check_targets ZLIB::ZLIB )
list(APPEND _cmake_import_check_files_for_ZLIB::ZLIB "${_IMPORT_PREFIX}/lib/zlib.lib" "${_IMPORT_PREFIX}/lib/zlib1.dll" )

# Import target "ZLIB::zlibstatic" for configuration "Release"
set_property(TARGET ZLIB::zlibstatic APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(ZLIB::zlibstatic PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_RELEASE "C"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/zlibstatic.lib"
  )

list(APPEND _cmake_import_check_targets ZLIB::zlibstatic )
list(APPEND _cmake_import_check_files_for_ZLIB::zlibstatic "${_IMPORT_PREFIX}/lib/zlibstatic.lib" )

# Commands beyond this point should not need to know the version.
set(CMAKE_IMPORT_FILE_VERSION)

And lib:

/cmake
/pkgconfig
zlib.lib
zlib1.dll
zlibstatic.lib

mingw-w64-clang-x86_64-zlib-ng-compat-2.1.6-1-any.pkg.tar.zst for comparison:

/cmake
/pkgconfig
libzlib.dll.a
zlib.a

Fedora:

%files compat
%{_libdir}/%{compat_soname}
%{_libdir}/libz.so.%{zlib_ver}.zlib-ng

%files compat-devel
%{_includedir}/zconf.h
%{_includedir}/zlib.h
%{_includedir}/zlib_name_mangling.h
%{_libdir}/libz.so
%{_libdir}/pkgconfig/zlib.pc
%{_libdir}/cmake/ZLIB/*
 
%files compat-static
%{_libdir}/libz.a

mingw-w64-zlib:

/pkgconfig
libz.a
libz.dll.a

@esator
Copy link

esator commented May 11, 2024

@ognevny
Copy link
Collaborator Author

ognevny commented May 11, 2024

@ognevny
Copy link
Collaborator Author

ognevny commented May 11, 2024

the lib prefix for import lib doesn't make sense. it should be linkable with -lz anyway

@esator
Copy link

esator commented May 12, 2024

the lib prefix for import lib doesn't make sense. it should be linkable with -lz anyway

At least to make consistent naming
But, ideally for zlib-ng-compat it would be better to mimic zlib names (libz.a, libz.dll.a) to avoid potentially breaking some very specific scripts and such

it's not merged yet

But, there is an explanation why it's not the case for zlib-ng and just copying that flag from other packages unless it's really necessary is not the best solution because it may make some optimizations worse, and better optimization/speed is the main reason to use zlib-ng
Besides, other very widely used and well tested apps and distros dont force this flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-package Pull request which adds new package(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add zlib-ng
3 participants