Bug 199173 - Review Request: clusterssh
Review Request: clusterssh
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-07-17 14:28 EDT by Duncan Ferguson
Modified: 2015-05-26 15:13 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-30 13:42:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+


Attachments (Terms of Use)
Twealked spec file (3.10 KB, text/plain)
2006-08-16 05:34 EDT, Paul Howarth
no flags Details

  None (edit)
Description Duncan Ferguson 2006-07-17 14:28:34 EDT
Spec URL: http://queeg.dyndns.org/clusterssh/clusterssh.spec
SRPM URL: http://queeg.dyndns.org/clusterssh/clusterssh-3.18.2.10-1.src.rpm
Description: Control multiple terminals open on different servers to perform administration tasks, for example multiple hosts requiring the same config within a cluster.  Not limited to use with clusters, however.

Note this is not the final version of the code, but a precusor to get all changes required for fedora in and accepted before being committed and publically released.
Comment 1 Devrim GUNDUZ 2006-07-18 12:20:37 EDT
There is a problem with Version and URL. There is no such version at project
page.   What is on the project page is 3.18.1. So:

Version:       3.8.1
Release:       1%{dist}

should be better; so that it will reflect the upstream version.

Also,the download URL should be:
http://osdn.dl.sourceforge.net/sourceforge/%{name}/%{name}-%{version}.tar.gz

BTW, rpmlint does not report any error.
Comment 2 Duncan Ferguson 2006-07-18 14:29:11 EDT
As mentioned in the initial request, I have raised this before commiting branch
changes to the project source.  After all issues with the spec file (and desktop
file and icons isnall etc) are fixed (apart from the version matching the
upstream version), then I will commit project code and release properly.  I
didnt want to field lots of problems with the project just after releasing a new
version (the SRPM contains the correct tar.gz file).

The Source0 download URL has been fixed, and new spec file and SRPM generated
and are available as per the initial request.

Thanks
Comment 3 Duncan Ferguson 2006-07-21 14:26:29 EDT
Spec URL: http://queeg.dyndns.org/clusterssh/clusterssh.spec
SRPM URL: http://queeg.dyndns.org/clusterssh/clusterssh-3.18.2.10-2.src.rpm

Should have reported the correct SRPM path before for the changes....

Any other stoppers before I commit projects changes?

Thanks
Comment 4 Devrim GUNDUZ 2006-07-23 16:32:31 EDT
Hi again,

There is an rpmlint warning from srpm:
W: clusterssh strange-permission clusterssh.spec 0600
A file that you listed to include in your package has strange
permissions. Usually, a file should have 0644 permissions.

I think you will need to repackage SRPM after changing the permission of spec file.

Except rebuilding in mock that I did not test, per review guidelines, spec and
srpm looks good. I built it on FC5 and used ;)

Regards, Devrim
Comment 5 Devrim GUNDUZ 2006-07-23 18:48:10 EDT
Ok, it builds cleanly in mock.
Comment 6 Duncan Ferguson 2006-07-24 02:36:26 EDT
Thanks - I'll double check the perms and then start on project commits.  Should
have the "real" version of the rpm within a few days.
Comment 7 Duncan Ferguson 2006-07-25 14:23:19 EDT
Spec URL: http://queeg.dyndns.org/clusterssh/clusterssh.spec
SRPM URL: http://queeg.dyndns.org/clusterssh/clusterssh-3.19.1-1.src.rpm

This is the real version i would like to put into extras.  There should
(hopefully) be no errors or issues with it at all (famous last words...)

Thanks

  Duncs
Comment 8 Kevin Fenzi 2006-08-12 13:44:57 EDT
Greetings. Here's a review:

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
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:
578208faf944100a233ae216fa230350  clusterssh-3.19.1.tar.gz
578208faf944100a233ae216fa230350  clusterssh-3.19.1.tar.gz.1
OK - Package compiles and builds on at least one arch.
n/a - Package needs ExcludeArch
OK - BuildRequires correct
n/a - Spec handles locales/find_lang
n/a - Spec has needed ldconfig in post and postun
n/a - Package is relocatable and has a reason to be.
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
n/a - -doc subpackage needed/used.
OK - Packages %doc files don't affect runtime.
n/a - Headers/static libs in -devel subpackage.
n/a - .pc files in -devel subpackage.
n/a - .so files in -devel subpackage.
n/a - -devel package Requires: %{name} = %{version}-%{release}
n/a - .la files are removed.
See below - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock. (fc6/i386)
OK - Should function as described.
OK - Should have sane scriptlets.

Issues:

