Red Hat Bugzilla – Bug 225792
Merge Review: gfs2-utils
Last modified: 2008-04-30 11:22:00 EDT
Fedora Merge Review: gfs2-utils http://cvs.fedora.redhat.com/viewcvs/devel/gfs2-utils/ Initial Owner: cfeist@redhat.com
What do we need to do in order to resolve this?
Josef, should we be doing something for this bz? Its been kicking about for a while and I don't know what we need to do to resolve it? Can we just close it or do we need to take some action first?
Is gfs2-utils in fedora? I don't remember why I have this b/c IIRC its already there. If its not then it needs to be assigned to somebody who reviews packages for inclusion into fedora, as its been quite a while since i looked at/cared about fedoras packaging guidlines.
Looks like it's already in. I'm closing this bug as CURRENTRELEASE.
No, all packages that were in 'core' before the merge need to be reviewed and make sure they meet all the guidelines, etc... So, someone needs to review this and confirm it's all good before we can close this.
then somebody else needs to be assigned to this, like i said before i havent done a package review in a while.
How do we get a reviewer assigned? According to the blurb on the Fedora site, it seems that its supposed to happen automatically, but thus far nobody has taken it on and its been open for over a year now.
Well, there are currently 745 pending Reviews in the NEW state: http://fedoraproject.org/PackageReviewStatus/NEW.html And a small handful of reviewers slowly processing them down... You could offer to review someone elses packages and have them review yours. You could bribe someone to review it. ;) Note that if you want it to be in the NEW pile for people to see, you should assign it to nobody@fedoraproject.org...
I'd seriously consider bribery if only I could find a list somewhere of potential bribees :-) Do you know if there is such a list? Again the wiki is silent on the matter.
Yeah, anyone who is a current maintainer... ie, everyone in the 'cvsextras' group. See: https://admin.fedoraproject.org/accounts/group/view/cvsextras However, I am in a reviewing mood... so I will go ahead and do a review for you... :) Look for a full review in a bit here.
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 (GPLv2) See below - License field in spec matches See below - License file included in package OK - Spec in American English OK - Spec is legible. See below - Sources match upstream md5sum: OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have dist tag See below - check for outstanding bugs on package. Issues: 1. The License tag is not right. Should be "GPLv2" 2. You might ask upstream to include a copy of the GPL with the package. Not a blocker. 3. Is there an upstream download URL you can place in the Source0 line? Or can you comment where to get the source? See: http://fedoraproject.org/wiki/Packaging/SourceURL 4. The URL seems to no longer be correct. That redirects me to: http://sources.redhat.com/cluster/wiki 5. Does parallel make not work with this package? If it does, please change the 'make' to 'make %{?_smp_mflags}' If not, please add a note that it doesn't work. 6. You might add: Requires(preun): /sbin/service and stop the service in preun? See: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-a6d7a1ed9d77dbb8d4af067378a79b838aebb20a 7. rpmlint says: gfs2-utils.src: W: invalid-license GPL gfs2-utils.x86_64: W: invalid-license GPL gfs2-utils-debuginfo.x86_64: W: invalid-license GPL These will be fixed by changing to 'GPLv2' gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_convert.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_fsck.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_mount.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_tool.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/mkfs.gfs2.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_grow.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_quota.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2_jadd.8.gz gfs2-utils.x86_64: W: spurious-executable-perm /usr/share/man/man8/gfs2.8.gz For some reason your man pages are not the right mode. Can you file a bug upstream to install them as 644? gfs2-utils.x86_64: E: non-standard-executable-perm /sbin/gfs2_jadd 0775 Why 775 here and not 755? gfs2-utils.x86_64: E: missing-mandatory-lsb-keyword Description in /etc/rc.d/init.d/gfs2 gfs2-utils.x86_64: E: missing-mandatory-lsb-keyword Short-Description in /etc/rc.d/init.d/gfs2 Might fix up the init script to have the lsb keywords in it. gfs2-utils.x86_64: W: incoherent-init-script-name gfs2 This is due to the init script not being the same name as the package. Ignore, unless you want to re-name it to 'gfs2-utils' instead? 8. Looking at the outstanding bugs against this package I see two: 224154 - Should just be closed now? 429769 - Can this be closed now? Is rawhide up to date? If you could address all the above, then let me know I will re-run my checks and we can get this closed out. Let me know if there are any questions.
So far... 1. I've fixed the license in F-8, and I'll see if we can get the right things added to the upstream source 3. I'll try and sort that out shortly 4. Fixed in F-8, will move changes to other versions shortly once we've worked through all the issues in F-8 5. probably. I need to check that one 6. I'll look into that too 7. license fixed, man page perms are fixed in upstream git, but not yet in the release tar ball. I'll try and get another release done. 8. There are more than just the two you've spotted, most of them depend on getting changes that are already done in upstream into the rpm. I've just added two more for build warnings in fact that we ought not to have. Kevin, don't rerun your stuff quite yet, but please do take this as an indication that we are working on the issues at the moment and it shouldn't be too long before we are done.
Excellent. Just let me know when you would like me to recheck things. Thanks for looking at it.
Ok. I think we've sorted out all of the above now. Please take a look and let us know if you can still see anything missing. bz #224154 and #433592 are fixed by the latest rawhide build. I'm not sure if I need to do anything to push that out like I did for F-8? make update doesn't seem to work for devel. I marked the bugs modified in the mean time.
Also updated now in rawhide/F-9. Currently all three are identical to each other.
Sorry for the delay here... ;( 1-5 look good. 6. You aren't stopping the service in preun, you might want to do that? 7. rpmlint now looks much better: gfs2-utils.x86_64: W: service-default-enabled /etc/init.d/gfs2 gfs2-utils.x86_64: E: missing-mandatory-lsb-keyword Description in /etc/init.d/gfs2 gfs2-utils.x86_64: E: missing-mandatory-lsb-keyword Short-Description in /etc/init.d/gfs2 gfs2-utils.x86_64: W: incoherent-init-script-name gfs2 So, I don't see any further blocker issues here. You might look at item 6 again, but otherwise things look good. This package is APPROVED. Feel free to close this bug RAWHIDE.
I added the Requires(preun): /sbin/service line as you'd suggested so what else do I need to do in order to satisfy item 6?
Simply make your preun: %preun if [ $1 = 0 ]; then /sbin/service gfs2 stop >/dev/null 2>&1 /sbin/chkconfig --del <script> fi Ie, stop the service before removing the init script and package...
Ah, I see now. I'll add that on the next update. Thanks.
I've added the line to preun as suggested above in the rawhide package. Once f-9 is out properly, I'll add it to that as well on the next update. So I'm now going to close this as suggested in comment #16.