Bug 226519

Summary: Merge Review: usermode
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mbacovsk, mitr, redhat-bugzilla
Target Milestone: ---Flags: j: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-01-15 23:02:43 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 426387    

Description Nobody's working on this, feel free to take it 2007-01-31 21:14:10 UTC
Fedora Merge Review: usermode

http://cvs.fedora.redhat.com/viewcvs/devel/usermode/
Initial Owner: mbacovsk

Comment 1 Jason Tibbitts 2007-12-22 03:26:24 UTC
I'll take a look at this.  Note that I'm happy to provide a patch fixing the issues I know how to fix, or make the changed directly in CVS if you prefer.

Does this package have an upstream?  If so, a URL tag is needed and if not, the spec needs a comment to that effect: http://fedoraproject.org/wiki/Packaging/SourceURL

No need for the SysVinit conflict; even RH9 has SysVinit 2.84.

Similarly, you can drop some of the versions from the dependencies, as we don't support any releases with such old versions of libselinux-devel or pam.

Is the WITH_SELINUX stuff still necessary these days?  No problem if it's still needed, but it might be a bit cleaner to use the %bcond_without macro.

The copying file must be include in the package as %doc.

Why does this have a direct dependency on /etc/pam.d/system-auth?  It's
been provided by the pam package as far back as the oldest machine I can
access (RH 7.2).

Some rpmlint complaints:

  usermode.x86_64: E: setuid-binary /usr/sbin/userhelper root 04711
  usermode.x86_64: E: non-standard-executable-perm /usr/sbin/userhelper 04711
  usermode.x86_64: E: non-standard-executable-perm /usr/sbin/userhelper 04711
These are expected

  usermode.x86_64: W: no-url-tag
  usermode-gtk.x86_64: W: no-url-tag
If there's an upstream web page, it should be indicated

  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/halt
  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/poweroff
  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/reboot
I'm not sure what to do with these.  If they're really configuration files
then they need to nave %noreplace so updates don't overwrite local changes.

Checklist:
? don't know if there's an upstream to compare against.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
X build root is not correct.
* license field matches the actual license.
* license is open source-compatible.
* license text is in the tarball but not in the package.
? can't tell latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has valid complaints.
? final provides and requires:
  usermode-1.93.1-1.fc9.x86_64.rpm
   config(usermode) = 1.93.1-1.fc9
   usermode = 1.93.1-1.fc9
  =
?  /etc/pam.d/system-auth
   config(usermode) = 1.93.1-1.fc9
   libglib-2.0.so.0()(64bit)
   libgmodule-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libpam.so.0()(64bit)
   libpam.so.0(LIBPAM_1.0)(64bit)
   libpam_misc.so.0()(64bit)
   libpam_misc.so.0(LIBPAM_MISC_1.0)(64bit)
   libselinux.so.1()(64bit)
   libuser.so.1()(64bit)
   pam >= 0.75-37
   passwd
   util-linux

  usermode-gtk-1.93.1-1.fc9.x86_64.rpm
   usermode-gtk = 1.93.1-1.fc9
  =
   libICE.so.6()(64bit)
   libSM.so.6()(64bit)
   libatk-1.0.so.0()(64bit)
   libblkid.so.1()(64bit)
   libcairo.so.2()(64bit)
   libgdk-x11-2.0.so.0()(64bit)
   libgdk_pixbuf-2.0.so.0()(64bit)
   libglade-2.0.so.0()(64bit)
   libglib-2.0.so.0()(64bit)
   libgmodule-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libpango-1.0.so.0()(64bit)
   libpangocairo-1.0.so.0()(64bit)
   libstartup-notification-1.so.0()(64bit)
   libxml2.so.2()(64bit)
   usermode = 1.93.1-1.fc9

* %check is not present, automated testing not possible.
* no shared libraries are added to the regular linker search paths.
* 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 static libraries.
* no libtool .la files.
* even though there are GUI applications here, there's no point in having 
   desktop files as the graphical bits aren't called directly.


Comment 2 Jason Tibbitts 2009-01-13 23:08:05 UTC
It's been quite some time since my previous comments, and some of it seems to be out of date when compared to the current state of the package.

There's now an upstream URL and released source tarball.
The WITH_SELINUX stuff is gone now.
COPYING is now included.

Most of the above rpmlint stuff is still there.  Additionally there are some bogus "no-dependency-on" complaints about the debuginfo package, which might be a bug in rpm but isn't an issue with this package.

So we're left with:

No need for the SysVinit conflict; even RH9 has SysVinit 2.84.

Similarly, you can drop some of the versions from the dependencies, as we don't
support any releases with such old versions of libselinux-devel or pam.

Why does this have a direct dependency on /etc/pam.d/system-auth?  It's
been provided by the pam package as far back as the oldest machine I can
access (RH 7.2).

  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/halt
  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/poweroff
  usermode.x86_64: W: conffile-without-noreplace-flag
   /etc/security/console.apps/reboot
I'm not sure what to do with these.  If they're really configuration files
then they need to nave %noreplace so updates don't overwrite local changes.

Comment 3 Jason Tibbitts 2009-01-14 01:40:14 UTC
Sorry, one more:

The build root is not correct.  It needs to reference at least %name, %version and %release.  Suggested values are at http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

Comment 4 Miloslav Trmač 2009-01-14 12:10:34 UTC
Thanks for the review, everything should be fixed in rawhide usermode-1.99-2.

Comment 5 Jason Tibbitts 2009-01-15 23:02:43 UTC
Indeed, that looks good.  Thanks for your time.

APPROVED