Bug 1424609 - minizip header uses undefined types
Summary: minizip header uses undefined types
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: zlib
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Pavel Raiskup
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1556438
TreeView+ depends on / blocked
 
Reported: 2017-02-17 19:15 UTC by Zbigniew Jędrzejewski-Szmek
Modified: 2018-03-30 12:58 UTC (History)
6 users (show)

Fixed In Version: zlib-1.2.11-7.fc28
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-03-30 12:58:01 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 985344 None None None Never

Internal Links: 985344

Description Zbigniew Jędrzejewski-Szmek 2017-02-17 19:15:58 UTC
Description of problem:
$ gcc -x c -include /usr/include/minizip/crypt.h -c - </dev/null
In file included from <command-line>:32:0:
/usr/include/minizip/crypt.h:35:53: error: unknown type name ‘z_crc_t’
 static int decrypt_byte(unsigned long* pkeys, const z_crc_t* pcrc_32_tab)
                                                     ^~~~~~~
/usr/include/minizip/crypt.h:48:51: error: unknown type name ‘z_crc_t’
 static int update_keys(unsigned long* pkeys,const z_crc_t* pcrc_32_tab,int c)
                                                   ^~~~~~~
/usr/include/minizip/crypt.h:65:69: error: unknown type name ‘z_crc_t’
 static void init_keys(const char* passwd,unsigned long* pkeys,const z_crc_t* pcrc_32_tab)
                                                                     ^~~~~~~

This break compilation of software which #includes minizip/crypt.h.

Those types are defined in zconf.h, which used to be included in crypt.h. I don't know why minizip dropped that include.

Version-Release number of selected component (if applicable):
minizip-1.2.11-2.fc26.x86_64

Comment 1 Pavel Raiskup 2017-02-19 13:55:46 UTC
Thanks for the report, this is partly my fault:
https://bugzilla.redhat.com/show_bug.cgi?id=985344
(reverted downstream-only, never proposed patch, I don't know why)

I'm curious how are you using the header, which is obviously not (yet?)
ready to be used "as-is".  Can you please specify your use-case?

Comment 2 Pavel Raiskup 2017-02-19 14:06:31 UTC
Note that the crypt.h defines only static methods which are to be included
in zip.c/unzip.c.  So you could be OK to include only zip.h/unzip.h and
link against libminizip.

Comment 3 Zbigniew Jędrzejewski-Szmek 2017-02-19 14:23:05 UTC
I noticed because libsbml FTBFS. It turns out that the header was included by mistake: there's a bunch of unrelated crypt.h files, and (although it was supposed to be included in one place) it was also included inadvertently in another place because -I/usr/include/minizip was added to CFLAGS. So in the end, my fix for libsbml was to change #include <unzip.h> to #include <minzip/unzip.h> and remove the -I flag.

That said, it's a general rule that headers should pull in all of their own dependencies, so that including them anywhere (and in any order), just works. Most projects I know try to that for all of their .h files, and certainly for any that are installed into publicly visible locations.

Comment 4 Pavel Raiskup 2017-02-19 15:25:37 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> That said, it's a general rule that headers should pull in all of their own
> dependencies, so that including them anywhere (and in any order), just
> works. Most projects I know try to that for all of their .h files, and
> certainly for any that are installed into publicly visible locations.

Agreed with this "rule of thumb", it is good idea to have test-suite for
this rule, for sure.  But we are not upstream and it makes no sense
to have maintain downstream patches for this (so it was dropped).

What I'm rather thinking about is to not install crypt.h at all, as that
seems to be bug anyways (internal headers should be noinst_*).

Comment 5 Pavel Raiskup 2017-02-19 15:51:14 UTC
https://github.com/madler/zlib/pull/229

Comment 6 Dan Horák 2018-03-15 10:04:11 UTC
Looks like we get a breakage in sigil due that, the python header most likely wants /usr/include/crypt.h, but due the build-system the minizip's crypt.h is preferred

...
[ 34%] Building CXX object src/CMakeFiles/sigil.dir/main.cpp.o
cd /builddir/build/BUILD/Sigil-0.9.9/build/src && /usr/bin/c++  -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_PRINTSUPPORT_LIB -DQT_SVG_LIB -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -DQT_WEBKITWIDGETS_LIB -DQT_WEBKIT_LIB -DQT_WIDGETS_LIB -DQT_XMLPATTERNS_LIB -DQT_XML_LIB -I/builddir/build/BUILD/Sigil-0.9.9/build/src/sigil_autogen/include -I/builddir/build/BUILD/Sigil-0.9.9/build/src -I/builddir/build/BUILD/Sigil-0.9.9/src -I/builddir/build/BUILD/Sigil-0.9.9/internal/gumbo -I/usr/include/python3.6m -I/usr/include/minizip -I/usr/include/hunspell -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtWidgets -isystem /usr/include/qt5/QtGui -isystem /usr/include/qt5/QtCore -isystem /usr/lib64/qt5/./mkspecs/linux-g++ -isystem /usr/include/qt5/QtXml -isystem /usr/include/qt5/QtXmlPatterns -isystem /usr/include/qt5/QtNetwork -isystem /usr/include/qt5/QtPrintSupport -isystem /usr/include/qt5/QtSvg -isystem /usr/lib64/qt5/../../include/qt5 -isystem /usr/lib64/qt5/../../include/qt5/QtWebKit -isystem /usr/lib64/qt5/../../include/qt5/QtWebKitWidgets -isystem /usr/include/qt5/QtConcurrent  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -mcet -fcf-protection -std=c++11   -Wall -fPIC -std=gnu++11 -o CMakeFiles/sigil.dir/main.cpp.o -c /builddir/build/BUILD/Sigil-0.9.9/src/main.cpp
In file included from /usr/include/python3.6m/Python.h:39,
                 from /builddir/build/BUILD/Sigil-0.9.9/src/Misc/EmbeddedPython.h:26,
                 from /builddir/build/BUILD/Sigil-0.9.9/src/main.cpp:22:
/usr/include/minizip/crypt.h:35:53: error: 'z_crc_t' does not name a type
 static int decrypt_byte(unsigned long* pkeys, const z_crc_t* pcrc_32_tab)
                                                     ^~~~~~~
/usr/include/minizip/crypt.h:48:51: error: 'z_crc_t' does not name a type
 static int update_keys(unsigned long* pkeys,const z_crc_t* pcrc_32_tab,int c)
                                                   ^~~~~~~
/usr/include/minizip/crypt.h:65:69: error: 'z_crc_t' does not name a type
 static void init_keys(const char* passwd,unsigned long* pkeys,const z_crc_t* pcrc_32_tab)
                                                                     ^~~~~~~

Comment 7 Fedora Update System 2018-03-15 15:20:28 UTC
zlib-1.2.11-7.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-c12b8339cd

Comment 9 Fedora Update System 2018-03-16 14:45:00 UTC
zlib-1.2.11-7.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-c12b8339cd

Comment 10 Fedora Update System 2018-03-30 12:58:01 UTC
zlib-1.2.11-7.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.