Bug 485418

> %files
> %defattr(-,root,root,-)
> %{_datadir}/vgabios/VGABIOS-lgpl-latest.bin
> %{_datadir}/vgabios/VGABIOS-lgpl-latest.cirrus.bin
> %{_datadir}/vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
> %{_datadir}/vgabios/VGABIOS-lgpl-latest.debug.bin

Your package also owns %{_datadir}/vgabios, doesn't it?

> %install

rm -rf $RPM_BUILD_ROOT

http://fedoraproject.org/wiki/Packaging/Guidelines#Prepping_BuildRoot_For_.25install

> %doc

Uh?

1) yeah, we do own vgabios directory. I now remember having this problem with etherboot too, but forgot about it. 

2) Forgot again

3) Got from a template, slipped through my fingers.

I'll update the packages.

Reviewers: see bug #464621 for a review of similar package (etherboot)

Ok, Updated it.

You can grab:


http://glommer.fedorapeople.org/vgabios-0.6-0.2beta.fc11.src.rpm
http://glommer.fedorapeople.org/vgabios.spec

%changelog seems to be in reverse order.  :)

I'll review it.

Blocker - it's a bad, bad idea, to use %{release} in source's naming scheme:

[petro@Sulaco SPECS]$ LANG=C rpmbuild -ba vgabios.spec 
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.6Ptmxi
+ umask 022
+ cd /home/petro/rpmbuild/BUILD
+ cd /home/petro/rpmbuild/BUILD
+ rm -rf vgabios-0.6
+ /usr/bin/gzip -dc /home/petro/rpmbuild/SOURCES/vgabios-0.6.tgz
+ /bin/tar -xf -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd vgabios-0.6
+ /bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ mkdir prebuilt
+ cd prebuilt
+ tar -zxf /home/petro/rpmbuild/SOURCES/vgabios-binaries-0.6-0.2beta.fc10.tar.gz
tar: /home/petro/rpmbuild/SOURCES/vgabios-binaries-0.6-0.2beta.fc10.tar.gz: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
tar: Child returned status 2
tar: Error exit delayed from previous errors
error: Bad exit status from /var/tmp/rpm-tmp.6Ptmxi (%prep)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.6Ptmxi (%prep)
[petro@Sulaco SPECS]$

I don't like the idea with shipping pre-built blobs. Please, explain, why we can't compile dev86 for non-x86 arches?

http://cvs.fedoraproject.org/viewvc/rpms/dev86/

I just commented out string with ExclusiveArch and it at least builds on my PowerPC. Will try to assembly this firmware with it.

Ok, I successed in building it on my Mac Mini G4. However, I don't know how to test these blobs :).

Anyway, here are my suggestions:

* Provide dev86 for all arches (not only for x86/x86_64)
* Mark this package as noarch

Few more notes:

* No need to install %docs. It's  enough to include them in %files, e.g. instead of

mkdir -p $RPM_BUILD_ROOT%{_docdir}/vgabios
install -m 0644 README COPYING $RPM_BUILD_ROOT%{_docdir}/vgabios 
...
%files
...
%docdir %{_docdir}/vgabios/
%doc %{_docdir}/vgabios/README
%doc %{_docdir}/vgabios/COPYING


you should write only 

%files
...
%doc README COPYING


* Source0 file doesn't match one, shipped within srpm:

[petro@Sulaco SOURCES]$ md5sum vgabios-0.6.tgz ~/Desktop/vgabios-0.6b.tgz
35ddf3eaf16abe546797ae9cc18cbd6e  vgabios-0.6.tgz
36399621c4d6753e83a3cec3009c7183  /home/petro/Desktop/vgabios-0.6b.tgz
[petro@Sulaco SOURCES]$

Generally, it's not a problem if you need to ship different (from upstream) source, but in case then you *really* need to modify upstream sources (or even create tarball from VCS), you *must* provide instructions on how to create exact the same source file in comments to your spec-file. See this spec-file as an example:

http://cvs.fedoraproject.org/viewvc/rpms/flashrom/devel/flashrom.spec?view=markup

Okay, I've updated the package to address your concerns. Hopefully I'm covering all of them

grab them at 

http://glommer.fedorapeople.org/vgabios-0.6-0.3beta.fc11.src.rpm
http://glommer.fedorapeople.org/vgabios.spec

Note that koji builds will fail on ppc64. This is due to a dev86 bug, to which I've already submitted the patch.

REVIEW:

- rpmlint is not silent:

