I'll review it.

But first of all I have a questions.

* Could you please explain the current situation with open OpenCL implementations in drivers?

* So far I found this project, opencl-utils (available in Fedora), and pocl ( http://pocl.sourceforge.net/ ). Is it possible to build, link and execute an OpenCL program with some of these? I mean w/o waiting for open videodrivers to catch up. I fully realize that this wouldn't be fast w/o GPU help but anyway - is it possible?

Thanks in advance! Now let's do the official part.

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is almost silent (the only warning may be safely ignored)


sulaco ~/rpmbuild/SPECS: rpmlint ../RPMS/ppc/ocl-icd-* ../SRPMS/ocl-icd-2.0.2-1.fc19.src.rpm 
ocl-icd.src: W: invalid-url Source0: ocl-icd-20130309.tar.bz2
4 packages and 0 specfiles checked; 0 errors, 1 warnings.
sulaco ~/rpmbuild/SPECS: 


+ 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.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.

- The License field in the package spec file MUST match the actual license. I think it's 2-clause BSD, not MIT:

* http://en.wikipedia.org/wiki/BSD_licenses#2-clause_license_.28.22Simplified_BSD_License.22_or_.22FreeBSD_License.22.29
* https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#2ClauseBSD

So correct licensing tag is BSD.

- The file, containing the text of the license(s) for the package, MUST BE included in %doc. Please mark it as %doc.

+ The spec file is written in American English.
+ The spec file for the package is legible.

- I don't know how to regenerate tarball used for building a package. Please add to the spec-file's header something like this:

* http://pkgs.fedoraproject.org/cgit/b43-tools.git/tree/b43-tools.spec#n15

+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
+ The package stores shared library files in some of the dynamic linker's default paths, and it calls ldconfig in %post and %postun.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.

- The package MUST own all directories that it creates. You forgot to mark "%{_datadir}/doc/ocl-icd" as %dir in the devel sub-package's %files section. So it should look like this:

%files devel
%defattr(-,root,root,-)
%{_includedir}/ocl_icd.h
%{_libdir}/pkgconfig/ocl-icd.pc
%{_libdir}/pkgconfig/OpenCL.pc
%{_libdir}/libOpenCL.so
%dir %{_datadir}/doc/ocl-icd
%dir %{_datadir}/doc/ocl-icd/examples
%{_datadir}/doc/ocl-icd/examples/ocl_icd_bindings.c
%{_datadir}/doc/ocl-icd/examples/ocl_icd_loader.map

+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files are stored in a -devel package.
0 No static libraries.
+ The pkgconfig(.pc) files are stored in a -devel package and necessary runtime requirement added.
+ The library file(s) that end in .so (without suffix) is(are) stored in a -devel package.

- The -devel package MUST require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release}. Note %{?_isa} additional tag.

+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ All filenames in rpm packages are valid UTF-8.



So here is a summary (with few more small proposals):

* Fix Licence field.
* Add COPYING as %doc
* Add a description of how to regenerate tarball used for building
* Add missing unowned directory in *-devel
* Add _isa to the dependency on the main package (in the devel sub-package). 
* You may drop %defattr(-,root,root,-) - it's no longer needed since RHEL5 / Fedora 6.
* Regarding this - "autoreconf -v --install || exit 1". You'd better use autoreconf -ivf (note "f" switch).

I'm looking forward to hearing from you!

