Bug 465382 - (bouncycastle-mail) Review Request: bouncycastle-mail - SMIME/CMS packages for Bouncy Castle
Review Request: bouncycastle-mail - SMIME/CMS packages for Bouncy Castle
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: itext
  Show dependency treegraph
 
Reported: 2008-10-02 20:52 EDT by Orcan Ogetbil
Modified: 2008-11-14 07:46 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-08 01:47:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Orcan Ogetbil 2008-10-02 20:52:43 EDT
Spec URL: http://oget.fedorapeople.org/review/bouncycastle-mail.spec
SRPM URL: http://oget.fedorapeople.org/review/bouncycastle-mail-1.41-1.fc10.src.rpm
Description: SMIME/CMS packages for Bouncy Castle

Depends on bouncycastle-%{version} which is in koji and will be in rawhide shortly. The spec file is directly taken from bouncycastle and modified accordingly.

See:
https://bugzilla.redhat.com/show_bug.cgi?id=465203
Comment 1 Mamoru TASAKA 2008-10-05 11:23:14 EDT
For 1.41-1

* Summary/Description
  - Would you change Summary/Description more informative?
    I don't think the Summary "Additional libraries" makes
    much sense.

* Naming
  - First of all, why is this srpm named as "bouncycastle-mail",
    not "bcmail"?

* License
  - License tag should be "MIT"

* SourceURL
  - SOURCE0 must be written with full URL:
    https://fedoraproject.org/wiki/Packaging/SourceURL

* (Build)Requires
  - I guess "BuildRequires: java-devel >= 1.7" is better than
    java-1.7.0-icedtea-devel.

* unpackaging source / removing precompiled binaries
  - Please unpack all sources in the tarball before removing
    precompiled binaries to make it sure that all precompiled
    binaries (including those in zip files if any) are
    correctly removed.

  ! By the way when using "unzip" adding "-qq" option is
    preferred. When using zip source tarball %setup -q
    uses this.

* absolute symlink
-----------------------------------------------------
W: symlink-should-be-relative /usr/share/java/gcj-endorsed/bcmail-1.41.jar /usr/share/java/bcmail-1.41.jar
-----------------------------------------------------
  - Mainly for chroot reason and so on, Fedora requests that all
    symlinks should be relative, not absolute.

! %postun
-----------------------------------------------------
%postun
if [ $1 -eq 0 ] ; then
  if [ -x %{_bindir}/rebuild-gcj-db ]; then
    %{_bindir}/rebuild-gcj-db
  fi
