Bug 463123

Summary: Review Request: gnomint - Graphical x509 Certification Authority management tool
Product: [Fedora] Fedora Reporter: Adam Huffman <bloch>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: mtasaka: fedora-review+
huzaifas: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-10-19 06:45:46 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 463501    
Bug Blocks:    

Description Adam Huffman 2008-09-21 23:34:50 UTC
Spec URL: http://verdurin.fedorapeople.org/review/gnomint/gnomint.spec
SRPM URL: http://verdurin.fedorapeople.org/review/gnomint/gnomint-0.5.3-1.fc10.src.rpm
Description: Certification Authority management made easy.

gnoMint is a x509 Certification Authority management tool for
GTK/Gnome environments.

Comment 1 Mamoru TASAKA 2008-09-23 16:00:07 UTC
For 0.5.3-1:

* License
  - As far as I checked the source codes, the license
    tag should be "GPLv3+".

* SourceURL
  - You may want to use %{name}, %{version} (especially %{version})
    tags for SourceURL, as with these tags you probably won't have
    to change SourceURL when the version is upgraded.

* BuildRequires
  * Perl module dependency
    - For perl module dependency, please don't write the rpm
      name directly but use virtual Provides modules names the
      rpm will provide
      (i.e. "BuildRequries: perl-XML-Parser" should be
       "BuildRequires: perl(XML::Parser)" :
       https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides )

  * gnutls BR
    - You should use "BuildRequires: gnutls-devel >= 2" and
      "Requires: gnutls >= 2" is not needed.

  * Redundant BR
    - Some BRs are redundant.
      ! gtk2-devel, GConf2-devel, libglade2-devel - all required
        by libgnomeui-devel
      ! libgcrypt-devel - required by gnutls-devel

* Compiler flags
  - build log shows Fedora specific compilations are not correctly passed.
    (You can check what compilar flags Fedora uses now by
     $ rpm --eval %optflags )

* Timestamps
  - Please consider to use
----------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    autotools.

* Desktop file
  - Category "Application;" is deprecated and should be removed
    (you can use --remove-category option).

* Scriptlets
  - Requires(post) or so is missing. Please refer to
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf

* Documents
  - Usually the file "INSTALL" is for people who want to compile
    and install this package by themselves and is not needed for
    rpm users.
  - "BUGS" has zero size and there doesn't seem to be any reason to
    contain this file.

Comment 2 Adam Huffman 2008-09-23 23:17:33 UTC
I have addressed most of these comments (and thanks again for all this assistance).  Two remaining points - having fixed the compiler flags, there is now an error during compilation.  I've contacted the upstream developer about this.  Secondly, on the Desktop file comment - the "--remove-category" option to desktop-file-install isn't listed when invoked with --help.  Should I report that as a bug?

Once I have resolved the compilation error I'll upload a new version.

