Bug 226218

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.  

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.

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]$                                                        
          

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)

This might be a good time to consider BZ #61198 again.

Looks good now.  I like the spec file cleanup. 

APPROVED.