[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||polarssl-0.14.0-4.fc14 Resolution||ERRATA Last Closed||2011-02-09 15:20:29 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #15 from Fedora Update System upda...@fedoraproject.org 2011-02-09 15:20:23 EST --- polarssl-0.14.0-4.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #14 from Fedora Update System upda...@fedoraproject.org 2011-02-01 15:55:01 EST --- polarssl-0.14.0-4.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update polarssl'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/polarssl-0.14.0-4.fc14 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #13 from Mads Kiilerich m...@kiilerich.com 2011-01-31 19:40:29 EST --- Thanks for the review(s). Upstream is very friendly and competent but also very busy, quiet and private, so it will be interesting to see how this will work out ... -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #11 from Dennis Gilmore den...@ausil.us 2011-01-30 19:02:48 EST --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025
Martin Gieseking martin.giesek...@uos.de changed:
What|Removed |Added
Status|NEW |ASSIGNED
AssignedTo|nob...@fedoraproject.org|martin.giesek...@uos.de
Flag||fedora-review+
--- Comment #9 from Martin Gieseking martin.giesek...@uos.de 2011-01-29
05:49:03 EST ---
OK, I'm volunteering. :)
The package looks fine. However, I recommend to apply the minor improvements
mentioned in comment #5. Also, please add short comments above the Patch fields
telling what the patches do. These are no blockers though.
$ rpmlint /var/lib/mock/fedora-14-x86_64/result/*.rpm
4 packages and 0 specfiles checked; 0 errors, 0 warnings.
-
key:
[+] OK
[.] OK, not applicable
[X] needs work
-
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
GPLv2+
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The file containing the text of the license(s) for the package must
be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
$ md5sum polarssl-0.14.0-gpl.tgz*
669a0582a27a5ec381542f0c67e966b7 polarssl-0.14.0-gpl.tgz
669a0582a27a5ec381542f0c67e966b7 polarssl-0.14.0-gpl.tgz.1
[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
koji scratch build (f15):
http://koji.fedoraproject.org/koji/taskinfo?taskID=2748827
[.] MUST: If the package does not successfully compile, build or work on an
architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, and Fortran files, %{optflags} must be
applied.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: If a package installs files below %{_datadir}/icons, the icon cache
must be updated.
[+] MUST: Packages storing shared library files (not just symlinks) must call
ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates.
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[.] MUST: Packages must not provide RPM dependency information when that
information is not global in nature, or are otherwise handled.
[.] MUST: When filtering automatically generated RPM dependency information,
the filtering system implemented by Fedora must be used.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[+] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[+] MUST: .so (without suffix) must go in a -devel package.
[+] MUST: devel packages must require the base package using a fully versioned
dependency
[+] MUST: Packages must NOT contain any .la libtool archives, these must be
removed in the spec if they are built.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop
file
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.
EPEL = 5 only:
[+] MUST: The spec file must contain a valid BuildRoot field.
[+] MUST: At the beginning of %install, each package must run rm -rf
%{buildroot}.
[+] MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot}.
[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
[.] SHOULD: If the source package does not include license text(s) as a
separate file from upstream,...
[X] SHOULD: Patch files should be prefixed with %{name}-
[X] SHOULD: All patches should be commented in the spec file.
[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 Mads Kiilerich m...@kiilerich.com changed: What|Removed |Added Flag||fedora-cvs? --- Comment #10 from Mads Kiilerich m...@kiilerich.com 2011-01-29 20:20:31 EST --- New Package SCM Request === Package Name: polarssl Short Description: Light-weight cryptographic and SSL/TLS library Owners: kiilerix Branches: f14 InitialCC: -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #8 from Jason Tibbitts ti...@math.uh.edu 2011-01-28 10:49:20 EST --- Golo, since you seem to be sponsored now, did you want to finish this review? Or, heck, anyone else who has commented here? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 Andy Bailey bai...@akamai.com changed: What|Removed |Added CC||bai...@akamai.com --- Comment #6 from Andy Bailey bai...@akamai.com 2010-12-08 12:04:21 EST --- Here's the culprit for the broken x86_64 build: --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -26,3 +26,7 @@ x509parse.c xtea.c ) + +INSTALL(TARGETS polarssl +DESTINATION lib +PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #4 from Golo Fuchert packa...@golotop.de 2010-11-13 04:13:31 EST --- First I like to clarify my comment [o] No Header files included. This sounds misleading and should of course be [+] ... outside of the -devel package. The package now builds fine and from my side there are no further objections. So let's see what the Gurus say! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025
Martin Gieseking martin.giesek...@uos.de changed:
What|Removed |Added
CC||martin.giesek...@uos.de
--- Comment #5 from Martin Gieseking martin.giesek...@uos.de 2010-11-13
04:45:13 EST ---
Here are two more (minor) things to consider:
- Please prefix the patch filenames with polarssl-. This way it's much easier
to identify them in rpmbuild/SOURCES.
- I also recommend to be more specific in %files:
%{_libexecdir}/* = %{_libexecdir}/polarssl/
%{_includedir}/* = %{_includedir}/polarssl/
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025
--- Comment #2 from Golo Fuchert packa...@golotop.de 2010-11-12 17:52:08 EST
---
The package looks quite clean to me, except of one issue (see below). But since
I am not yet sponsored I can only make an inofficial review.
Inofficial review:
[+] = ok
[o] = does not apply
[-] = not ok
[?] = see comments
[+] rpmlint is quiet
rpmlint polarssl-0.14.0-1.fc13.src.rpm
../RPMS/i686/polarssl-devel-0.14.0-1.fc14.i686.rpm
../RPMS/i686/polarssl-0.14.0-1.fc14.i686.rpm ../SPECS/polarssl.spec
3 packages and 1 specfiles checked; 0 errors, 0 warnings.
[+] The package is named according to the guidelines
[+] Spec file name matches base package name
[+] The package follows the Packaging Guidelines
[+] The license is an approved licence
[+] The License field matches the actual licence
[+] License file from source file is included in %doc
[+] The spec file is written in American English
[+] The spec file is legible
[+] Used sources match width upstream sources (md5)
[+] Package build at least on one primary architecture (i686)
[?] ExecludeArch for x86_64?? SEE COMMENT BELOW
[+] All build dependencies are listed in the BuildRequires section
[o] No locales for the package
[+] Package stores shared libraries and calls ldconfig in %post/%postun
[+] Package does not bundle copies of system libraries
[o] Package is not relocatable
[o] Package does not install any directories to be owned
[+] No files are listed more then once in the %files section
[+] File permissions are set properly (%defattr(...) is used)
[+] Consistent use of macros
[+] Package contains code and documentation only, no content
[o] No large documentation files
[+] %doc files do not affect runtime
[o] No Header files included
[o] No static libraries
[+] Library files ending with .so correctly in a -devel package
[+] -devel package requires the package correctly as %{name} =
%{version}-%{release}
[+] No libtool .la archives included
[o] No GUI application
[+] Package does not own files or directories that are owned by other packages
[+] All filenames are valid UTF-8
-
Comments:
- Wouldn't it be good practice to comment on the patches (i.e. where do they
come from, why are they needed and does upstream know about them)? I think the
guidelines encourage you to do this
(http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment).
- The package fails to build here on the x86_64 architecture. Some files seem
to be put in $BUILDROOT/usr/lib but later %{_libdir} is expanded to /usr/lib64
(of course). My fault or a bug?
-
So in my opinion the package is ready after either dealing with the x86_64
issue, excluding this arch or tell me what I did wrong. ;-)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025
Golo Fuchert packa...@golotop.de changed:
What|Removed |Added
CC||packa...@golotop.de
--- Comment #1 from Golo Fuchert packa...@golotop.de 2010-11-12 17:50:55 EST
---
The package looks quite clean to me, except of one issue (see below). But since
I am not yet sponsored I can only make an inofficial review.
Inofficial review:
[+] = ok
[o] = does not apply
[-] = not ok
[?] = see comments
[+] rpmlint is quiet
rpmlint polarssl-0.14.0-1.fc13.src.rpm
../RPMS/i686/polarssl-devel-0.14.0-1.fc14.i686.rpm
../RPMS/i686/polarssl-0.14.0-1.fc14.i686.rpm ../SPECS/polarssl.spec
3 packages and 1 specfiles checked; 0 errors, 0 warnings.
[+] The package is named according to the guidelines
[+] Spec file name matches base package name
[+] The package follows the Packaging Guidelines
[+] The license is an approved licence
[+] The License field matches the actual licence
[+] License file from source file is included in %doc
[+] The spec file is written in American English
[+] The spec file is legible
[+] Used sources match width upstream sources (md5)
[+] Package build at least on one primary architecture (i686)
[?] ExecludeArch for x86_64?? SEE COMMENT BELOW
[+] All build dependencies are listed in the BuildRequires section
[o] No locales for the package
[+] Package stores shared libraries and calls ldconfig in %post/%postun
[+] Package does not bundle copies of system libraries
[o] Package is not relocatable
[o] Package does not install any directories to be owned
[+] No files are listed more then once in the %files section
[+] File permissions are set properly (%defattr(...) is used)
[+] Consistent use of macros
[+] Package contains code and documentation only, no content
[o] No large documentation files
[+] %doc files do not affect runtime
[o] No Header files included
[o] No static libraries
[+] Library files ending with .so correctly in a -devel package
[+] -devel package requires the package correctly as %{name} =
%{version}-%{release}
[+] No libtool .la archives included
[o] No GUI application
[+] Package does not own files or directories that are owned by other packages
[+] All filenames are valid UTF-8
-
Comments:
- Wouldn't it be good practice to comment on the patches (i.e. where do they
come from, why are they needed and does upstream know about them)? I think the
guidelines encourage you to do this
(http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment).
- The package fails to build here on the x86_64 architecture. Some files seem
to be put in $BUILDROOT/usr/lib but later %{_libdir} is expanded to /usr/lib64
(of course). My fault or a bug?
-
So in my opinion the package is ready after either dealing with the x86_64
issue, excluding this arch or tell me what I did wrong. ;-)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 634025] Review Request: PolarSSL - Light-weight cryptographic and SSL/TLS library
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=634025 --- Comment #3 from Mads Kiilerich m...@kiilerich.com 2010-11-12 19:39:58 EST --- Thank you for the comments. Please verify that the issues are addressed with: http://people.fedoraproject.org/~kiilerix/polarssl.spec http://people.fedoraproject.org/~kiilerix/polarssl-0.14.0-2.fc14.src.rpm http://koji.fedoraproject.org/koji/taskinfo?taskID=2598431 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
