Spec URL: http://mcepl.fedorapeople.org/tmp/piglit.spec SRPM URL: http://mcepl.fedorapeople.org/tmp/task_3635565/piglit-1-0.9.git20120110Rf26fbd0.fc17.src.rpm Description: Piglit is a collection of automated tests for OpenGL implementations. The goal of Piglit is to help improve the quality of open source OpenGL drivers by providing developers with a simple means to perform regression tests. Built in koji https://koji.fedoraproject.org/koji/taskinfo?taskID=3635565
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.
(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.
(In reply to comment #2) > Hmm, fails to build > http://mcepl.fedorapeople.org/tmp/_build-1-0.9.git20120110Rf26fbd0.el6.log Any > ideas what's going on here? I'll give it a shoot but please feel free to beat me to it ^_^ >> the ".py" at the end of "$(basename ${srcfile} .py)" is superfluous. > ... > I am not sure, whether it works with GNU basename, but I don't feel like going > against the standard without a reason. I didn't realize basename has this filename suffix stripping functionality and was totally wrong here. Please ignore. >> tests/util/glew.{ch} is a bundled copy of the "glew" library > .. > 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. After looking into the way piglit patches glew: http://cgit.freedesktop.org/piglit/log/tests/util/glew.c http://sourceforge.net/tracker/?func=detail&aid=3433049&group_id=67586&atid=523274 (open glew upstream bug with patch, known piglit crasher) http://sourceforge.net/tracker/?func=detail&aid=3433049&group_id=67586&atid=523274 and http://lists.freedesktop.org/archives/piglit/2012-January/001612.html (MESA on i965 Sandy Bridge can support OpenGL 3.0 but not the ARB_uniform_buffer_object extension, glew upstream apparently never ran into such an OpenGL implementation) Note that piglit is patching glew.c which is in fact an auto generated file in upstream glew. Since piglit tests are often added after a new OpenGL extension / feature is added to some (MESA, hardware driver) combination and the exact OpenGL functionality supported by MESA is necessary "quirky" as it continues to develop I think we can expect the bundled copy of glew in piglit to only occasionally be in sync with upstream. I seriously suggest you use the bundled copy as is or ask FESCO for an bundled library exception if necessary. I've also opened bug 773679 to request that the glew package be updated to 1.7.0 in Fedora. (piglit has patches on top of glew 1.7.0, Fedora currently has glew 1.6.0)
(In reply to comment #2) In http://mcepl.fedorapeople.org/tmp/_build-1-0.9.git20120110Rf26fbd0.el6.log the build failure is caused by: /usr/bin/gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Wall -g -fPIC CMakeFiles/crash-cubemap-order.dir/crash-cubemap-order.c.o -o ../../../../bin/crash-cubemap-order -rdynamic ../../../../lib/libpiglitutil.so -lGL -lGLU -lglut -lX11 -lm ../../../../lib/libpiglitutil.so: undefined reference to `piglit_window_mode' ../../../../lib/libpiglitutil.so: undefined reference to `piglit_height' ../../../../lib/libpiglitutil.so: undefined reference to `piglit_display' ../../../../lib/libpiglitutil.so: undefined reference to `piglit_width' ../../../../lib/libpiglitutil.so: undefined reference to `piglit_init' If you remove "-DBUILD_SHARED_LIBS:BOOL=ON" from you cmake invocation, libpiglitutil would get built as a static library and the link failure would not happen. Analysis: apparently piglit tests that use window related functions from libpiglit must define the "piglit_window_mode" etc global variables. crash-cubemap-order.c doesn't use those functions, doesn't define those global variables and thus linking fails when libpiglit is built as a shared library.
(In reply to comment #4) s/libpiglit/libpiglitutil/
New version is http://mcepl.fedorapeople.org/tmp/piglit-1-0.10.git20120110Rf26fbd0.el6.src.rpm and http://mcepl.fedorapeople.org/tmp/piglit.spec Also filed https://fedorahosted.org/fpc/ticket/132 the request for bundled library exception.
(In reply to comment #6) You're not using the %cmake macro: -cmake -DCMAKE_BUILD_TYPE:STRING=Debug -DBUILD_SHARED_LIBS:BOOL=OFF . +%cmake -DCMAKE_BUILD_TYPE:STRING=Debug -DBUILD_SHARED_LIBS:BOOL=OFF .
(In reply to comment #6) While trying to run: piglit-run /usr/lib64/piglit/tests/quick.tests RESULTS I noticed that piglit is trying to "mkdir /usr/lib64/piglit/RESULTS" and came up with this patch: http://lists.freedesktop.org/archives/piglit/2012-January/001697.html I think you'll want to include the patch.
(In reply to comment #8) > (In reply to comment #6) > While trying to run: > piglit-run /usr/lib64/piglit/tests/quick.tests RESULTS > I noticed that piglit is trying to "mkdir /usr/lib64/piglit/RESULTS" and came > up with this patch: > http://lists.freedesktop.org/archives/piglit/2012-January/001697.html > > I think you'll want to include the patch. Yes, I did. We use absolute paths in our QA, and I wanted to preserve the previous behavior, but yes, I agree with you, this is better. New version is on http://mcepl.fedorapeople.org/tmp/piglit-1-0.11.git20120110Rf26fbd0.el6.src.rpm (SPEC file in the same URL as before).
1. You're shipping the GPL-2 file twice: /usr/share/doc/piglit-1/GPL-2 and /usr/share/doc/piglit-1/documentation/GPL-2 I suggest you remove /usr/share/doc/piglit-1/documentation/ as it contains only GPL-2. 2. You're shipping cmake files and the generated Makefile: /usr/lib64/piglit/generated_tests/CMakeFiles /usr/lib64/piglit/generated_tests/CMakeFiles/CMakeDirectoryInformation.cmake /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/DependInfo.cmake /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/build.make /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/cmake_clean.cmake /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/depend.internal /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/depend.make /usr/lib64/piglit/generated_tests/CMakeFiles/gen-tests.dir/progress.make /usr/lib64/piglit/generated_tests/CMakeFiles/progress.marks /usr/lib64/piglit/generated_tests/CMakeLists.txt /usr/lib64/piglit/generated_tests/Makefile /usr/lib64/piglit/generated_tests/cmake_install.cmake /usr/lib64/piglit/tests/CMakeLists.txt /usr/lib64/piglit/tests/asmparsertest/CMakeLists.gl.txt /usr/lib64/piglit/tests/asmparsertest/CMakeLists.txt /usr/lib64/piglit/tests/bugs/CMakeLists.gl.txt /usr/lib64/piglit/tests/bugs/CMakeLists.txt /usr/lib64/piglit/tests/egl/CMakeLists.gl.txt /usr/lib64/piglit/tests/egl/CMakeLists.txt /usr/lib64/piglit/tests/fbo/CMakeLists.gl.txt /usr/lib64/piglit/tests/fbo/CMakeLists.txt /usr/lib64/piglit/tests/general/CMakeLists.gl.txt /usr/lib64/piglit/tests/general/CMakeLists.txt /usr/lib64/piglit/tests/glean/CMakeLists.gl.txt /usr/lib64/piglit/tests/glean/CMakeLists.txt /usr/lib64/piglit/tests/gles2/CMakeLists.gles2.txt /usr/lib64/piglit/tests/gles2/CMakeLists.txt /usr/lib64/piglit/tests/glslparsertest/CMakeLists.gl.txt /usr/lib64/piglit/tests/glslparsertest/CMakeLists.gles2.txt /usr/lib64/piglit/tests/glslparsertest/CMakeLists.txt /usr/lib64/piglit/tests/glx/CMakeLists.gl.txt /usr/lib64/piglit/tests/glx/CMakeLists.txt /usr/lib64/piglit/tests/hiz/CMakeLists.gl.txt /usr/lib64/piglit/tests/hiz/CMakeLists.txt /usr/lib64/piglit/tests/mesa/CMakeLists.txt /usr/lib64/piglit/tests/mesa/tests/CMakeLists.gl.txt /usr/lib64/piglit/tests/mesa/tests/CMakeLists.txt /usr/lib64/piglit/tests/mesa/util/CMakeLists.gl.txt /usr/lib64/piglit/tests/mesa/util/CMakeLists.txt /usr/lib64/piglit/tests/shaders/CMakeLists.gl.txt /usr/lib64/piglit/tests/shaders/CMakeLists.txt /usr/lib64/piglit/tests/spec/CMakeLists.txt /usr/lib64/piglit/tests/spec/amd_seamless_cubemap_per_texture/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/amd_seamless_cubemap_per_texture/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_blend_func_extended/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_blend_func_extended/api/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_blend_func_extended/api/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_color_buffer_float/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_color_buffer_float/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_copy_buffer/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_copy_buffer/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_draw_buffers/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_draw_buffers/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_draw_elements_base_vertex/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_draw_elements_base_vertex/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_es2_compatibility/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_es2_compatibility/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_fragment_program/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_fragment_program/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_framebuffer_object/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_framebuffer_object/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_instanced_arrays/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_instanced_arrays/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_map_buffer_range/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_map_buffer_range/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_robustness/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_robustness/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_sampler_objects/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_sampler_objects/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_seamless_cube_map/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_seamless_cube_map/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_shader_objects/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_shader_objects/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_shader_texture_lod/execution/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_texture_compression/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_texture_compression/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_texture_float/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_texture_float/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_texture_storage/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_texture_storage/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_transform_feedback2/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_transform_feedback2/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_vertex_buffer_object/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_vertex_buffer_object/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_vertex_program/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_vertex_program/CMakeLists.txt /usr/lib64/piglit/tests/spec/arb_vertex_type_2_10_10_10_rev/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/arb_vertex_type_2_10_10_10_rev/CMakeLists.txt /usr/lib64/piglit/tests/spec/ati_draw_buffers/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ati_draw_buffers/CMakeLists.txt /usr/lib64/piglit/tests/spec/ati_envmap_bumpmap/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ati_envmap_bumpmap/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_fog_coord/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_fog_coord/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_packed_depth_stencil/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_packed_depth_stencil/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_packed_float/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_packed_float/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_texture_integer/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_texture_integer/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_timer_query/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_timer_query/CMakeLists.txt /usr/lib64/piglit/tests/spec/ext_transform_feedback/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/ext_transform_feedback/CMakeLists.txt /usr/lib64/piglit/tests/spec/gl-2.0/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/gl-2.0/CMakeLists.txt /usr/lib64/piglit/tests/spec/gl-2.0/api/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/gl-2.0/api/CMakeLists.txt /usr/lib64/piglit/tests/spec/gl-2.1/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/gl-2.1/CMakeLists.txt /usr/lib64/piglit/tests/spec/gl-3.0/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/gl-3.0/CMakeLists.txt /usr/lib64/piglit/tests/spec/gl-3.0/api/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/gl-3.0/api/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.10/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.10/execution/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.10/execution/clipping/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.10/execution/clipping/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.20/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.20/recursion/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.20/recursion/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.30/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.30/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.30/execution/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.30/execution/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.30/execution/clipping/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.30/execution/clipping/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.30/linker/CMakeLists.txt /usr/lib64/piglit/tests/spec/glsl-1.30/linker/clipping/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glsl-1.30/linker/clipping/CMakeLists.txt /usr/lib64/piglit/tests/spec/glx_arb_create_context/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glx_arb_create_context/CMakeLists.txt /usr/lib64/piglit/tests/spec/glx_ext_import_context/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/glx_ext_import_context/CMakeLists.txt /usr/lib64/piglit/tests/spec/nv_conditional_render/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/nv_conditional_render/CMakeLists.txt /usr/lib64/piglit/tests/spec/nv_texture_barrier/CMakeLists.gl.txt /usr/lib64/piglit/tests/spec/nv_texture_barrier/CMakeLists.txt /usr/lib64/piglit/tests/spec/oes_compressed_etc1_rgb8_texture/CMakeLists.gles1.txt /usr/lib64/piglit/tests/spec/oes_compressed_etc1_rgb8_texture/CMakeLists.txt /usr/lib64/piglit/tests/spec/oes_compressed_paletted_texture/CMakeLists.gles1.txt /usr/lib64/piglit/tests/spec/oes_compressed_paletted_texture/CMakeLists.txt /usr/lib64/piglit/tests/spec/oes_draw_texture/CMakeLists.gles1.txt /usr/lib64/piglit/tests/spec/oes_draw_texture/CMakeLists.txt /usr/lib64/piglit/tests/texturing/CMakeLists.gl.txt /usr/lib64/piglit/tests/texturing/CMakeLists.txt /usr/lib64/piglit/tests/texturing/shaders/CMakeLists.gl.txt /usr/lib64/piglit/tests/texturing/shaders/CMakeLists.txt /usr/lib64/piglit/tests/util/CMakeLists.gl.txt /usr/lib64/piglit/tests/util/CMakeLists.gles1.txt /usr/lib64/piglit/tests/util/CMakeLists.gles2.txt /usr/lib64/piglit/tests/util/CMakeLists.txt Please remove these files if they're useless. Package Review ============== Key: - = N/A x = Pass ! = Fail ? = Not evaluated ==== C/C++ ==== [x]: MUST Header files in -devel subpackage, if present. [x]: MUST Package does not contain any libtool archives (.la) [x]: MUST Package does not contain kernel modules. [x]: MUST Package contains no static executables. [x]: MUST Rpath absent or only used for internal libs. [x]: MUST Package is not relocatable. ==== Generic ==== [x]: MUST Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: MUST Package successfully compiles and builds into binary rpms on at least one supported architecture. [x]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-]: MUST Buildroot is not present Note: Buildroot is not needed unless packager plans to package for EPEL5 [?]: MUST Package contains no bundled libraries. Pending FESCO approval glew bundling in: https://fedorahosted.org/fpc/ticket/132 If you could get a knowledgable MESA developer like ajax to take a look at the list of piglit patches in: https://bugzilla.redhat.com/show_bug.cgi?id=773021#c3 and make a recommendataion one way or another I'm sure it'll help. [x]: MUST Changelog in prescribed format. [-]: MUST Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) Note: Clean is needed only if supporting EPEL [x]: MUST Sources contain only permissible code or content. [-]: MUST Each %files section contains %defattr if rpm < 4.4 Note: defattr(....) present in %files section. This is OK if packaging for EPEL5. Otherwise not needed [x]: MUST Macros in Summary, %description expandable at SRPM build time. [x]: MUST Package requires other packages for directories it uses. [x]: MUST Package uses nothing in %doc for runtime. [x]: MUST Package is not known to require ExcludeArch. [x]: MUST Permissions on files are set properly. [x]: MUST Package does not contain duplicates in %files. [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. [-]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf is only needed if supporting EPEL5 [x]: MUST If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. Assuming you no longer ship GPL-2 twice. [x]: MUST License field in the package spec file matches the actual license. [x]: MUST Package consistently uses macros (instead of hard-coded directory names). [x]: MUST Package meets the Packaging Guidelines. [x]: MUST Package is named according to the Package Naming Guidelines. [x]: MUST Package does not generates any conflict. [x]: MUST Package obeys FHS, except libexecdir and /usr/target. [x]: MUST Package must own all directories that it creates. [x]: MUST Package does not own files or directories owned by other packages. [x]: MUST Package installs properly. [x]: MUST Requires correct, justified where necessary. [x]: MUST Rpmlint output is silent. All rpmlint output is justified: Incorrect FSF address shouldn't block package acceptance: piglit.x86_64: E: incorrect-fsf-address /usr/lib64/piglit/tests/glslparsertest/glsl2/xonotic-vs-generic-diffuse.vert "No manual page" shouldn't block package acceptance: piglit.x86_64: W: no-manual-page-for-binary piglit-summary-html piglit.x86_64: W: no-manual-page-for-binary piglit-run piglit.x86_64: W: no-manual-page-for-binary piglit-merge-results This is a git snapshot so "invalid-url for Source0" is acceptable: piglit.src: W: invalid-url Source0: piglit.tar.bz2 3 packages and 0 specfiles checked; 1 errors, 4 warnings. [-]: MUST Sources used to build the package match the upstream source, as provided in the spec URL. piglit.tar.bz2 : MD5SUM this package : a0c58448bcab817a610227c2bb86579c MD5SUM upstream package : upstream source not found This is a git snapshot. It's fine. [x]: MUST Spec file is legible and written in American English. [x]: MUST Spec file name must match the spec package %{name}, in the format %{name}.spec. [-]: MUST Package contains a SysV-style init script if in need of one. [x]: MUST File names are valid UTF-8. [x]: SHOULD Reviewer should test that the package builds in mock. [-]: SHOULD If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: SHOULD Dist tag is present. [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [x]: SHOULD Package functions as described. I tried: piglit-run /usr/lib64/piglit/tests/quick.tests result0 piglit-run /usr/lib64/piglit/tests/quick.tests result1 piglit-summary-html summary results0 results1 xdg-open summary/index.html piglit-merge-results results0/main results1/main and was able to get test resports and expected. Note that the piglit-summary-html report shows a few tests turning from fail to pass and vice versa just from running quick.tests twice on the same machine, so actaully interpreting test results for QA could get "interesting". [x]: SHOULD Package does not include license text files separate from upstream. [x]: SHOULD SourceX is a working URL. [-]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: SHOULD Package should compile and build into binary rpms on all supported architectures. [-]: SHOULD %check is present and all tests pass. [-]: SHOULD Packages should try to preserve timestamps of original installed files. [x]: SHOULD Spec use %global instead of %define. I'll approve this package once points one and two above and the glew bundling FESCO ticket is resolved.
(In reply to comment #10) > 1. You're shipping the GPL-2 file twice: > /usr/share/doc/piglit-1/GPL-2 > and > /usr/share/doc/piglit-1/documentation/GPL-2 > I suggest you remove /usr/share/doc/piglit-1/documentation/ as it contains only > GPL-2. I just don't install documentation directory now. > 2. You're shipping cmake files and the generated Makefile: > .... > Please remove these files if they're useless. Most of them removed (removing more would be probably not cost:benefit positive). > [?]: MUST Package contains no bundled libraries. > > Pending FESCO approval glew bundling in: https://fedorahosted.org/fpc/ticket/132 > If you could get a knowledgable MESA developer like ajax to take a look at the > list of piglit patches in: https://bugzilla.redhat.com/show_bug.cgi?id=773021#c3 > and make a recommendataion one way or another I'm sure it'll help. I have discussed that with ajax and airlied yesterday and they seemed generally positive to bundling. It is just too much work for no benefit (there are no secuirty issues with piglit). > [x]: MUST Changelog in prescribed format. > [-]: MUST Package has no %clean section with rm -rf %{buildroot} (or > $RPM_BUILD_ROOT) > Note: Clean is needed only if supporting EPEL And yes, of course, I want to support EPEL in the first place (we use piglit for the internal RH testing of RHEL). > Note that the piglit-summary-html report shows a few tests turning from fail to > pass and vice versa just from running quick.tests twice on the same machine, so > actaully interpreting test results for QA could get "interesting". That's interseting ... which ones? Isn't it a bug in piglit? > [x]: SHOULD Package should compile and build into binary rpms on all supported > architectures. Built in koji on all non-excluded RHEL-6 platforms. package piglit-1-0.12.git20120110Rf26fbd0.el6.src.rpm is in the same folder as previous ones, .spec file as well.
(In reply to comment #11) Your spec changes look fine. I'll approve this package now and assume FPC ticket 132 will get approved (I'm an optimist ;) > I have discussed that with ajax and airlied yesterday and they seemed generally > positive to bundling. It is just too much work for no benefit (there are no > secuirty issues with piglit). Maybe get them to leave some comments on https://fedorahosted.org/fpc/ticket/132 ? > > Note: Clean is needed only if supporting EPEL > And yes, of course, I want to support EPEL That "Note:" was just boilerplate text generated by the fedora-review tool, please ignore. > > piglit-summary-html shows unstable test results > That's interseting ... which ones? Isn't it a bug in piglit? I uploaded the report to http://scottt.tw/piglit-summary/index.html This is on an Intel Sandy Bridge machine running up to date F16: http://www.smolts.org/client/show_all/pub_19218c58-fdb9-4c7e-81d7-c0d1e5b8c17d Manually scanning over the test results, I see at least these results changing: fbo-3d fbo-sys-blit fbo-sys-blit bgra-sec-color-pointer scissor-clear glsl-deriv-varyings glsl-fs-color-matrix glsl-mat-attribute useprogram-flushverts-1 draw-elements-base-vertex-neg (I only read half way through the result summary page) I do believe these are bugs in piglit or they at least makes the testsuite as a whole hard to use for regression testing. APPROVED.
(In reply to comment #12) > > > piglit-summary-html shows unstable test results > > That's interseting ... which ones? Isn't it a bug in piglit? > > I uploaded the report to http://scottt.tw/piglit-summary/index.html > This is on an Intel Sandy Bridge machine running up to date F16: > http://www.smolts.org/client/show_all/pub_19218c58-fdb9-4c7e-81d7-c0d1e5b8c17d > > Manually scanning over the test results, I see at least these results changing: > fbo-3d > fbo-sys-blit > fbo-sys-blit > bgra-sec-color-pointer > scissor-clear > glsl-deriv-varyings > glsl-fs-color-matrix > glsl-mat-attribute > useprogram-flushverts-1 > draw-elements-base-vertex-neg > (I only read half way through the result summary page) > I do believe these are bugs in piglit or they at least makes the testsuite as a > whole hard to use for regression testing. Will you write about it to the piglit list, or should I? I could do it, but I don't want to use my name on your discovery (and you would be probably better qualified in explaining what's going on).
(In reply to comment #13) > Will you write about it to the piglit list, or should I? Just sent out the email to piglit list. Could you also try running: piglit-run /usr/lib64/piglit/tests/quick.tests result0 piglit-run /usr/lib64/piglit/tests/quick.tests result1 piglit-summary-html summary results0 results1 xdg-open summary/index.html and see if you get tests that change from fail to pass etc?
(In reply to comment #14) > (In reply to comment #13) > > Will you write about it to the piglit list, or should I? > > Just sent out the email to piglit list. Could you also try running: > piglit-run /usr/lib64/piglit/tests/quick.tests result0 > piglit-run /usr/lib64/piglit/tests/quick.tests result1 > piglit-summary-html summary results0 results1 > xdg-open summary/index.html > and see if you get tests that change from fail to pass etc? Are you absolutely sure you haven't done anything between two runs of tests which could affect the result? For me the only difference is one test which aborted mitmanek:~ $ json_diff -i result /tmp/result01/main /tmp/result02/main { "_update": { "tests": { "_update": { "glx/glx-tfp": { "_update": { "result": "abort" } } } } } } mitmanek:~ $ (json_diff is my script http://pypi.python.org/pypi/json_diff, and packaged in Fedora) The response from the aborted test is (the first run it passed without any message): glx-tfp: xcb_io.c:176: process_responses: Assertion `!(req && current_request && !(((long) (req->sequence) - (long) (current_request)) <= 0))' failed.
(In reply to comment #15) > Are you absolutely sure you haven't done anything between two runs of tests > which could affect the result? Quote sure. I just issued the same piglit-run command from bash history on an idle machine. I suspect the difference is due to us running on different hardware. python ~/json_diff/lib/python2.7/site-packages/json_diff.py -i result Q0/main Q1/main { "_update": { "tests": { "_update": { "fbo/fbo-sys-sub-blit": { "_update": { "result": "pass" } }, "fbo/fbo-sys-blit": { "_update": { "result": "pass" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-array-mat4-index-col-row-wr": { "_update": { "result": "fail" } }, "texturing/depth-tex-modes-rg": { "_update": { "result": "pass" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-mat2-col-wr": { "_update": { "result": "fail" } }, "spec/glsl-1.10/execution/variable-indexing/vs-temp-array-mat4-index-col-row-wr": { "_update": { "result": "fail" } }, "spec/ARB_shader_texture_lod/execution/arb_shader_texture_lod-texgrad": { "_update": { "result": "pass" } }, "general/scissor-clear": { "_update": { "result": "pass" } }, "spec/glsl-1.10/execution/variable-indexing/vs-temp-array-mat2-index-col-row-wr": { "_update": { "result": "fail" } }, "spec/glsl-1.10/execution/variable-indexing/vs-temp-array-mat4-index-row-wr": { "_update": { "result": "pass" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-mat3-col-row-wr": { "_update": { "result": "pass" } }, "spec/ARB_draw_elements_base_vertex/draw-elements-base-vertex": { "_update": { "result": "pass" } }, "general/read-front": { "_update": { "result": "fail" } }, "texturing/tex-miplevel-selection": { "_update": { "result": "pass" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-array-mat2-index-col-wr": { "_update": { "result": "pass" } }, "shaders/glsl-lod-bias": { "_update": { "result": "fail" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-array-mat4-index-col-wr": { "_update": { "result": "fail" } }, "spec/ARB_texture_rg/fbo-clear-formats": { "_update": { "result": "fail" } }, "spec/glsl-1.10/execution/variable-indexing/vs-temp-array-mat4-col-row-wr": { "_update": { "result": "fail" } }, "shaders/useprogram-flushverts-1": { "_update": { "result": "fail" } }, "shaders/sso-uniforms-01": { "_update": { "result": "fail" } }, "spec/glsl-1.20/execution/variable-indexing/vs-temp-mat2-col-row-wr": { "_update": { "result": "fail" } }, "shaders/vp-address-01": { "_update": { "result": "fail" } }, "spec/EXT_texture_sRGB/fbo-generatemipmap-formats": { "_update": { "result": "pass" } } } } } } (json_diff-1.1.0-1.fc16.noarch gave me a trace back expecting some local variable "opt" to be a dictionary when it's an old style class. I cloned from gitorious instead)
(In reply to comment #16) > (json_diff-1.1.0-1.fc16.noarch gave me a trace back expecting some local > variable "opt" to be a dictionary when it's an old style class. I cloned from > gitorious instead) Damn. I have forgotten to push updates to Bodhi. It should be there now. OK, so where we are with the package review? Just waiting on the Fedora Packaging Committee?
(In reply to comment #17) > OK, so where we are with the package review? Just waiting on the Fedora > Packaging Committee? I've already set the "fedora-review" flag to "+" and consider this package APPROVED.
Oh well, https://fedorahosted.org/fpc/ticket/132#comment:4 CLOSED/WONTFIX