[Piglit] [PATCH v2 08/13] piglit-dispatch: Add generated files.

[Jose Fonseca]

----- Original Message ----- 

> On 12 March 2012 15:08, Jose Fonseca < jfonseca at vmware.com > wrote:

> > ----- Original Message -----
> 
> > > This patch adds the files glapi/glapi.json,
> 
> > > tests/util/generated_dispatch.c, and
> > > tests/util/generated_dispatch.h,
> 
> > > which were generated using the command "make
> > > piglit_dispatch_gen".
> 
> > > ---
> 
> > > glapi/glapi.json |55751
> 
> > > +++++++++++++++++++++++++++++++++++++++
> 
> > > tests/util/generated_dispatch.c |39920
> > > ++++++++++++++++++++++++++++
> 
> > > tests/util/generated_dispatch.h |14993 +++++++++++
> 

> > Do any of these files really need to be commited, given the source
> > .spec have been committed too?
> 

> That's a good question, and I don't really know the best answer. See
> below for more discussion.

> > If so, have you ensured that everytime they are regenerate they
> > (Care
> > must be taken with python dictionaries as they easily lead to
> > variable ordering).
> 

> Yes, I was careful about this in the code generation script. All the
> output of the code generator is sorted so that it will happen in a
> consistent order and so that the diffs will be reasonable when the
> generated code changes.

> > But I feel that either the derived result is committed (and the
> > source is left out and manually downloaded when upstream changes)
> > or
> > the sources (and the derived files generated during the build).
> > Including both seems redundant.
> 

> > Jose
> 

> Yeah, I agree that what I've put on the mailing list seems redundant.
> It seems like we have three options on the table:

> (1) Leave the patches as they are.

> CON: Seems redundant.

> CON: There's a danger that someone will edit the source files (or the
> generator script) and forget to re-run code generation.

> (2) Don't check the generated code into CMake; instead generate it at
> build time.

> PRO: Since this forces everyone to do code generation all the time,
> it will help ensure that the code generation script is platform
> independent and doesn't accumulate bugs. Also it ensures that I'm
> not the only person who knows how to run the code generator and
> remembers to do so :)

> PRO: There's no danger of someone accidentally editing the generated
> files and checking in the results.

> CON: I'm not 100% sure how to program the correct dependencies into
> CMake, to ensure that (a) the code generation script runs before any
> C files are built, and (b) all C files are rebuilt after the
> generated code changes. (Chad helped me get this set up on an early
> draft of the code but I was unable to keep it working properly).

Apitrace does a lot of code generation with cmake. Feel free to look into https://github.com/apitrace/apitrace/blob/master/CMakeLists.txt as inspiration. For example:

    add_custom_command (
        OUTPUT glxtrace.cpp
        COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/glxtrace.py > ${CMAKE_CURRENT_BINARY_DIR}/glxtrace.cpp
        DEPENDS glxtrace.py gltrace.py trace.py specs/glxapi.py specs/glapi.py specs/glparams.py specs/gltypes.py specs/stdapi.py
    )

You jut need to ensure that all DEPENDS args are there. When generating .h files, you also need to ensure that ${CMAKE_CURRENT_BINARY_DIR} is in the include path.

> CON: When changing the code generation scripts (or the .spec and .tm
> files), more work will be required to manually verify that the
> generated code is correct, because the generated code won't show up
> in git diffs.

It's a test suite. I suppose tests would suddenly fail/skip if there's a bug here, so it should be easy to catch this.

> (3) Don't check the source files into CMake; instead download them
> and manually rebuild whenever upstream changes.

> CON: the source files from opengl.org contained errors that I had to
> manually correct, and some information was missing (see patches 5
> and 6 in the series); if we take this approach then I'm not sure
> where to store corrections and additions.

Fair enough. (3) doesn't work.

This is also the reason why I don't code generate from Khronos .spec .tm in apitrace neither, as apitrace needs a lot of extra manual info that's not included there.

> CON: if the OpenGL folks ever decide to drop the .spec and .tm files
> from their web site, we will have to scrounge to find copies of
> them.


> Personally I have a slight preference for sticking with approach (1)
> for now (in the interest of getting these patches into the codebase
> soon) and experimenting with approach (2) as a follow-up patch
> series. But I could be talked into changing my tune. I'd like to
> hear more opinions.

> Ian, I recall that you had some strong opinions about whether
> generated code should be stored in source control. Would you like to
> weigh in on the discussion?

-- 
Jose