fi
-----------------------------------------------------
  - While I am not familiar with gcj, would you explain why
    it is sufficient that these scripts are called only when
    [ $1 -eq 0 ] ? (please also refer to:
    https://fedoraproject.org/wiki/Packaging/GCJGuidelines )

* %attr
  - Although GCJGuidelines uses it, the part "%attr(-,root,root)"
    is completely redundant.
Comment 2 Orcan Ogetbil 2008-10-05 23:27:12 EDT
(In reply to comment #1)
Dear mtasaka, 
Thanks for the review. I stole the spec file directly from the fedora bouncycastle package and did not modify much except the %{name}'s. I knew it lacks certain things. All of the errors/flaws of my spec is inherited from the original spec file.

> For 1.41-1
> 
> * Summary/Description
>   - Would you change Summary/Description more informative?
>     I don't think the Summary "Additional libraries" makes
>     much sense.
> 
I added to Summary/Description. I think it's better now.

> * Naming
>   - First of all, why is this srpm named as "bouncycastle-mail",
>     not "bcmail"?
> 
Let me tell you the situation. The actual Bouncy Castle is a suite consisting of many libraries. bcprov* and bcmail are two of these libraries among many others. In Fedora, the bcprov library is already packed as "bouncycastle.<version>.rpm" but not "bcprov.<version>.rpm". 

Originally, I was going back and forth between the names: bcmail and bouncycastle-mail . I decided on the latter for the sake of staying consistent with the existing bouncycastle package. But I am fine with renaming the package. 

Let me know what you think.

> * License
>   - License tag should be "MIT"
> 
I know. See comment #4 of 
https://bugzilla.redhat.com/show_bug.cgi?id=465203
Fixed now.

> * SourceURL
>   - SOURCE0 must be written with full URL:
>     https://fedoraproject.org/wiki/Packaging/SourceURL
> 
Done


> * (Build)Requires
>   - I guess "BuildRequires: java-devel >= 1.7" is better than
>     java-1.7.0-icedtea-devel.
> 
Done

> * unpackaging source / removing precompiled binaries
>   - Please unpack all sources in the tarball before removing
>     precompiled binaries to make it sure that all precompiled
>     binaries (including those in zip files if any) are
>     correctly removed.
> 
>   ! By the way when using "unzip" adding "-qq" option is
>     preferred. When using zip source tarball %setup -q
>     uses this.
> 
Fixed

> * absolute symlink
> -----------------------------------------------------
> W: symlink-should-be-relative /usr/share/java/gcj-endorsed/bcmail-1.41.jar
> /usr/share/java/bcmail-1.41.jar
> -----------------------------------------------------
>   - Mainly for chroot reason and so on, Fedora requests that all
>     symlinks should be relative, not absolute.
> 
Fixed

> ! %postun
> -----------------------------------------------------
> %postun
> if [ $1 -eq 0 ] ; then
>   if [ -x %{_bindir}/rebuild-gcj-db ]; then
>     %{_bindir}/rebuild-gcj-db
>   fi
> fi
> -----------------------------------------------------
>   - While I am not familiar with gcj, would you explain why
>     it is sufficient that these scripts are called only when
>     [ $1 -eq 0 ] ? (please also refer to:
>     https://fedoraproject.org/wiki/Packaging/GCJGuidelines )
> 
> * %attr
>   - Although GCJGuidelines uses it, the part "%attr(-,root,root)"
>     is completely redundant.

My best answer will be: That's the way it is in the original bouncycastle.spec file. Maybe it remained from pre-F-8 days where no JDK was available. Now I took that part off and redesigned the parts regarding GCJ honoring the guidelines (except %attr).

I added the if-clauses "%if %{with_gcj}" as the guidelines propose but this results in the rpmlint warning:
   bouncycastle-mail.spec:98: W: libdir-macro-in-noarch-package   %{_libdir}/gcj/%{name}
which is a wrong warning because noarch does not apply to that line (is this an rpmlint bug?). Should I take those "%if %{with_gcj}" off from the spec file?**

I packaged bcmail because it is a requirement for iText (bug #   	    465511) which will let me enable the pdf plugin of tuxguitar in the future. I don't know much about the cryptography otherwise.

New files:
SPEC: http://oget.fedorapeople.org/review/bouncycastle-mail.spec
SRPM: http://oget.fedorapeople.org/review/bouncycastle-mail-1.41-2.fc10.src.rpm

Thanks again!

*bcprov is the main library. The other libraries depend on it and they don't mean anything without it.
** You can build the package with "rpmbuild -ba --without gcj bouncycastle-mail.spec" now and this will produce a noarch rpm, without those arch dependent .so files produced by gcj.
Comment 3 Mamoru TASAKA 2008-10-06 10:45:03 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > * Naming
> >   - First of all, why is this srpm named as "bouncycastle-mail",
> >     not "bcmail"?
> > 
> Let me tell you the situation. The actual Bouncy Castle is a suite consisting
> of many libraries. bcprov* and bcmail are two of these libraries among many
> others. In Fedora, the bcprov library is already packed as
> "bouncycastle.<version>.rpm" but not "bcprov.<version>.rpm". 
>
> Let me know what you think.
  - It may be better that "Provides: bcmail = %{version}-%{release}" is
    added (if you think if it is worth doing)

> I added the if-clauses "%if %{with_gcj}" as the guidelines propose but this
> results in the rpmlint warning:
>    bouncycastle-mail.spec:98: W: libdir-macro-in-noarch-package  
> %{_libdir}/gcj/%{name}
> which is a wrong warning because noarch does not apply to that line (is this an
> rpmlint bug?). Should I take those "%if %{with_gcj}" off from the spec file?**

  - Please ignore this rpmlint complaint for this case. You can also use
----------------------------------------------------------
%if %{with_gcj}
  %{_javadir}/gcj-endorsed/bcmail-%{version}.jar
  %{_libdir}/gcj/%{name}
%endif
----------------------------------------------------------

> *bcprov is the main library. The other libraries depend on it and they don't
> mean anything without it.
  - Then perhaps "Requires: bouncycastle (= %{version})" is needed?

Other things are okay.
-------------------------------------------------------------------
    This package (bouncycastle-mail) is APPROVED by mtasaka
-------------------------------------------------------------------

By the way would you have some time to review my review request
(bug 465740)?
Comment 4 Orcan Ogetbil 2008-10-06 12:46:29 EDT
Thanks, I added them since they may help some people.
I'll have a look at your package now.

New Package CVS Request
=======================
Package Name: bouncycastle-mail
Short Description: S/MIME and CMS libraries for Bouncy Castle
Owners: oget
Branches:
InitialCC:
Comment 5 Kevin Fenzi 2008-10-07 13:37:13 EDT
cvs done.
Comment 6 Mamoru TASAKA 2008-10-08 01:47:21 EDT
Closing (as this is requested for only F-10).
Comment 7 Mary Ellen Foster 2008-11-06 09:37:06 EST
Any chance of this making it into Fedora 9 as well? It's a dependency of something I want to package ...
Comment 8 Orcan Ogetbil 2008-11-06 10:43:22 EST
Yes that is something I want to do as well. Bouncycastle (bcprov) of F-9 is an old version. That needs to be updated if we want to push bcmail into F-9.

I have been trying to reach the F-9 maintainer (fitzsim) of bcprov for a while. He didn't reply to my emails and my pkgdb requests. You might, as well, want to try your chances with him. I'd be happy to make bcmail into F-9 if he allows me to co-maintain bcprov, or updates it himself.
Comment 9 Mary Ellen Foster 2008-11-06 10:52:20 EST
For what it's worth, I just tried rebuilding the bouncycastle-mail SRPM on Fedora 9. All I had to do was change 1.41 to 1.38 (the current F9 version) in a couple of places and it seemed to work fine.
Comment 10 Orcan Ogetbil 2008-11-06 11:29:59 EST
Well, I thought about packaging bcmail-1.38 for F-9. The source is available. But this violates the "Latest version must be packaged" guideline. There's not much I can do for F-9 since my hands are tied.
Comment 11 Orcan Ogetbil 2008-11-11 14:48:08 EST
I finally got a hold of fitzsim. He released bouncycastle to me. I will update both bcprov and bcmail this week (for now for F-9 only. I'm not sure about F-8).

Package Change Request
=======================
Package Name: bouncycastle-mail
New Branch: F-9
Owner: oget
Comment 12 Kevin Fenzi 2008-11-12 13:57:42 EST
cvs done.
Comment 13 Fedora Update System 2008-11-12 16:10:55 EST
bouncycastle-mail-1.41-3.fc9,bouncycastle-1.41-2.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/bouncycastle-mail-1.41-3.fc9,bouncycastle-1.41-2.fc9
Comment 14 Fedora Update System 2008-11-14 07:46:06 EST
bouncycastle-mail-1.41-3.fc9, bouncycastle-1.41-2.fc9 has been pushed to the Fedora 9 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.