[Bug 678442] Review Request: os-prober - Probes disks on the system for installed operating systems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=678442

--- Comment #12 from Hedayat Vatankhah <hedayatv@xxxxxxxxx> 2011-05-03 20:41:37 EDT ---
Thanks for your through review. 

(In reply to comment #11)
> Here are a few general comments before I get to the usual review items.  First,
> if this spec file will only be used for Fedora, then the following elements of
> the spec file are unnecessary:
> - The BuildRoot tag
> - rm -rf %{buildroot} at the top of the %install script
> - The %clean script
> - %defattr(-,root,root,-) at the top of %files
Done.


> 
> Second, there are some debian-isms in these script files.  For example,
> /usr/libexec/os-probes/init/10filesystems contains references to anna-install
> and /lib/debian-installer, and /usr/share/os-prober/common.sh contains
> references to mapdevfs.  Will the absence of these on Fedora lead to problems?
No, those will be used only when this package is used inside debian installer.
They'll be ignored otherwise.


> 
> Third, from poking around in the scripts a little, I see uses of binaries in
> the following packages: coreutils, cryptsetup-luks, dmraid, grep, lvm2,
> module-init-tools, udev, and util-linux-ng.  None of them are Requires of this
> package.  Should they be?
Yes, you are right. I've added a number of them as Requires, but leaving a few
of them out since they don't look to be really useful, and will be silently
ignored if not available.


> 
> Output from rpmlint:
> os-prober.x86_64: W: no-manual-page-for-binary os-prober
> os-prober.x86_64: W: no-manual-page-for-binary linux-boot-prober
> os-prober-debuginfo.x86_64: E: empty-debuginfo-package
> 2 packages and 1 specfiles checked; 1 errors, 2 warnings.
> 
> The debuginfo package is empty because the newns binary is not in a location
> where rpmbuild expects an ELF object.  Is there any way that binary could be
> put directly in /usr/libexec; i.e., not in a subdirectory?
Done.


> -: must be fixed
> =: needs attention
>
> MUST:
> [-]: rpmlint output (posted above, shows the -debuginfo problem)
Addressed above.

[...]
> SHOULD:
> [=]: if not license text file, packager should query upstream
> [=]: package functions as described (minimal testing only, as I have only
> Fedora linux disks)
> [=]: man pages for binaries: there are none
I'll contact upstream about both man page(s) and license text.

New files:
SPEC: http://hedayat.fedorapeople.org/reviews/os-prober/1.46-2/os-prober.spec
SRPM:
http://hedayat.fedorapeople.org/reviews/os-prober/1.46-2/os-prober-1.46-2.fc15.src.rpm

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]