(In reply to comment #1)
> I'll review it.
> 
> But first of all I have a questions.
> 
> * Could you please explain the current situation with open OpenCL
> implementations in drivers?
> 
> * So far I found this project, opencl-utils (available in Fedora), and pocl
> ( http://pocl.sourceforge.net/ ). Is it possible to build, link and execute
> an OpenCL program with some of these? I mean w/o waiting for open
> videodrivers to catch up. I fully realize that this wouldn't be fast w/o GPU
> help but anyway - is it possible?

fwiw, Dave Airlie was enabling mesa opencl support in rawhide, and I believe he had some plans to use ocl-icd with the mesa ocl drivers.

> Thanks in advance! Now let's do the official part.
> 
> REVIEW:
> 
> Legend: + = PASSED, - = FAILED, 0 = Not Applicable
> 
> + rpmlint is almost silent (the only warning may be safely ignored)
> 
> 
> sulaco ~/rpmbuild/SPECS: rpmlint ../RPMS/ppc/ocl-icd-*
> ../SRPMS/ocl-icd-2.0.2-1.fc19.src.rpm 
> ocl-icd.src: W: invalid-url Source0: ocl-icd-20130309.tar.bz2
> 4 packages and 0 specfiles checked; 0 errors, 1 warnings.
> sulaco ~/rpmbuild/SPECS: 
> 
> 
> + 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.
> + The package is licensed with a Fedora approved license and meets the
> Licensing Guidelines.
> 
> - The License field in the package spec file MUST match the actual license.
> I think it's 2-clause BSD, not MIT:
> 
> *
> http://en.wikipedia.org/wiki/BSD_licenses#2-clause_license_.28.
> 22Simplified_BSD_License.22_or_.22FreeBSD_License.22.29
> * https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#2ClauseBSD
> 
> So correct licensing tag is BSD.
> 
> - The file, containing the text of the license(s) for the package, MUST BE
> included in %doc. Please mark it as %doc.
> 
> + The spec file is written in American English.
> + The spec file for the package is legible.
> 
> - I don't know how to regenerate tarball used for building a package. Please
> add to the spec-file's header something like this:
> 
> * http://pkgs.fedoraproject.org/cgit/b43-tools.git/tree/b43-tools.spec#n15

fwiw, I've made (I think) all the other updates except this one, because I was not entirely sure what to do.  I used libdrm.spec as a template, but I'm not entirely sure what triggers running the make-git-snapshot.sh script.

updated spec file at same location:
http://people.freedesktop.org/~robclark/rpmbuild/ocl-icd/ocl-icd.spec

BR,
-R

> + The package successfully compiles and builds into binary rpms on at least
> one primary architecture.
> + All build dependencies are listed in BuildRequires.
> 0 No need to handle locales.
> + The package stores shared library files in some of the dynamic linker's
> default paths, and it calls ldconfig in %post and %postun.
> + The package does NOT bundle copies of system libraries.
> 0 The package is not designed to be relocatable.
> 
> - The package MUST own all directories that it creates. You forgot to mark
> "%{_datadir}/doc/ocl-icd" as %dir in the devel sub-package's %files section.
> So it should look like this:
> 
> %files devel
> %defattr(-,root,root,-)
> %{_includedir}/ocl_icd.h
> %{_libdir}/pkgconfig/ocl-icd.pc
> %{_libdir}/pkgconfig/OpenCL.pc
> %{_libdir}/libOpenCL.so
> %dir %{_datadir}/doc/ocl-icd
> %dir %{_datadir}/doc/ocl-icd/examples
> %{_datadir}/doc/ocl-icd/examples/ocl_icd_bindings.c
> %{_datadir}/doc/ocl-icd/examples/ocl_icd_loader.map
> 
> + The package does not list a file more than once in the spec file's %files
> listings.
> + Permissions on files are set properly.
> + The package consistently uses macros.
> + The package contains code, or permissible content.
> 0 No extremely large documentation files.
> + Anything, the package includes as %doc, does not affect the runtime of the
> application.
> + Header files are stored in a -devel package.
> 0 No static libraries.
> + The pkgconfig(.pc) files are stored in a -devel package and necessary
> runtime requirement added.
> + The library file(s) that end in .so (without suffix) is(are) stored in a
> -devel package.
> 
> - The -devel package MUST require the base package using a fully versioned
> dependency: Requires: %{name}%{?_isa} = %{version}-%{release}. Note %{?_isa}
> additional tag.
> 
> + The package does NOT contain any .la libtool archives.
> 0 Not a GUI application.
> + The package does not own files or directories already owned by other
> packages.
> + All filenames in rpm packages are valid UTF-8.
> 
> 
> 
> So here is a summary (with few more small proposals):
> 
> * Fix Licence field.
> * Add COPYING as %doc
> * Add a description of how to regenerate tarball used for building
> * Add missing unowned directory in *-devel
> * Add _isa to the dependency on the main package (in the devel sub-package). 
> * You may drop %defattr(-,root,root,-) - it's no longer needed since RHEL5 /
> Fedora 6.
> * Regarding this - "autoreconf -v --install || exit 1". You'd better use
> autoreconf -ivf (note "f" switch).
> 
> I'm looking forward to hearing from you!

Regarding tarball regeneration - that's not a big deal anyway, so I won't insist here. All other issues were fixed.

OK, I don't see any other issues so this package is


APPROVED.

New Package SCM Request
=======================
Package Name: ocl-icd
Short Description: OpenCL ICD Bindings
Owners: rclark
Branches: f19
InitialCC: airlied whot xgl-maint

Git done (by process-git-requests).