Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
Summary: Merge Review: apr-util Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: jorton@redhat.com
Fedora Merge Review: apr-util
http://cvs.fedora.redhat.com/viewcvs/devel/apr-util/
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium
jeremy@hinegardner.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |jeremy@hinegardner.org Flag| |fedora-review?
------- Additional Comments From jeremy@hinegardner.org 2007-03-29 22:51 EST ------- I'll be happy to review this package, please look for full review shortly.
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From jeremy@hinegardner.org 2007-03-30 01:00 EST ------- OK - Package meets naming and packaging guidelines Ok - Spec file matches base package name. OK - Spec has consistent macro usage. ISSUE (4) - Meets Packaging Guidelines. OK - License OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: a2e2e54d65e9eae961f7171335cf2550 rpmbuild/SOURCES/apr-util-1.2.8.tar.gz a2e2e54d65e9eae961f7171335cf2550 reviews/apr-util/apr-util-1.2.8.tar.gz
ISSUE (6) - BuildRequires correct OK - Spec handles locales/find_lang OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. ISSUE (1) - Package has correct buildroot OK - Package is code or permissible content. OK - Doc subpackage needed/used. OK - Packages %doc files don't affect runtime.
OK - Headers/static libs in -devel subpackage. OK - Spec has needed ldconfig in post and postun OK - .pc files in -devel subpackage/requires pkgconfig OK - .so files in -devel subpackage. OK - -devel package Requires: %{name} = %{version}-%{release} ISSUE (7) - .la files are removed.
OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. ISSUE (5) - rpmlint output. OK - Should build in mock. OK - Should have sane scriptlets. OK - Should have subpackages require base package with fully versioned depend. ISSUE (2) - Should have dist tag OK - Should package latest version ISSUE (3) - check for outstanding bugs on package. (For core merge reviews)
Issues:
1. Build root is not one of the recommended options
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
2. It is recommended to use %{?dist} in Release:
3. There is an outstanding bug for apr-util, but it may not apply to Fedora.
4. Conflicts: should not be used. Perhaps Requires: subversion >= 0.20.1-2 instead?
5 rpmlint output
W: apr-util invalid-license Apache Software License 2.0 W: apr-util-debuginfo invalid-license Apache Software License 2.0 W: apr-util-devel invalid-license Apache Software License 2.0 W: apr-util-mysql invalid-license Apache Software License 2.0 W: apr-util-pgsql invalid-license Apache Software License 2.0
probably false positives
E: apr-util use-of-RPM_SOURCE_DIR
in %prep use cp ${SOURCE1} dbd instead of cp $RPM_SOURCE_DIR/apr_dbd_mysql.c dbd
W: apr-util-devel no-documentation W: apr-util-mysql no-documentation W: apr-util-pgsql no-documentation
Is there any documentation to include?
6. Build Requires doxygen is in BuildRequires but it does not seem to be used in the build process. Is a step missing in the build process to create the documentation and then put %doc for each of the -devel -mysql and -pgsql sub packages ?
7. .la files exist and must be removed
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From jeremy@hinegardner.org 2007-03-30 01:14 EST ------- (In reply to comment #2)
- Conflicts: should not be used. Perhaps Requires: subversion >= 0.20.1-2 instead?
I think the Conflicts: should just be removed completely since apr-util does not actually depend on subversion.
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
jorton@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
------- Additional Comments From jorton@redhat.com 2007-04-05 12:11 EST ------- Thanks for the review! Please see 1.2.8-7 in CVS/Raw Hide soon.
- Build root is not one of the recommended options
The buildroot used does meet the mandatory requirements.
- It is recommended to use %{?dist} in Release:
This is not mandatory.
- There is an outstanding bug for apr-util, but it may not apply to Fedora.
Not relevant to review process.
- Conflicts: should not be used. Perhaps Requires: subversion >= 0.20.1-2 instead?
Removed, as suggested.
5 rpmlint output
W: apr-util invalid-license Apache Software License 2.0
No standard for License tags exists yet.
E: apr-util use-of-RPM_SOURCE_DIR
This is not prohibited by packaging guidelines, see f-m@ flamewar.
W: apr-util-devel no-documentation
All expected.
- Build Requires doxygen is in BuildRequires but it does not seem to be used in the
Removed.
- .la files exist and must be removed
The .la file is part of the defined build interface for this package.
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From jeremy@hinegardner.org 2007-04-08 01:55 EST ------- (In reply to comment #4) Thanks for the changes. Everything is good except for (7)
E: apr-util use-of-RPM_SOURCE_DIR
This is not prohibited by packaging guidelines, see f-m@ flamewar.
Understood, since the spec does not write to RPM_SOURCE_DIR then it is ok.
- .la files exist and must be removed
The .la file is part of the defined build interface for this package.
According to http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
- MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec.
Accordingly I cannot give final approval until this is addressed.
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From redhat-bugzilla@linuxnetz.de 2007-04-08 05:57 EST ------- Jeremy, you're wrong. There ARE cases, where you NEED a .la file (e.g. neon, ImageMagick just to name a few, to avoid breakings of e.g. rpm or convert).
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From redhat-bugzilla@linuxnetz.de 2007-04-08 05:59 EST ------- And if Joe says .la is needed, we IMHO really should trust him (he maintains neon package and knows how to handle .la files, because rpm depends on neon and you can't simply drop the .la file there, too).
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: apr-util
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225254
------- Additional Comments From rdieter@math.unl.edu 2007-04-09 08:08 EST ------- Created an attachment (id=151985) --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=151985&action=vie...) move static-only libs to Libs.private section
similar to pkgconfig patch provided for apr
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: apr-util
https://bugzilla.redhat.com/show_bug.cgi?id=225254
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide
jeremy@hinegardner.org changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|jeremy@hinegardner.org |nobody@fedoraproject.org Status|MODIFIED |ASSIGNED Flag|fedora-review? |
------- Additional Comments From jeremy@hinegardner.org 2008-01-20 12:31 EST ------- I'm unable to continue reviewing this package. Someone else should probably take it over.
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=225254
Bojan Smojver bojan@rexursive.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bojan@rexursive.com
--- Comment #10 from Bojan Smojver bojan@rexursive.com 2008-08-22 22:19:52 EDT --- CVS admins, please remove tag apr-util-1_3_4-8_fc10. It was created by mistake.
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=225254
--- Comment #11 from Kevin Fenzi kevin@tummy.com 2008-08-23 00:26:24 EDT --- We don't remove tags from cvs. Just bump the release and retag, or use 'make force-tag'.
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=225254
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW
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=225254
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name Status Whiteboard| |NotReady
--- Comment #12 from Till Maas opensource@till.name 2009-09-16 19:36:29 EDT --- Please explain why the patch from comment:8 cannot be used or apply it. Also the patches need an explanation wrt. their upstream status: http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_a...
Please remove NotReady from the whiteboard, once this is done.
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=225254
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |limburgher@gmail.com AssignedTo|nobody@fedoraproject.org |limburgher@gmail.com Flag| |fedora-review?
--- Comment #13 from Jon Ciesla limburgher@gmail.com 2012-04-03 13:13:21 EDT --- Fresh review:
Good:
- rpmlint checks return:
apr-util-devel.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-devel.x86_64: W: no-manual-page-for-binary apu-1-config Each executable in standard binary directories should have a man page.
apr-util-freetds.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-ldap.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-mysql.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-nss.x86_64: W: spelling-error Summary(en_US) crytpo -> crypt The value of this tag appears to be misspelled. Please double-check.
apr-util-nss.x86_64: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o The value of this tag appears to be misspelled. Please double-check.
apr-util-nss.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-odbc.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-openssl.x86_64: W: spelling-error Summary(en_US) crytpo -> crypt The value of this tag appears to be misspelled. Please double-check.
apr-util-openssl.x86_64: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o The value of this tag appears to be misspelled. Please double-check.
apr-util-openssl.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-pgsql.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
apr-util-sqlite.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files.
Mostly ignorable.
- package meets naming guidelines - package meets packaging guidelines - license ( ASL 2.0 ) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file - devel package ok - no .la files
Patch from comment * should be applied.
- post/postun ldconfig ok - devel requires base package n-v-r
So it's pretty much just the static lib issue. Let me know if you want me to commit anything.
package-review@lists.fedoraproject.org