[Bug 773021] Review Request: piglit - Collection of automated tests for OpenGL implementations

[bugzilla at redhat.com]

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

--- Comment #1 from Scott Tsai <scottt.tw at gmail.com> 2012-01-11 21:43:37 EST ---
1. I believe the license should be "MIT and GPLv2+ and GPLv3 and LGPLv2".
Fro the COPYING file:
"The following is the 'standard copyright' agreed upon by most
contributors, and is currently the canonical license preferred by the
piglit project." and the "Copyright @ 2010 Intel" license that follows is the
"Modern Style without sublicense" variant of the MIT license from:
http://fedoraproject.org/wiki/Licensing:MIT. Grepping for "Intel" shows that a
lot of files carry this license.

2. If you add a BuildRequires on mesa-libGLES-devel then the
OPENGL_gles{1,2}_LIBRARY cmake variables would be set and the tests in the
corrpesponding directories would get built.

3. When you invoke cmake, you're not passing %{optflags} down like the "%cmake"
macro in /etc/rpm/macros.cmake thus piglit is compiled at the default
optimization level.  From the README file, I see that's probably what upstream
developers do as well, but I recommend you add a comment in the spec file if
it's intentional.

4. When you're creating the /usr/bin/piglit* symlinks with:
for srcfile in piglit*.py ; do
    ln -sf ../..%{_libdir}/%{name}/${srcfile} \
        $RPM_BUILD_ROOT%{_bindir}/$(basename ${srcfile} .py)
done

Since "../../%{_libdir}" hard codes the assumption that %{_bindir}
is only two levels from '/' you might as well make the symlink paths absolute:
 -ln -sf ../..%{_libdir}/%{name}/${srcfile} \
 +ln -sf %{_libdir}/%{name}/${srcfile} \

5. Also in the above shell snippet, I believe the ".py" at the end of
"$(basename ${srcfile} .py)" is superfluous.

Note that tests/util/glew.{ch} and friends in the source is an embedded copy of
the OpenGL Extension Wrangler Library ("glew" in Fedora). I frankly think that
a device driver testsuite embedding a copy of a small utility library causes no
harm and would approve this package regardless.

-- 
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.