Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #24 from Steven Dake <sd...@redhat.com> 2012-04-12 10:48:36 EDT --- 1) The BuildRoot is not necessary, Fedora figures this out automatically: BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) 2) This looks a little suspicious. Especially in FC17+ the gdm release shouldn't be el6? %define gdm_version gdm-2.30.4 %define gdm_release %{gdm_version}-14.el6 Are these still needed now that your using the gdm-devel package? # The following requirements were copied from the gdm.spec file. BuildRequires: pkgconfig(libcanberra-gtk) BuildRequires: scrollkeeper >= 0:%{scrollkeeper_version} BuildRequires: pango-devel >= 0:%{pango_version} BuildRequires: gtk2-devel >= 0:%{gtk2_version} BuildRequires: libglade2-devel >= 0:%{libglade2_version} BuildRequires: libgnomeui-devel >= 0:%{libgnomeui_version} BuildRequires: pam-devel >= 0:%{pam_version} BuildRequires: fontconfig >= 0:%{fontconfig_version} BuildRequires: desktop-file-utils >= %{desktop_file_utils_version} BuildRequires: gail-devel >= 0:%{gail_version} if they are still needed, please get rid of the defines: %define libauditver 1.0.6 %define pango_version 1.2.0 %define gtk2_version 2.6.0 %define libglade2_version 2.0.0 %define libgnomeui_version 2.2.0 %define scrollkeeper_version 0.3.4 %define pam_version 0.99.8.1-11 %define desktop_file_utils_version 0.2.90 %define gail_version 1.2.0 %define nss_version 3.11.1 %define fontconfig_version 2.6.0 and put them directly in the buildrequires. The current spec file is very difficult to read. 3) Make spec files fedora specific please otherwise the packaging is very difficult to read ie: %if 0%{?rhel} --with-gdm-src-dir=%{_topdir}/BUILD/%{gdm_version} \ --with-simple-greeter-plugins-dir=%{_libdir}/gdm/simple-greeter/plugins \ %endif %if 0%{?rhel} sed -i "s~parent->setObjectName(\"welcome\");~parent->setObjectName(\"talker\");~" kdm-plugin/src/kgreet_ovirtcred.cpp %endif just assume systemd will be used # Install systemd script. install -Dm 0644 ovirt-guest-agent/ovirt-guest-agent.service $RPM_BUILD_ROOT%{_unitdir}/ovirt-guest-agent.service %if 0%{?rhel} # No longer needed and is provided by the gdm package. rm -f $RPM_BUILD_ROOT%{_libdir}/libgdmsimplegreeter.so rm -f $RPM_BUILD_ROOT%{_libdir}/gdm/simple-greeter/plugins/ovirtcred.a rm -f $RPM_BUILD_ROOT%{_libdir}/gdm/simple-greeter/plugins/ovirtcred.la %else 4) /var/run is mounted dynamically as a tempfs. As a result, you will not want it in the package. If you need directories in /var/run, you will need to create them. I don't like it either if it makes you feel better ;) mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/run/ovirt-guest-agent 5) The clean section isn't needed in fedora since about 12ish or so. It can be removed. 6) please do not use static IDS (such as 175) in useradd. Also the proper thing is not being done here re handling useradd failures. See http://fedoraproject.org/wiki/Packaging:UsersAndGroups for the proper mechanism. After correcting the above, I'll go through an official review -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
