Bug 201656 - Review Request: gstm
Review Request: gstm
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
: Reopened
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-08-07 19:21 EDT by Damien Durand
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-06 16:02:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Patch that create a gaskpass package (748 bytes, patch)
2006-08-22 05:30 EDT, Laurent Rineau
no flags Details | Diff

  None (edit)
Description Damien Durand 2006-08-07 19:21:35 EDT
Spec URL: http://glive.tuxfamily.org/fedora/gstm/gstm.spec
SRPM URL: http://glive.tuxfamily.org/fedora/gstm/gstm-1.2-1.fc6.src.rpm
Description: Gstm, short for Gnome SSH Tunnel Manager, is a graphical front-end for managing ssh tunneled portredirects.
Comment 1 Parag AN(पराग) 2006-08-08 00:09:44 EDT
packaging looks ok.
+ Mockbuild is successfull for i386 FC6
+ rpmlint on binary rpm is silent
+ dist tag is present
+ Buildroot is correct
+ source URL is correct
+ BR is correct
+ License used is GPL
+ License file COPYING is included
+ Desktop file is handled correctly
+ MD5 sum on tarball is matching upstream tarball
7fa71b86969d8d695c3b062780a5694e  gstm-1.2.tar.gz
+ No duplicate files
Comment 2 Jason Tibbitts 2006-08-08 23:26:35 EDT
Why has this been closed and checked in?  I don't see any approval, and it's
still blocking FE-NEW.
Comment 3 Damien Durand 2006-08-09 04:30:58 EDT
Confused... But make the a complet review please! Someone could review this package?
Comment 4 Jason Tibbitts 2006-08-09 11:08:26 EDT
In the interests of getting everything straightened out, I'll go ahead and
review this package.
Comment 5 Jason Tibbitts 2006-08-09 21:05:25 EDT
The only rpmlint complaint:

W: gstm mixed-use-of-spaces-and-tabs
  There is a mix of indentation types used in the spec, but I don't know if
there's a particular use that it's complaining about.

What's the point of the BuildRequires: openssh?  That just gives you ssh-keygen
and some documentation; did you mean to BR: openssh-clients instead?  Or,
perhaps, did you mean to Requires: openssh-clients?  Currently there's no SSH
dependency in the final package.

* source files match upstream:
   7fa71b86969d8d695c3b062780a5694e  gstm-1.2.tar.gz
* package meets naming and packaging guidelines.
O specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
? BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* debuginfo package looks complete.
X rpmlint is silent.
? final provides and requires are sane:
   gstm = 1.2-1.fc6
  =
   libICE.so.6()(64bit)
   libORBit-2.so.0()(64bit)
   libSM.so.6()(64bit)
   libX11.so.6()(64bit)
   libart_lgpl_2.so.2()(64bit)
   libatk-1.0.so.0()(64bit)
   libbonobo-2.so.0()(64bit)
   libbonobo-activation.so.4()(64bit)
   libbonoboui-2.so.0()(64bit)
   libcairo.so.2()(64bit)
   libgconf-2.so.4()(64bit)
   libgdk-x11-2.0.so.0()(64bit)
   libgdk_pixbuf-2.0.so.0()(64bit)
   libglib-2.0.so.0()(64bit)
   libgmodule-2.0.so.0()(64bit)
   libgnome-2.so.0()(64bit)
   libgnome-keyring.so.0()(64bit)
   libgnomecanvas-2.so.0()(64bit)
   libgnomeui-2.so.0()(64bit)
   libgnomevfs-2.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgthread-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libpango-1.0.so.0()(64bit)
   libpangocairo-1.0.so.0()(64bit)
   libpangoft2-1.0.so.0()(64bit)
   libpopt.so.0()(64bit)
   libxml2.so.2()(64bit)
   libz.so.1()(64bit)
* %check is not present; no test suite upstream.
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* GUI app; desktop file looks OK and is installed properly.
Comment 6 Damien Durand 2006-08-12 04:15:32 EDT
Well I removed openssh in br :)
New Spec : http://glive.tuxfamily.org/fedora/gstm/gstm.spec
Nex SRPM : http://glive.tuxfamily.org/fedora/gstm/gstm-1.2-2.src.rpm
Comment 7 Jason Tibbitts 2006-08-14 21:44:09 EDT
For some reason several things have stopped building for me in rawhide,
including this package.  It still builds fine in FC5, though.

OK, so you removed the BR: openssh-client, but you seem to have neglected the
other comments I made.  The mixed-spaces-and-tabs thing isn't really a blocker
but it's still there and it would be good if you fixed it.  More troubling is
that there's still no dependency on openssh-clients.  At minimum you need to
require /usr/bin/ssh because gstm will call that directly.  (Actually it calls
"ssh" and relies on PATH to find it.)
Comment 8 Laurent Rineau 2006-08-15 06:26:41 EDT
** gstm-1.2-2.src.rpm builds and runs fine on my x86_64 machine under FC-5.

