https://bugzilla.redhat.com/show_bug.cgi?id=915864 --- Comment #7 from Steven Dake <sdake@xxxxxxxxxx> --- Gang Wei, Typically it is better (tips for next time) to keep each version of the RPM and update the spec file with version numbers so diffs can be done against the spec files. As you update the spec file, bump the Release number and update the changelog as well. The spec files look far improved from what I recall, however, I notice the following issues that need resolution before a formal review. (The formal review takes awhile, so I prefer to get all the obvious issues out of the way). Is it necessary to have a separate version # and dist for each package? I can see this causing all kinds of confusion and it is probably a violation of the packaging guidelines, but I can't find the specific item on this point. Maybe your the first one that has tried it. If you don't absolutely need it, I wouldn't do it. eg: %package appraiser Summary: Appraisal Server for OpenAttestation Version: 1.6.0 Release: 1%{?dist} %package client Summary: Host Agent for OpenAttestation Version: 1.6.0 Release: 1%{?dist} In fedora packages there is no need for a clean section. It is safe to remove %clean rm -rf %{buildroot} The scriplets should not be removing files managed by rpm - specifically: %preun appraiser if [ -d %{_sharedstatedir}/oat-appraiser/ ]; then rm -rf %{_sharedstatedir}/oat-appraiser/ fi Or databases: if [ -e %{_datadir}/oat-appraiser/oat-db-drop.sh ]; then bash %{_datadir}/oat-appraiser/oat-db-drop.sh fi Or config files: if [ -d %{_datadir}/oat-appraiser/ ]; then rm -rf %{_datadir}/oat-appraiser/ fi This looks suspicious as well, but I'm not sure exactly how the package is supposed to behave: if [ -d %{_datadir}/java/oat ]; then rm -rf %{_datadir}/java/oat fi This is wrong: if [ -e %{_sysconfdir}/systemd/system/oat-client.service ]; then systemctl stop oat-client.service rm -f %{_sysconfdir}/systemd/system/oat-client.service fi A far superior way to do the systemd magic is to use the scripts built for it. For examples, take a look at: https://raw.github.com/sdake/fedora-reviews/master/openstack-heat/openstack-heat.spec deleting files managed by rpm: if [ -d %{_sysconfdir}/oat-client/ ]; then rm -rf %{_sysconfdir}/oat-client/ fi All of these deletes should be managed by RPM. I didn't build the package and will need a rawhide install to do so but nothing should be installing files in /: if [ -d /OAT/ ]; then rm -rf /OAT/ fi Is there a strong rationale for having the build.sh script not just be part of the %build section? I'd prefer an unwieldy build section that is maintainable to an upstream build script that could change. Seems like opportunity for breakage later if the upstream build.sh script changes in some way that is incompatible with RPM. Since there are new deps added, I will have to install rawhide in a VM. I'll work on that today but I am having technical issues with our networks and unable to download large files. I'll sort that out, but it may cause delay. Regards -steve -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Rktq9ZsEtE&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review