Fedora Merge Review: usermode http://cvs.fedora.redhat.com/viewcvs/devel/usermode/ Initial Owner: mbacovsk
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.
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.
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
Thanks for the review, everything should be fixed in rawhide usermode-1.99-2.
Indeed, that looks good. Thanks for your time. APPROVED