[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226671 Jason Tibbitts ti...@math.uh.edu changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #35 from Jason Tibbitts ti...@math.uh.edu 2009-01-13 16:12:52 EDT --- With these things in mind, I believe that the autoconfiscation incures less maintanance costs than the complicated spec file would. That seems quite reasonable. I'd suggest just noting that in the specfile with something like: # The following two patches add an autotools build system to work around # problems in the regular zlib build system. just to make things clear, but it's not seriously important. I think that takes care of this review. Thanks for your time. APPROVED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226671 Jason Tibbitts ti...@math.uh.edu changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution||RAWHIDE -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226671 Ivana Varekova varek...@redhat.com changed: What|Removed |Added Flag|needinfo?(varek...@redhat.c | |om) | --- Comment #33 from Ivana Varekova varek...@redhat.com 2009-01-07 06:47:02 EDT --- Hello, Stepan Kasal will look at this issue. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Comment #34 from Stepan Kasal ska...@redhat.com 2009-01-07 07:58:20 EDT --- (In reply to comment #31) Really the only thing that bothers me is [...] the autool-ization of the original non-autotools-using source. First, about problems with the original build system of zlib: The same CFLAGS variable is used for static and dynamic library. So using this simple build system is not so simple as using the complex autotools system. You need to do something like: make mv libz.a save-libz.a make clean ./configure -s make mv save-libz.a libz.a To see another variant of this trick, see the spec file just before the autoconfiscation: http://cvs.fedora.redhat.com/viewvc/devel/zlib/zlib.spec?revision=zlib-1_2_3-6_fc7 Second, there is minizip-*-autotools.patch. contrib/minizip/Makefile does not contain any rules for building libminizip.so. Consequently, some hacking is needed to get the library built; using libtool (through Automake) is a sensible way. With these things in mind, I believe that the autoconfiscation incures less maintanance costs than the complicated spec file would. Does this sound fair? If yes, should an excerpt of this explanation go to the spec file? (Text suggestions welcome. ;-) BTW, I'm going to do some cleanup of the autotools patches. But I don't think it would be a good idea to go back to the build system shipped with zlib. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226671 Robert Scheck red...@linuxnetz.de changed: What|Removed |Added Flag||needinfo?(varek...@redhat.c ||om) --- Comment #32 from Robert Scheck red...@linuxnetz.de 2008-12-20 09:15:17 EDT --- Ivana, ping? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2008-03-03 00:57 EST --- I think this package is quite clean now. Really the only thing that bothers me is the what was bothering Patrice earlier: the autool-ization of the original non-autotools-using source. According to cvs annotate, this was done in February, 2007 by Atam Tkac: * Tue Feb 20 2007 Adam Tkac atkac redhat com - 1.2.3-7 - building is now automatized - specfile cleanup However, there's nothing that explains why this is needed or what advantage there is to building it this way versus the way the package normally builds. I know this review is dragging on, but is it possible to at least get a couple of lines of comment in the specfile that explain why we need to do this, what bugs it solves, or what problems it avoids? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2008-02-13 03:33 EST --- Thanks, license tag is fixed in zlib-1.2.3-18.fc9. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added CC||[EMAIL PROTECTED] --- Additional Comments From [EMAIL PROTECTED] 2008-02-12 11:35 EST --- Ivana, Apologies if I told you that zlib should be under the BSD license. Zlib has its own license, you can use License: zlib for the spec file here. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2008-02-11 10:02 EST --- zlib.x86_64: W: file-not-utf8 /usr/share/doc/zlib-1.2.3/ChangeLog The ChangeLog file could use a trip through iconv. Thanks - fixed The License: tag says BSD, but I would find it odd if the zlib package isn't under the zlib license. I believe the License: tag should be zlib. I have discussed this issue with Tom Callaway and his recommendation was it is BSD license so. I note that there's a small test suite; generally it's a good thing to call test suites even if they don't do all that much because they serve as a useful sanity check. But I added a %check section and it seems that make check doesn't actually do anything for some reason. When I call it manually I get some output ending with *** zlib test OK ***. I'm not really sure what the problem is. check added. I don't thing there is any reason to add minizip executables to subpackage so I don't like to do it. If I overlook some good reason to do it, write it here please. Thanks a lot. The last version is zlib-1.2.3-17.fc9. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2008-01-08 04:01 EST --- (In reply to comment #25) Is anyone actually reviewing this? fedora-review is set to '?' but I dont' see anyone assigned to the package. I'm happy to review this if nobody else is doing so. I did some comments, but I disagree to much on the use of autotools without upstream consent. Especially when this is not really necessary and the tests aren't run anymore. I don't have any particular opinion on the minizip executables. I don't see why it would be mandatory to ship them if the maintainer doesn't want to, however. They're just source in the contrib directory of upstream's tarball, and it's not really common for that kind of thing to be shipped as part of our packages unless the maintainer feels some need to include it. I don't think it is mandatory, but I haven't seen a good reason not to ship them. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added CC||[EMAIL PROTECTED] --- Additional Comments From [EMAIL PROTECTED] 2007-12-21 21:03 EST --- Is anyone actually reviewing this? fedora-review is set to '?' but I dont' see anyone assigned to the package. I'm happy to review this if nobody else is doing so. I note a coupe of minor things: zlib.x86_64: W: file-not-utf8 /usr/share/doc/zlib-1.2.3/ChangeLog The ChangeLog file could use a trip through iconv. The License: tag says BSD, but I would find it odd if the zlib package isn't under the zlib license. I believe the License: tag should be zlib. I can attach a patch or just fix these in CVS if you like. There are also several undefined-non-weak-symbol warnings like: minizip.x86_64: W: undefined-non-weak-symbol /usr/lib64/libminizip.so.1.0.1 inflateEnd and also for these symbols: deflate inflateInit2_ inflate crc32 deflateEnd deflateInit2_ get_crc_table I think these are bad practice but OK because the minizip.pc file specifies that packages which use it always link with libz, which will provide the symbols. I don't have any particular opinion on the minizip executables. I don't see why it would be mandatory to ship them if the maintainer doesn't want to, however. They're just source in the contrib directory of upstream's tarball, and it's not really common for that kind of thing to be shipped as part of our packages unless the maintainer feels some need to include it. I note that there's a small test suite; generally it's a good thing to call test suites even if they don't do all that much because they serve as a useful sanity check. But I added a %check section and it seems that make check doesn't actually do anything for some reason. When I call it manually I get some output ending with *** zlib test OK ***. I'm not really sure what the problem is. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added Version|devel |rawhide [EMAIL PROTECTED] changed: What|Removed |Added OtherBugsDependingO||426387 nThis|| -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-23 05:46 EST --- Thanks Patrice, fixed (zlib-1.2.3-16.fc9). There is no need to have minizip and miniunzip executables in this package it should only be use as a library. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-23 06:28 EST --- (In reply to comment #21) Thanks Patrice, fixed (zlib-1.2.3-16.fc9). There is no need to have minizip and miniunzip executables in this package it should only be use as a library. They can be in a subpackage, but they should be shipped. If not in this package, which package should they be part of? In fact it seems to me that the most common setup, when you want to separate executables from libraries is to have a minizip-libs package with the libraries (which will be automatically installed, or as a dependency of minizip-devel), and a minizip package with the executables. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-23 08:12 EST --- I think they could not be shipped at all. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-23 13:14 EST --- Why? Are they buggy? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-18 05:20 EST --- The timestamps of the minizip headers are not kept. Using make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p' should do the trick. The minizip headers should be in minizip-devel. in the minizip.pc file, either includedir should be /usr/include/minizip or the Cflags should have minizip subdir. A dot is missing at the end of the minizip description. Why aren't the minizip and miniunzip executables shipped? minizip-devel should Requires pkgconfig. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-11-14 10:44 EST --- Hello, thanks for your comments. All problems you mentioned here are fixed in zlib-1.2.3-15.fc9. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-10-28 05:04 EST --- (In reply to comment #17) (In reply to comment #16) Comment #14 is a must fix. But in any case I have too much The devel spec contains a -static subpackage: http://cvs.fedoraproject.org/viewcvs/rpms/libpng/devel/libpng.spec?view=markup You seem to be mixing with the libpng review ;-) btw. %makeinstall should not be used, better is to use (when it works, which is afaik almost always): make DESTDIR=$RPM_BUILD_ROOT install See: http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: zlib
https://bugzilla.redhat.com/show_bug.cgi?id=226671
[EMAIL PROTECTED] changed:
What|Removed |Added
AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED]
--- Additional Comments From [EMAIL PROTECTED] 2007-10-27 16:55 EST ---
Comment #14 is a must fix. But in any case I have too much
disagreement to approve this package (the use of autotools
and not shipping the api). I'll keep on commenting on other
issues, though.
At the first %build line, there is a spurious
make %{?_smp_mflags}
I don't see other obvious issues.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-10-27 20:31 EST --- (In reply to comment #16) Comment #14 is a must fix. But in any case I have too much The devel spec contains a -static subpackage: http://cvs.fedoraproject.org/viewcvs/rpms/libpng/devel/libpng.spec?view=markup btw. %makeinstall should not be used, better is to use (when it works, which is afaik almost always): make DESTDIR=$RPM_BUILD_ROOT install See: http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-09-06 10:25 EST --- Hello Patrice, please could you look to zlib-1.2.3-14, it seems for me to be right now - there is used automake but I think every other parts are clear. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: zlib
https://bugzilla.redhat.com/show_bug.cgi?id=226671
[EMAIL PROTECTED] changed:
What|Removed |Added
CC||[EMAIL PROTECTED]
--- Additional Comments From [EMAIL PROTECTED] 2007-08-31 10:03 EST ---
This should go in a -static subpackage:
%{_libdir}/*.a
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added Priority|normal |medium --- Additional Comments From [EMAIL PROTECTED] 2007-05-21 06:41 EST --- Also I won't make shipping the api doc a must but I really can't understand why you don't want it, since the devel package is nearly unuseful without api documentation. It could be in a separated -doc subpackage, though. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added Severity|normal |medium [EMAIL PROTECTED] changed: What|Removed |Added Status|NEEDINFO|ASSIGNED Flag|needinfo? | --- Additional Comments From [EMAIL PROTECTED] 2007-04-06 05:46 EST --- Patrice, could you please look at the last version zlib-1.2.3-10.fc7 and approved this review request or if you see any reason why you don't want to aproved it here. But at first I want to reply to your comments: * adding autotools is the most clear solution and there is no problem with it * zutil.h is removed * timestamps are kept * the description is easy to get/find/grab it is not a part of zlib package and upstream don't want to add it so I think it is not necessary to add it too Thanks for your comments. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-04-06 07:47 EST --- Created an attachment (id=151869) -- (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=151869action=view) spec file that incorporates all my comments I removed the autotools patch in this spec. I think that such change is for upstream, not in a fedora package. Moreover this is not similar with upstream since the tests are removed. I cleaned the build such that the package is built in the build section and also used more rpm macros. and I added the manual, in my opinion this is a must - don't use the autotools, instead revert to the previous build procedure - ship the manual to have a description of the API - build the libraries in the %%build section rpmlint output is explained in comments in the spec E: zlib configure-without-libdir-spec W: zlib make-check-outside-check-section make check E: zlib configure-without-libdir-spec W: zlib make-check-outside-check-section make check Feel free to modify it again (for example if you like the build but not the manual). -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-04-06 07:55 EST --- Created an attachment (id=151870) -- (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=151870action=view) keep header and man page timestamps -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-04-06 09:43 EST --- I think that the spec I proposed in Comment #10 is right, but if you really want to use the autotools and think my proposal is not right, I could also accept an autotool based package -- although I think it is not right. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |NEEDINFO Flag||needinfo? -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-02-21 07:12 EST --- Fixed in zlib-1.2.3-7.fc7. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-02-21 18:30 EST --- * I don't like that much adding autotools support with a patch. Did upstream accept the patch? * zutil.h shouldn't be shipped * timestamp of .h and man files should be kept. * there is no description of the API. Please consider shipping the html pages as I suggest above (or any other description of the API). Suggestion: * in the libtool comment, replace bogus with unuseful, libtool is right in installing .la files. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-02-18 08:46 EST --- Next the build should be done in build and the install in install. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-02-17 04:43 EST --- I also suggest adding the dist tag. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: zlib
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671
[EMAIL PROTECTED] changed:
What|Removed |Added
CC||[EMAIL PROTECTED]
--- Additional Comments From [EMAIL PROTECTED] 2007-02-16 16:16 EST ---
* the source is not one of those appearing on the home page. Moreover
you could use the tar.bz2.
* Is Prefix: %{_prefix} needed?
* BuildRoot is not the preferred one
* The comment in %build is misleading. It should better be something like
# prepare Makefile for the static lib
and in %install there could be a comment saying
# the first make triggers compilation of the object files, linking of the
# shared library and installs the library
# The second make triggers the linking of the static library and
# its installation
* I think it would be better to have, in -devel
http://www.zlib.net/manual.html
and
http://www.zlib.net/zlib_how.html
* it seems to me that FAQ should be in %doc, and ChangeLog should be
in the main package
* -devel should
Requires: zlib = %{version}-%{release}
* It seems to me that there should be a make clean between the 2
make -f invocations, to trigger recompilation with the flags without -fPIC
* I'll attach a patch to simplify the build and install, and use more
macros.
* zutil.h seems to be an internal header that should no be shipped
* seems like that spec is not in utf8, certainly because of Glomsrød
* remove the dots at the end of the Summaries
Suggestion:
Change %defattr(-,root,root) to %defattr(-,root,root,-)
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226671] Merge Review: zlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: zlib https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226671 --- Additional Comments From [EMAIL PROTECTED] 2007-02-16 16:18 EST --- Created an attachment (id=148247) -- (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=148247action=view) simplify %build and %install, remove redundant Prefix -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