** rpmlint produces:
W: gstm mixed-use-of-spaces-and-tabs

** However, I think that %{_bindir}/gaskpass should go into a separate 
package.
Comment 9 Laurent Rineau 2006-08-15 06:40:38 EDT
(In reply to comment #5)
> * doesn't own any directories it shouldn't.

Actually, it does: in %files, "%{_datadir}/pixmaps" should 
be "%{_datadir}/pixmaps/*". This is a MUST.
Comment 10 Laurent Rineau 2006-08-15 06:42:56 EDT
(Sorry for the spam.) Another comment: the %description should preserve the 
case chosen upstream: "gSTM" instead of "Gstm".
Comment 11 Jason Tibbitts 2006-08-15 09:15:27 EDT
(In reply to comment #9)
> Actually, it does: in %files, "%{_datadir}/pixmaps" should 
> be "%{_datadir}/pixmaps/*". This is a MUST.

Indeed.  In FC5 is it very common for gnome packages to own this directory; 40
packages provide it.  When I looked at the list of packages I seem to have
missed the fact that "filesystem" is one of those packages, and so you must not
own it.
Comment 12 Damien Durand 2006-08-19 18:01:51 EDT
Added openssh-clients and repackaged.

SRPM : http://glive.tuxfamily.org/fedora/gstm/gstm-1.2-3.src.rpm
SPEC : http://glive.tuxfamily.org/fedora/gstm/gstm.spec
Comment 13 Jason Tibbitts 2006-08-21 21:52:37 EDT
I think there must be some sort of communication problem here.  The way this
works is that reviewers make comments and point out things that need to be
changed.  You should either make the indicated changes or discuss why you think
those changes shouldn't be made.  Instead, you seem to be ignoring most of the
comments.

The following issues have not been addressed:
  Still owns %{_datadir}/pixmaps.
  %description still says Gstm instead of gSTM.

In addition, could you also comment on Laurent's comment that gaskpass should be
in a separate package?  (I admit to not understanding the issue here; perhaps
Laurent could elaborate.)
Comment 14 Laurent Rineau 2006-08-22 05:28:11 EDT
(In reply to comment #13)
> In addition, could you also comment on Laurent's comment that gaskpass 
should be
> in a separate package?  (I admit to not understanding the issue here; 
perhaps
> Laurent could elaborate.)

This is not an "issue". gAskpass could be totally independent from gSTM. I 
think that it has an interest outside of gSTM: one could like to install 
gAskpass without gSTM.
Comment 15 Laurent Rineau 2006-08-22 05:30:23 EDT
Created attachment 134623 [details]
Patch that create a gaskpass package
Comment 16 Damien Durand 2006-08-26 14:02:06 EDT
Thanks for your patch Laurent, it's added to the spec file.

Spec : http://glive.tuxfamily.org/fedora/gstm/gstm.spec
SPRM : http://glive.tuxfamily.org/fedora/gstm/gstm-1.2-4.fc6.src.rpm
Comment 17 Jason Tibbitts 2006-08-28 12:05:14 EDT
Still owns %datadir/pixmaps.
%description still says Gstm instead of gSTM.
Comment 18 Laurent Rineau 2006-09-05 07:21:01 EDT
Damien, could you remove gstm from owners.list, until this package is fully 
accepted (maybe soon)? gstm has been reported bogus in the last "FE package 
status" mail.
Comment 19 Damien Durand 2006-09-05 10:02:35 EDT
Sorry, I've been busy a while. You'll find a new spec and srpm where
%description and %datadir/pixmaps have been fixed.

spec: http://glive.tuxfamily.org/fedora/gstm/gstm.spec
srpm: http://glive.tuxfamily.org/fedora/gstm/gstm-1.2-5.fc6.src.rpm
Comment 20 Jason Tibbitts 2006-09-06 13:25:43 EDT
Indeed, those two issues have been fixed.  Three rpmlint issues have cropped up,
but they're not blockers:

W: gstm macro-in-%changelog description
  You need to double every percent symbol in your changelog.

W: gstm mixed-use-of-spaces-and-tabs
  I believe this warning is completely bogus in this situation.

W: gaskpass no-documentation
  Indeed, there isn't any, but this isn't really an issue as far as I can see.

The issues that were raised have been addressed.

APPROVED
Comment 21 Christian Iseli 2007-01-02 19:37:22 EST
Changed summary for tracking purposes.

Note You need to log in before you can comment on or make changes to this bug.