This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 226218 - Merge Review: openssh
Merge Review: openssh
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 15:19 EST by Nobody's working on this, feel free to take it
Modified: 2008-04-03 12:04 EDT (History)
3 users (show)

See Also:
Fixed In Version: openssh-4.5p1-5.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-04-03 12:04:28 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mastahnke: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:19:07 EST
Fedora Merge Review: openssh

http://cvs.fedora.redhat.com/viewcvs/devel/openssh/
Initial Owner: tmraz@redhat.com
Comment 1 Michael Stahnke 2007-02-15 01:37:15 EST
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 08:05:37 EST
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 00:15:29 EST
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 07:59:59 EDT
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 18:47:34 EDT
This might be a good time to consider BZ #61198 again.
Comment 6 Michael Stahnke 2007-06-30 22:23:16 EDT
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.