Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226453 - Merge Review: system-config-boot
Summary: Merge Review: system-config-boot
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:04 UTC by Nobody's working on this, feel free to take it
Modified: 2009-09-21 20:30 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-08-05 19:18:24 UTC
kevin: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 21:04:35 UTC
Fedora Merge Review: system-config-boot

http://cvs.fedora.redhat.com/viewcvs/devel/system-config-boot/
Initial Owner: harald@redhat.com

Comment 1 Kevin Fenzi 2007-03-14 05:16:48 UTC
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 (GPL)
OK - 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:
See below - Package needs ExcludeArch
OK - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.

See below - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
See below - Package doesn't own any directories other packages own.
See below- 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 sane scriptlets.
OK - Should have dist tag
OK - Should package latest version
2 outstanding bugs - check for outstanding bugs on package.

Issues:

1. Minor: might include a copy of the GPL.

2. Since redhat/fedora is upstream for this, can you make a note in the spec
as suggested in:
http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e

3. I assume the reason it only builds on ix86/x86_64 is that it only understands
lilo/grub? Might be worth filing a bug and noting it in the spec and see if
some ppc folk are interested in contributing yaboot support.

4. Please use one of the preferred buildroots, such as:
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

5. Do not use %makeinstall. See:
http://www.fedoraproject.org/wiki/PackagingDrafts/MakeInstall

6. The desktop file is missing a valid Main Category, see:
http://standards.freedesktop.org/menu-spec/latest/apa.html
Suggest: System or Settings be added.
Without this, this tool shows up under a "Other" menu in Xfce.

7. The buildrequires are not all needed, suggest changing:
BuildRequires: python >= 0:2.2, perl, gettext, glibc-devel, gcc,
desktop-file-utils, yelp, perl-XML-Parser

to

BuildRequires: python, gettext, desktop-file-utils, yelp, perl-XML-Parser

8. Shouldn't the firstboot package own
%dir /usr/share/firstboot/
%dir /usr/share/firstboot/modules
and not this package?

9. 2 outstanding bugs. Might look if either can be resolved easily.

10. rpmlint says:

a)
E: system-config-boot no-binary

I assume this is not noarch since it can be only run on ix86/x86_64?

b)
W: system-config-boot conffile-without-noreplace-flag /etc/pam.d/system-config-boot
W: system-config-boot conffile-without-noreplace-flag
/etc/security/console.apps/system-config-boot

Suggest: should those be (noreplace)?

c)
W: system-config-boot no-documentation

No docs available?

d)
E: system-config-boot non-executable-script
/usr/share/firstboot/modules/boot_gui.py 0644
E: system-config-boot non-executable-script
/usr/share/system-config-boot/system-config-boot.py 0644
Suggest: remove the #!/usr/bin/python from those.

e)
W: system-config-boot unversioned-explicit-obsoletes redhat-config-boot

Suggest: add a version here? or just remove it at this point?

f)
W: system-config-boot rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT
E: system-config-boot no-cleaning-of-buildroot %install

Suggest: Move the rm from prep to the top of install?

g)
W: system-config-boot macro-in-%changelog dist
W: system-config-boot macro-in-%changelog dist

Suggest: Change occurances of %dist in the changelog with %%dist

h)
E: system-config-boot-debuginfo empty-debuginfo-package

I guess you need to add
 %define debug_package %{nil}
if this really has to be an arch package.


Comment 2 Harald Hoyer 2007-03-23 12:36:09 UTC
> 1. Minor: might include a copy of the GPL.
done

> 2. Since redhat/fedora is upstream for this, can you make a note in the spec
> as suggested in:
>
http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e

done

> 3. I assume the reason it only builds on ix86/x86_64 is that it only understands
> lilo/grub? Might be worth filing a bug and noting it in the spec and see if
> some ppc folk are interested in contributing yaboot support.

what about sparc, s390 and the others?

> 4. Please use one of the preferred buildroots, such as:
>  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

done

> 5. Do not use %makeinstall. See:
> http://www.fedoraproject.org/wiki/PackagingDrafts/MakeInstall

done

