On 12 March 2012 15:08, Jose Fonseca <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">----- 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 +++++++++++ </div>Do any of these files really need to be commited, given the source .spec have been committed too? </blockquote><div> That's a good question, and I don't really know the best answer. See below for more discussion. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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). </blockquote><div> 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. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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 </blockquote></div> 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). 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. (3) Don't check the source files into CMake; instead download them and manually rebuild whenever upstream changes. CON: the source files from <a href="http://opengl.org">opengl.org</a> 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. 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?