Comment 3 Mamoru TASAKA 2008-09-24 03:00:33 UTC
(In reply to comment #2)
> having fixed the compiler flags, there is
> now an error during compilation.  I've contacted the upstream developer about
> this.  

If you want to do a workaround for this, perhaps it is sufficient that you
remove -Werror (however if you want to wait for upstream fix it is okay)

> Secondly, on the Desktop file comment - the "--remove-category" option
> to desktop-file-install isn't listed when invoked with --help.  Should I report
> that as a bug?

Perhaps "--help-all" will show the option (--help says:
"--help-all  Show all help options")

Comment 4 Mamoru TASAKA 2008-10-02 15:22:04 UTC
ping?

Comment 5 Adam Huffman 2008-10-02 16:47:21 UTC
I found a fix for that compilation error, but then there was another error.  Again I corresponded with the upstream author and it appears to be a glib bug:

http://bugzilla.gnome.org/show_bug.cgi?id=316221

Is it acceptable to turn off -Werror in the meantime?

Comment 6 Mamoru TASAKA 2008-10-02 17:23:37 UTC
(In reply to comment #5)
> Is it acceptable to turn off -Werror in the meantime?

Yes.

Comment 7 Adam Huffman 2008-10-05 10:17:43 UTC
New version at:

http://verdurin.fedorapeople.org/review/gnomint/gnomint-0.5.4-1.fc10.src.rpm

http://verdurin.fedorapeople.org/review/gnomint/gnomint.spec

Includes more fixes and new upstream release.

Comment 8 Mamoru TASAKA 2008-10-08 13:38:37 UTC
Almost okay.

* CFLAGS
* Gconf schemas install
  -- Actually the reason this build didn't honor Fedora cflags
     is because of configure(.in)

     From configure.in:
-----------------------------------------------------------
   119  if test "x$GCC" = "xyes"; then
   120          CFLAGS="-Wall -Werror -g "
   121  fi
-----------------------------------------------------------
     The line 120 is completely unneeded (the corresponding
     line on configure is around the lines 16745)
     For this package just modifying configure is sufficient.

   - build.log shows:
-----------------------------------------------------------
   560  if test -z "/builddir/build/BUILDROOT/gnomint-0.5.4-1.fc10.i386" ; then \
   561                  for p in gnomint.schemas ; do \
   562                          GCONF_CONFIG_SOURCE=xml:merged:/etc/gconf/gconf.xml.defaults gconftool-2 --makefile-install-rule $p; \
   563                  done \
   564          fi
-----------------------------------------------------------
     Calling gconf-2 during rpmbuild is not preferred (see:
     https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf )

     Fortunately configure accepts --disable-schemas-install, which
     prevents gconf-2 call.

    So changing %build stage as below will solve these two issues:
-----------------------------------------------------------
%build
sed -i -e 's|CFLAGS="-Wall -Werror -g "|true|' configure
%configure --disable-schemas-install
make %{?_smp_mflags}
-----------------------------------------------------------

* Scriptlets
  - For safety close the lines with || : even for update-desktop-database:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database


Other things are okay.
-----------------------------------------------------------
   This package (gnomint) is APPROVED by mtasaka
-----------------------------------------------------------
As I wrote on another review request:

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji) ".
I am already sponsoring you.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Comment 9 Adam Huffman 2008-10-08 23:53:27 UTC
Thanks again.  Have uploaded:

http://verdurin.fedorapeople.org/review/gnomint/gnomint.spec
http://verdurin.fedorapeople.org/review/gnomint/gnomint-0.5.4-2.fc10.src.rpm

in the light of your comments.  

One thing I noticed during rpmbuild is the message "/home/adam/rpmbuild/BUILDROOT/gnomint-0.5.4-2.fc10.x86_64/usr/share/applications/gnomint.desktop: key "Categories" is a list and does not have a semicolon as trailing character, fixing".

Would it be cleaner to remove the "Application" category with a patch, rather than using  the --remove-category option?  Or doesn't it really matter?

Comment 10 Adam Huffman 2008-10-09 00:28:19 UTC
New Package CVS Request
=======================
Package Name: gnomint
Short Description: Graphical x509 Certification Authority management tool
Owners: verdurin
Branches: F-9 F-10
InitialCC: verdurin

Comment 11 Mamoru TASAKA 2008-10-09 01:39:17 UTC
(In reply to comment #9)
> One thing I noticed during rpmbuild is the message
> "/home/adam/rpmbuild/BUILDROOT/gnomint-0.5.4-2.fc10.x86_64/usr/share/applications/gnomint.desktop:
> key "Categories" is a list and does not have a semicolon as trailing character,
> fixing".
> 
> Would it be cleaner to remove the "Application" category with a patch, rather
> than using  the --remove-category option?  Or doesn't it really matter?

It does not matter because as the message says desktop-file-install fixes
this "Category does not end with semicolon" common mistake.
If you want to fix this by patch it should modify gui/gnomint.desktop.in
like:
--------------------------------------------------------
Categories=GNOME;System;Security;
--------------------------------------------------------
(i.e. Security must have semicolon), however as I said desktop-file-install
fixes this issue.

Comment 12 Huzaifa S. Sidhpurwala 2008-10-09 02:53:20 UTC
cvs done

Comment 13 Mamoru TASAKA 2008-10-16 15:20:46 UTC
ping?

Comment 14 Adam Huffman 2008-10-18 10:12:45 UTC
I've built this package for F-9 and F-10 and it seems to be running acceptably on both.  What is the procedure now?

Does the package need to be imported into the most recent release before the review is considered closed?

Comment 15 Mamoru TASAKA 2008-10-18 14:57:37 UTC
For F-9 package please submit a request to push the rebuilt packages
to repositories on bodhi:
https://admin.fedoraproject.org/updates/

Then you can close this bug as NEXTRELEASE.

Comment 16 Fedora Update System 2008-10-18 22:50:52 UTC
gnomint-0.5.4-2.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/gnomint-0.5.4-2.fc9

Comment 17 Mamoru TASAKA 2008-10-19 06:45:46 UTC
Okay, thanks.

When you think the rebuilt packages can be moved from testing to
stable repository, please modify (edit) the submitted push request.

Closing as NEXTRELEASE.

Comment 18 Fedora Update System 2008-11-12 02:56:10 UTC
gnomint-0.5.4-2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.