1. The Source0 line doesn't work for me. I guess the osdl mirror
doesn't have a copy of this source.
http://easynews.dl.sourceforge.net/sourceforge/clusterssh/clusterssh-
3.19.1.tar.gz
worked ok for me.

2. You need to run desktop-file-install in %install for your .desktop
file. See:
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

3. Might include the "THANKS" file as a %doc?

4. Why do you have the:
Requires:      perl-Tk perl-X11-Protocol
rpm picks up those requirements just fine without help. You should remove
those Requires unless there is some reason I don't see why they are there. 
Comment 9 Duncan Ferguson 2006-08-15 05:26:44 EDT
A question on point 2 - the "desktop-file-install".  I have included 

Source1: %{name}.desktop

and

%install
desktop-file-install --vendor %{desktop_vendor}                 \
        --dir ${RPM_BUILD_ROOT}%{_datadir}/applications         \
        --add-category X-Fedora                                 \
        %{SOURCE1}

but when i try to build it i get an error "cannot find clusterssh.spec".  It
seems as though it is not looking in the right place to find it.  How should I
specify the Source1:?

Thanks
Comment 10 Kevin Fenzi 2006-08-15 22:31:25 EDT
Odd. Can you upload your current spec and desktop file?

what is %{desktop_vendor} set to?

%{SOURCE1} should be right for Source1. 
Comment 11 Duncan Ferguson 2006-08-16 03:23:51 EDT
I have put 3 files onto

http://queeg.dyndns.org/clusterssh/

clusterssh.spec
clusterssh.desktop
clusterssh.rpmbuild - contains all the output from me trying to run
$ rpmbuild -ta SOURCES/clusterssh-3.19.1.tar.gz 

This process was working until I attempted to add the "desktop-file-install". 
From what I was able to find out, "%(desktop_vendor}" was a more neutral way of
setting the vendor to "fedora", but in the files above it is expliclty set to
"fedora" (which makes no difference).

Thanks
Comment 12 Duncan Ferguson 2006-08-16 03:25:11 EDT
Oh, and in "playing" I have set replaced SOURCE1 with %{name}.desktop, but that
made no difference either.
Comment 13 Paul Howarth 2006-08-16 05:34:27 EDT
Created attachment 134287 [details]
Twealked spec file

The attached spec file works for me, both for mock builds of a regular SRPM,
and when inserted into the tarball, using "rpmbuild -ta tarball"
Comment 14 Kevin Fenzi 2006-08-16 13:33:47 EDT
Thanks Paul.

If you will look at Paul's working spec file you can see the issue...
With desktop-file-install it adds the vendor to the filename, so your 
files section needs to change from: 
%{_datadir}/applications/clusterssh.desktop
to
%{_datadir}/applications/fedora-clusterssh.desktop

Comment 15 Duncan Ferguson 2006-08-16 13:50:24 EDT
Thanks for your help

Spec URL: http://queeg.dyndns.org/clusterssh/clusterssh.spec
SRPM URL: http://queeg.dyndns.org/clusterssh/clusterssh-3.19.1-1.src.rpm
Comment 16 Kevin Fenzi 2006-08-16 18:16:29 EDT
All the outstanding issues from comment #8 appear to be fixed, everything looks 
good to me, so this package is APPROVED. 

Don't forget to close this bug with NEXTRELEASE once this package is imported 
and built for devel. 
Comment 17 Kevin Fenzi 2006-08-27 14:46:40 EDT
Ping Duncan. Do you still intend to import and maintain this package? 
It should be all approved and ready to import...
Comment 18 Duncan Ferguson 2006-08-27 17:49:01 EDT
I have every intention of sorting it out.  Time, however, is a little lacking
currently.

  Duncs
Comment 19 Kevin Fenzi 2006-08-27 18:03:16 EDT
Excellent. I just wanted to make sure you didn't miss that it had been 
approved. :) If you require any assistance at all, feel free to drop me a 
email or catch me on irc. 
Comment 20 Mark Chappell 2010-03-23 06:29:25 EDT
Have tried to contact the Fedora maintainer with no response:

Initially by email (4 Feb) then through bugzilla: 
https://bugzilla.redhat.com/show_bug.cgi?id=574430

Package Change Request
======================
Package Name: clusterssh
New Branches: EL-5
Owners: tremble
Comment 21 Kevin Fenzi 2010-03-23 23:16:12 EDT
cvs done.
Comment 22 Mark Chappell 2015-05-26 03:36:05 EDT
Package Change Request
======================
Package Name: clusterssh
New Branches: epel7
Owners: tremble filiperosset
Comment 23 Gwyn Ciesla 2015-05-26 15:13:18 EDT
Git done (by process-git-requests).

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