[petro@Sulaco rpmbuild]$ rpmlint RPMS/noarch/vgabios-0.6-0.3beta.fc10.noarch.rpm 
vgabios.noarch: W: non-standard-group Application/Emulators
vgabios.noarch: W: incoherent-version-in-changelog 0.6.0.3 ['0.6-0.3beta.fc10', '0.6-0.3beta']
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
[petro@Sulaco rpmbuild]$

These two warnings are easy-to-fix. First, you accidentally made a typo - non Application/Emulators, but Applications/Emulators (see /usr/share/doc/rpm-4.6.0/GROUPS ). Second, just use correct EVR in %changelog, e.g. replace 

* Mon Feb 16 2009 Glauber Costa <glommer> - 0.6.0.3

with

* Mon Feb 16 2009 Glauber Costa <glommer> - 0.6-0.3beta

+ 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, and I have only one small advice. This string (looks like leftover) should be removed:

mkdir -p $RPM_BUILD_ROOT%{_docdir}/vgabios

+ 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 with text of the license is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as provided in the spec URL. 

[petro@Sulaco SOURCES]$ md5sum vgabios-0.6b.tgz*
36399621c4d6753e83a3cec3009c7183  vgabios-0.6b.tgz
36399621c4d6753e83a3cec3009c7183  vgabios-0.6b.tgz.from_srpm
[petro@Sulaco SOURCES]$

+ The package successfully compiles and builds into binary rpms on at least one primary architecture (my ppc)
+ All build dependencies are listed in BuildRequires.
+ No need to handle locales.
+ No shared libraries.
+ The package owns all directories that it creates.
+ The package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently use macros.
+ The package contains code, or permissible content.
+ No large documentation files.
+ The stuff, that the package includes as %doc, does not affect the runtime of the application.
+ No header files.
+ No static libraries.
+ No pkgconfig(.pc) files.
+ No library files with a suffix
+ No .la libtool archives
+ Not a GUI app
+ 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 the package must be valid UTF-8.

Assuming, that you fixed three issues, described above (two from rpmlint and one leftover), and that buildroot already contains latest dev86 rpm, this package is 


APPROVED

Ok, I addressed your issues:

http://glommer.fedorapeople.org/vgabios.spec
http://glommer.fedorapeople.org/vgabios-0.6-0.4beta.fc11.src.rpm

Can you take a look at: 
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

I think you want 0.6-0.1.b or 0.6-0.1.beta as your verson? Or does upstream have different beta versions? I only see the b tag.

Glauber: Any reply to comment #16?

hope it is okay now.

you can grab:
http://glommer.fedorapeople.org/vgabios.spec
http://glommer.fedorapeople.org/vgabios-0.6-0.5.b.fc11.src.rpm

New Package CVS Request
=======================
Package Name: vgabios
Short Description: an LPGL implementation of a bios for a video card.
Owners: glommer
Branches: F-11
InitialCC: virt-maint

I don't see virt-maint in the list of cc lists we can add. 

cvs done otherwise.

(In reply to comment #20)
> I don't see virt-maint in the list of cc lists we can add. 

It's fedora-virt-maint. Can it be added?

(In reply to comment #21)
> (In reply to comment #20)
> > I don't see virt-maint in the list of cc lists we can add. 
> 
> It's fedora-virt-maint. Can it be added?

I can do that.  How is this different from xen-maint?  Should I just replace that with this one or do they exist for separate purposes?

hmm... and looking at the mailing list, it looks like there is an account for this already -- Someone seems to have set up "virtmaint" to go there.

This is a tiny bit dangerous as it's a real account, not one of our pseudo accounts.  So it will be subject to account expiration, someone could compromise its password, etc....

(In reply to comment #22)
> > 
> > It's fedora-virt-maint. Can it be added?
> 
> I can do that.  How is this different from xen-maint?  Should I just
> replace that with this one or do they exist for separate purposes?

xen-maint is used for different purposes. Please keep xen-maint as is.

Okay.  I've added virtmaint on the CC.  Do you know who created/is responsible for virtmaint?  I need to get it straightened out if it should be a pseudo account or we're likely to have problems down the line.

(In reply to comment #25)
> Okay.  I've added virtmaint on the CC.  Do you know who created/is responsible
> for virtmaint?  I need to get it straightened out if it should be a pseudo
> account or we're likely to have problems down the line.

Mark McLoughlin probably created it, but he is away for a few weeks. But if the existing notifications and the fedora-virt-maint mailing list keep working, I think you can go ahead change it to the safer setup.

Clearning cvs flag here.