Bug 485420

Summary: Review Request: openbios - Open Source implementation of IEEE 1275-1994
Product: [Fedora] Fedora Reporter: Glauber Costa <gcosta>
Component: Package ReviewAssignee: Peter Lemenkov <lemenkov>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: ehabkost, fedora-package-review, lemenkov, markmc, notting, virt-maint
Target Milestone: ---Flags: lemenkov: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-03-07 10:31:12 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Glauber Costa 2009-02-13 09:35:35 EST
Spec URL: http://glommer.fedorapeople.org/openbios.spec
SRPM URL: http://glommer.fedorapeople.org/openbios-1.0-0.1.svn450.fc11.src.rpm
Description: 
The OpenBIOS project provides you with most free and open source Open Firmware
implementations available. Here you find several implementations of
IEEE 1275-1994 (Referred to as Open Firmware) compliant firmware. Among its
features, Open Firmware provides an instruction set independent device interface.
This can be used to boot the operating system from expansion cards without
native initialization code.

It is Open Firmware's goal to work on all common platforms, like x86, AMD64,
PowerPC, ARM and Mips. With its flexible and modular design, Open Firmware
targets servers, workstations and embedded systems, where a sane and unified
firmware is a crucial design goal and reduces porting efforts noticably.

Open Firmware is found on many servers and workstations and there are sever
commercial implementations from SUN, Firmworks, CodeGen, Apple, IBM and others.

In most cases, the Open Firmware implementations provided on this site rely on
an additional low-level firmware for hardware initialization, such as coreboot
or U-Boot.
Comment 1 Eduardo Habkost 2009-02-13 10:21:43 EST
> %files
> %defattr(-,root,root,-)
> %dir %{_datadir}/openbios/openbios-sparc32
> %dir %{_datadir}/openbios/openbios-sparc64
> #%dir %{_datadir}/openbios/openbios-ppc

Shouldn't the package own the %{_datadir}/openbios directory?

http://fedoraproject.org/wiki/Packaging/UnownedDirectories
Comment 2 Mark McLoughlin 2009-02-13 10:25:26 EST
Reviewers: see bug #464621 for a review of similar package (etherboot)
Comment 4 Peter Lemenkov 2009-02-14 06:39:32 EST
I'll review it.
Comment 5 Peter Lemenkov 2009-02-14 06:43:17 EST
Blocker - missing fcode utils as BuildReqires. You should consider packaging them also.

http://www.openfirmware.info/FCODE_suite
Comment 6 Peter Lemenkov 2009-02-14 15:23:00 EST
Ok, let's assume that you (or something else) already packaged fcode and you added proper BuildRequires into spec-file.

Notes:

* %prep section contains leftover (ls command)
* This package must be marked as noarch
* The same issue with %docs as in bz#485418
* I still think that shipping pre-build firmware is a bad idea. What prevents us to build them all from source on every arch?
Comment 7 Glauber Costa 2009-02-17 10:22:40 EST
Peter,

Fcode was not required to build the pieces I'm building so far. This is the reason I have not included it. But I can happily shoot a FCode package if you really feel necessary.

As for the binaries, nobody likes them. Your suggestion in vgabios was totally welcome and accepted. But in that case, dev86 was compiling everything. In this package, (as well as bochs-bios and etherboot), gcc is doing a big part of the job. By purely gcc suckiness, we can't easily generate code for alternate platforms other than native.

So I'm afraid we do have to build once, and then pack binaries in the way we did for etherboot. Please take a look at that package review for more information.
Comment 8 Glauber Costa 2009-03-02 21:21:11 EST
Peter, please take a look at

http://glommer.fedorapeople.org/openbios-1.0-0.3.svn463.fc11.src.rpm
http://glommer.fedorapeople.org/openbios.spec

I had a nice idea of how to explore koji's new capabilities, and this is the result.

Because of the conditional architecture, openbios-xxx will only be built in the architecture it was targeted for, and will then span to all other targets.

If you want to have a better picture of what I'm talking about, there are koji builds at:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1215501 (primary arches)
http://sparc.koji.fedoraproject.org/koji//taskinfo?taskID=160708 (sparc)

The only remaining issue, is the fact that sparc is a secondary architecture. And as of now, builds in secondary architectures don't reach primary ones. But this seems the best we can do now.
Comment 9 Peter Lemenkov 2009-03-03 16:57:19 EST
From my PoV it;s still look ugly - I don't think that it's easy to understand that all these tricks with exactarch/noarch for w/o comments. In other words, I think that this package is not legible enough. However, keeping in mind that we really need this package in order to clean up mess with pre-built blobs in qemu here is my 

REVIEW:

- rpmlint is not silent (checked on ppc):

openbios.src:137: E: files-attr-not-set
openbios.src:138: E: files-attr-not-set
openbios.src:139: E: files-attr-not-set
openbios.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 10)
openbios.src: W: summary-ended-with-dot OpenBios implementation of IEEE 1275-1994.
openbios.src: E: description-line-too-long features, Open Firmware provides an instruction set independent device interface.
openbios-debuginfo.ppc: E: empty-debuginfo-package
openbios-ppc.noarch: W: no-documentation
openbios-ppc.noarch: E: arch-independent-package-contains-binary-or-object /usr/share/openbios/openbios-ppc
4 packages and 0 specfiles checked; 6 errors, 3 warnings.

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+/- The package meets the Packaging Guidelines. I've got only one complain about explicit installation of docs in %install section. I't enogh just to list them as %doc
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines .
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
- The spec file for the package is not legible enough (see messages above). unfortunately, I don't think that this can be easily fixed, because this situation is relatively complex (different parts of this package should be built on different arches).

- The sources used to build the package must match the upstream source, as provided in the spec URL. Unfortunately, I failed to download sources using the following address (taken from spec-file):

http://downloads.sourceforge.net/openbios/openbios-1.0.tar.gz

+ The package successfully compiles and builds into binary rpms on at least one primary architecture (see koji ogs above)
+ All build dependencies are listed in BuildRequires.

- The package must own all directories that it creates. Unfortunately, I can't find that sub-package owns %{_datadir}/openbios

+ A package does not contain any duplicate files in the %files listing.

- Permissions on files must be set properly. See rpmlint warnings above.
+ The package has a %clean section.
+ The package consistently uses macros.
+ The package contains code, or permissable content.
+ Everything, the package includes as %doc, does not affect the runtime of the application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Please, fix issues, mentioned above.
Comment 10 Glauber Costa 2009-03-04 15:29:48 EST
Ok, uploaded a new version.

http://glommer.fedorapeople.org/openbios-1.0-0.4.svn463.fc11.src.rpm
http://glommer.fedorapeople.org/openbios.spec

As for rpmlint complaining about binaries in noarch subpackages... There's nothing we can do here ;-)

I also added a good amount of comments that might help people looking at it.
Comment 11 Glauber Costa 2009-03-05 09:43:16 EST
Uploaded:

http://glommer.fedorapeople.org/openbios-1.0-0.5.svn463.fc11.src.rpm
http://glommer.fedorapeople.org/openbios.spec

changing openbios-doc to openbios-common
Comment 12 Peter Lemenkov 2009-03-05 13:14:03 EST
OK, all issues are fixed, so this package is 

APPROVED
Comment 13 Glauber Costa 2009-03-05 13:19:01 EST
New Package CVS Request
=======================
Package Name: openbios
Short Description: OpenBios implementation of IEEE 1275-1994
Owners: glommer
Branches:
InitialCC: fedora-virt-maint@redhat.com
Comment 14 Kevin Fenzi 2009-03-05 14:13:49 EST
cvs done.