Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226218 - Merge Review: openssh
Summary: Merge Review: openssh
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tomas Mraz
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:19 UTC by Nobody's working on this, feel free to take it
Modified: 2008-04-03 16:04 UTC (History)
3 users (show)

Fixed In Version: openssh-4.5p1-5.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-04-03 16:04:28 UTC
mastahnke: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 20:19:07 UTC
Fedora Merge Review: openssh

http://cvs.fedora.redhat.com/viewcvs/devel/openssh/
Initial Owner: tmraz@redhat.com

Comment 1 Michael Stahnke 2007-02-15 06:37:15 UTC
Template I am using for review -- thanks KevinFenzi

 OK - Package meets naming and packaging guidelines
 OK - Spec file matches base package name.
 OK - Spec has consistant macro usage.
 OK - Meets Packaging Guidelines.
 OK - License
 OK - License field in spec matches
 OK - License file included in package
 OK - Spec in American English
 OK - Spec is legible.
 XX - Sources match upstream md5sum --sources do not match.  Explanation is
provided and acceptable, IMO. 
 OK - BuildRequires correct
 OK - Spec handles locales/find_lang
 OK - Package has %defattr and permissions on files is good.
 OK - Package has a correct %clean section.
 XX Package has correct buildroot  -- package has incorrect build root.  See
FESCO meeting minutes from this week or last.
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 OK - Package is code or permissible content.
 OK - Packages %doc files don't affect runtime.
 OK - Package compiles and builds on at least one arch.
 XX - Package has no duplicate files in %files.  -- /etc/ssh is provided by
openssh-server and openssh-clients
 OK - Package doesn't own any directories other packages own.
 OK - Package owns all the directories it creates.
 XX (see below) - No rpmlint output.

SHOULD Items:

 OK - Should build in mock.
 OK - Should function as described.
 OK - Should have sane scriptlets.
 OK - Should have subpackages require base package with fully versioned depend.
 OK - Should have dist tag
 OK - Should package latest version
 XX- check for outstanding bugs on package. (For core merge reviews)  Several
other bugs exists, however most appear to be RFEs for items not seen in
upstream, so I don't consider them problems for merge.  (and wow some of those
RFEs don't look fun)



Most of the rpmlint output makes perfect sense to me, but I am not
all-authoratative. My biggest concern is no documentation for openssh-askpass

rpmlint output for binary packagees
openssh-4.5p1-2.i386.rpm
E: openssh setuid-binary /usr/libexec/openssh/ssh-keysign root 04755
E: openssh non-standard-executable-perm /usr/libexec/openssh/ssh-keysign 04755
E: openssh non-readable /etc/ssh/moduli 0600
openssh-askpass-4.5p1-2.i386.rpm
W: openssh-askpass conffile-without-noreplace-flag
/etc/profile.d/gnome-ssh-askpass.csh
W: openssh-askpass conffile-without-noreplace-flag
/etc/profile.d/gnome-ssh-askpass.sh
W: openssh-askpass no-documentation
E: openssh-askpass executable-marked-as-config-file
/etc/profile.d/gnome-ssh-askpass.sh
E: openssh-askpass executable-sourced-script /etc/profile.d/gnome-ssh-askpass.sh
0755
E: openssh-askpass executable-marked-as-config-file
/etc/profile.d/gnome-ssh-askpass.csh
E: openssh-askpass executable-sourced-script
/etc/profile.d/gnome-ssh-askpass.csh 0755
openssh-clients-4.5p1-2.i386.rpm
E: openssh-clients setgid-binary /usr/bin/ssh-agent nobody 02755
E: openssh-clients non-standard-executable-perm /usr/bin/ssh-agent 02755
openssh-debuginfo-4.5p1-2.i386.rpm
openssh-server-4.5p1-2.i386.rpm
E: openssh-server non-standard-dir-perm /var/empty/sshd 0711
E: openssh-server non-readable /etc/ssh/sshd_config 0600
W: openssh-server non-standard-dir-in-var empty
W: openssh-server dangerous-command-in-%trigger rm
W: openssh-server service-default-enabled /etc/rc.d/init.d/sshd
W: openssh-server incoherent-init-script-name sshd

Please fix space/tab issue
Perms on openssh-nukeacss.sh could also probably be changed. 
rpmlint openssh-4.5p1-2.src.rpm
W: openssh strange-permission openssh-nukeacss.sh 0775
W: openssh unversioned-explicit-obsoletes openssh-askpass-gnome
W: openssh unversioned-explicit-provides openssh-askpass-gnome
W: openssh mixed-use-of-spaces-and-tabs (spaces: line 10, tab: line 239)


Note:  I am not a member of FedoraBugs yet (still waiting) so I can't claim the
ticket and pass it on.  

Comment 2 Tomas Mraz 2007-02-22 13:05:37 UTC
I've fixed the following in openssh-4.5p1-3.fc7:
- Package has no duplicate files in %files.  -- /etc/ssh is provided by
openssh-server and openssh. (removed from openssh-server)
- Package has incorrect build root. (replaced with the standard one)

The gnome-ssh-askpass is really simple thing which works out of the box so I
don't think doc is necessary for it. The perms on openssh-nukeacss.sh in srpm
would have to be fixed manually in the CVS repository. The 'mixed use of spaces
and tabs' is a nonsense as both are used in completely different places for
different purposes.


Comment 3 Michael Stahnke 2007-03-04 05:15:29 UTC
According to http://fedoraproject.org/wiki/Packaging/SourceURL , it looks like
we need just a tiny bit more detail on howto generate the source file used for
this package form upstream.  I assume that you just download upstream's version
and run  openssh-nukeacss.sh, is there anything more to it than that?  Even if
that is all, please place that comment in the spec. 

I am trying to find the right way to approcah the /etc/profile.d/ files.  They
are quite similar to init scripts in that many people think they shouldn't be
edited, if so, they don't have to be %config.  If they are %config they should
probably be noreplace to preserve edits. 

Also, if they are only sourced and not executed, could they be 644 permissions
rather than 755?

[builder@rawhide i386]$ rpmlint openssh-askpass-4.5p1-4.i386.rpm
W: openssh-askpass conffile-without-noreplace-flag
/etc/profile.d/gnome-ssh-askpass.csh
W: openssh-askpass conffile-without-noreplace-flag
/etc/profile.d/gnome-ssh-askpass.sh
W: openssh-askpass no-documentation
E: openssh-askpass executable-marked-as-config-file
/etc/profile.d/gnome-ssh-askpass.sh
E: openssh-askpass executable-sourced-script /etc/profile.d/gnome-ssh-askpass.sh
0755
E: openssh-askpass executable-marked-as-config-file
/etc/profile.d/gnome-ssh-askpass.csh
E: openssh-askpass executable-sourced-script
/etc/profile.d/gnome-ssh-askpass.csh 0755
[builder@rawhide i386]$                                                        
          

Comment 4 Tomas Mraz 2007-03-19 11:59:59 UTC
I improved the source tarball comment as requested. I also changed
profile.d/gnome-ssh-askpass.* to be regular files (644 permission, not %config).

(openssh-4.5p1-5.fc7.src.rpm)


Comment 5 Jonathan Underwood 2007-03-30 22:47:34 UTC
This might be a good time to consider BZ #61198 again.

Comment 6 Michael Stahnke 2007-07-01 02:23:16 UTC
Looks good now.  I like the spec file cleanup. 

APPROVED.




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