> 6. The desktop file is missing a valid Main Category, see:
> http://standards.freedesktop.org/menu-spec/latest/apa.html
> Suggest: System or Settings be added.
> Without this, this tool shows up under a "Other" menu in Xfce.

Categories=System;Application;SystemSetup;X-Red-Hat-Base;

> 7. The buildrequires are not all needed, suggest changing:
> BuildRequires: python >= 0:2.2, perl, gettext, glibc-devel, gcc,
> desktop-file-utils, yelp, perl-XML-Parser

done

> 8. Shouldn't the firstboot package own
> %dir /usr/share/firstboot/
> %dir /usr/share/firstboot/modules
> and not this package?

done

> 9. 2 outstanding bugs. Might look if either can be resolved easily.

bug #134548 and bug #181749 are not easily fixable

> 10. rpmlint says:
>
> a) E: system-config-boot no-binary
> I assume this is not noarch since it can be only run on ix86/x86_64?

yep

> b)
> W: system-config-boot conffile-without-noreplace-flag 
> /etc/pam.d/system-config-boot
> W: system-config-boot conffile-without-noreplace-flag
> /etc/security/console.apps/system-config-boot
> Suggest: should those be (noreplace)?

done

> c) W: system-config-boot no-documentation 
> No docs available?

no .)

> d)
> Suggest: remove the #!/usr/bin/python from those.

done

> e) W: system-config-boot unversioned-explicit-obsoletes redhat-config-boot
> Suggest: add a version here? or just remove it at this point?

added version

> f)
> W: system-config-boot rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT
> E: system-config-boot no-cleaning-of-buildroot %install
> Suggest: Move the rm from prep to the top of install?

done

> g)
> W: system-config-boot macro-in-%changelog dist
> W: system-config-boot macro-in-%changelog dist
> 
> Suggest: Change occurances of %dist in the changelog with %%dist

done

> h) E: system-config-boot-debuginfo empty-debuginfo-package
> I guess you need to add
>  %define debug_package %{nil}
> if this really has to be an arch package.

done

please check system-config-boot-0.2.15-1.fc7


Comment 3 Kevin Fenzi 2007-03-24 03:12:31 UTC
>> 3. I assume the reason it only builds on ix86/x86_64 is that it only understands
>> lilo/grub? Might be worth filing a bug and noting it in the spec and see if
>> some ppc folk are interested in contributing yaboot support.
>
>what about sparc, s390 and the others?

Sure. I suppose there could be one bug about "non x86/x86_64" support?

>> 6. The desktop file is missing a valid Main Category, see:
>> http://standards.freedesktop.org/menu-spec/latest/apa.html
>> Suggest: System or Settings be added.
>> Without this, this tool shows up under a "Other" menu in Xfce.
>
>Categories=System;Application;SystemSetup;X-Red-Hat-Base;

Humm...my mistake. I thought it didn't have "System" in there.

>> g)
>> W: system-config-boot macro-in-%changelog dist
>> W: system-config-boot macro-in-%changelog dist
>>
>> Suggest: Change occurances of %dist in the changelog with %%dist
>
>done

I still see dist in the changelog. Looking further at it, I think
they shouldn't be there at all. You don't need to have them in the
version string per release.

Thanks for the quick fixes!


Comment 4 Kevin Fenzi 2007-05-30 03:07:10 UTC
Sorry for the delay in looking back at this. 

About the point #3, we are supposed to file a bug on excluding other primary
arches. This allows folks interested in those arches to see items they might
want to work on fixing. So, I think it might be good to have a ppc bug filed
blocking the ppcexcludearch blocker, so ppc hackers might add yaboot support.
Other platforms aren't primary arches, so I don't think we need to worry about
them right now. 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=FE-ExcludeArch-ppc

I guess removing the %{?dist} from the changelog is cosmetic, but if you get a
chance it should be easy to do. 

Provided you file that bug and fix the dist the next time you need to push a
update I will go ahead and approve this now. Feel free to close this rawhide
once those things are done.  

Comment 5 Kevin Fenzi 2007-08-05 19:18:24 UTC
This can be closed. 


Note You need to log in before you can comment on or make changes to this bug.