Re: [Piglit] [PATCH 00/14] Move main() from libpiglitutil into each test

2012-06-15 Thread Chad Versace
On 06/15/2012 10:27 AM, Brian Paul wrote:
 On Fri, Jun 15, 2012 at 11:18 AM, Chad Versace
 chad.vers...@linux.intel.com wrote:
 On 06/13/2012 10:25 AM, Chad Versace wrote:
 On 06/13/2012 09:06 AM, Brian Paul wrote:
 On 06/12/2012 05:02 PM, Chad Versace wrote:

 Could you check this into a branch first for additional testing (MSVC, 
 mingw32, etc)?

 Sure. The series is on my separate-main-v11 branch. I will be posting a 
 v12, v13, etc
 as I incorporate review comments.

 Did you also want the series checked into a branch in the origin repo?

 Brian, do you need more time to test the series on Windows? I wanted to 
 commit this soon
 if you didn't find any problems.
 
 I just grabbed your branch now.  It's failing with mingw32 because of
 the stdint.h header.  I think I remember you and Jose talking about
 that.  Is that a patch you can apply to your branch?
 
 [  0%] Building C object
 target_api/gl/tests/util/CMakeFiles/piglitutil.dir/fdo-bitmap.c.obj
 In file included from
 /home/brian/projects/piglit-chad/piglit/tests/util/fdo-bitmap.c:4:
 /home/brian/projects/piglit-chad/piglit/include/msvc/c99/stdint.h:33:2:
 error: #error Use this header only with Microsoft Visual C++
 compilers!

Hmm... That branch has the fix that Jose suggested.

I've listed the guilty bit of code below. I'm not familiar with Windows,
so I can't diagnose it. Do you see any apparent problem?

file: CMakeLists.txt
if (NOT MSVC)
...
else ()
include_directories(include/msvc/c99)
...
endif()

file: tests/util/piglit-util.h
// This include is not in an #ifdef.
#include stdint.h


Since include/msvc/c99 is added to the include directories only when MSVC is
defined, I don't understand why mingw32 is including it.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 00/14] Move main() from libpiglitutil into each test

2012-06-13 Thread Brian Paul

On 06/12/2012 05:02 PM, Chad Versace wrote:

The goal that this series achieves is to replace the main() function in
piglit-framework.c with a new function, piglit_gl_test_run(), that takes as
its input a data structure describing the test initialization info.  This goal
is a stepping stone towards a larger set of goals discussed earlier this month 
[1].
In the future, the piglit_gl_test_info struct will also contain a declarative
desciption of each test's requirements on GL context flavor, extensions, and
window systems.

There is another reason, in addition to those discussed in [1], for why main()
should be moved out of libpiglitutil.  The EGL tests, GLX tests, and glean all
define main(), yet also link to libpiglitutil.  Surprisingly, the linker
doesn't complain. However, the situation is fragile. If you modify the
libraries or tests in a way that changes the order in which the linker
resolves things, link failure occurs.  The proper solution is not to tiptoe
around the linker; it's to remove main() from the library.

If you want to skip to the important stuff, see patches 11-14. All the other
patches are just prerequisite cleanups that prepare for those last 4.

This series taks a few detours, cleans out the closet, and reorganizes your
kitchen junk drawer.  Often, you must clean up the house before remodeling it.
I feel that all the little detours were needed and that Piglit really benefits
from them. I wanted to clean up a lot more, but I restrained myself only to
the cleanups that directly benefited the series' stated goal.

No regresssions found on Sandybridge.


Could you check this into a branch first for additional testing (MSVC, 
mingw32, etc)?


-Brian

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit