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

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

--- Comment #2 from Matěj Cepl <mcepl@xxxxxxxxxx> 2012-01-12 09:01:09 EST ---
(In reply to comment #1)
> 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.

Right. Thank you.

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

Will investigate. We don't have it on RHEL-6 (with mesa 7.11), so I need to
investigate what it brings to me if used. Probably will end if with some kind
of %if ...

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

Right, I'll try %cmake.

Hmm, fails to build
http://mcepl.fedorapeople.org/tmp/_build-1-0.9.git20120110Rf26fbd0.el6.log Any
ideas what's going on here?

I will try to ask upstream developers for help.

> 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} \

Right. Fixed.

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

basename(1) says:

 If specified, also remove a trailing SUFFIX.

Which is actually quite the same what
http://pubs.opengroup.org/onlinepubs/009695399/utilities/basename.html says.

I am not sure, whether it works with GNU basename, but I don't feel like going
against the standard without a reason.

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

Will investigate later ... we have glew on all imagineagble distros so it
shouldn't be the problem, but first let's make the package building again.

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