[Mesa-dev] [PATCH] mesa: Remove the generated glapi from source control, and just build it.

Kenneth Graunke kenneth at whitecape.org
Wed May 23 14:09:28 PDT 2012


On 05/23/2012 01:33 PM, Ian Romanick wrote:
> On 05/23/2012 12:45 PM, Eric Anholt wrote:
>> Mesa already always depends on python to build.  The checked in
>> changes are not reviewed (because any trivial change rewrites the
>> world).  We also have been pushing commits between xml change and
>> regen where at-build-time xml-generated code disagrees with committed
>> xml-generated code.  And worst of all, sometimes we ("I") check in
>> *stale* xml-generated code.
> 
> <broken_record>
> Though the person making the changes will typically review them.

I disagree.  I recently added a new extension (in a patch I haven't sent
out yet) that added a few new API entrypoints and enums, and discovered
that running the generator(*) caused:

 11 files changed, 6691 insertions(+), 5692 deletions(-)

I haven't looked at any of this generated code before, I don't know how
it works, or have any sense of what should change.  I just know that
some absurd amount of data changed and there's no way in hell I'm
reading it.

I'm also not sure whether all of that was caused by my changes, or
catching up after someone forgot to re-run it.

(*) Technically, the build system was so broken that many files did
*not* get automatically regenerated by 'make mesa' in glapi/gen.  I had
to manually use git log to find two or three previous "regenerate the
world" commits, get a list of the changed files, and manually delete
them prior to running the generator.  And after that failed, I finally
had to go find all files that said "DO NOT EDIT" and throw those out.
Then un-throw out the generators themselves.

If "make clean" or "git clean" threw out the generated files, I would've
had a lot more confidence that everything was up to date.

> I renew my objection that making tiny changes to the generator scripts can
> result in catastrophic changes in the generated code.  We've encountered
> several situations where changing one part of the generator causes sizes
> of broad categories of GLX protocol messages to be miscalculated.
> git-diff makes this easy to notice.  "Wait, why did indirect_size.c
> change at all?  That has nothing to do with the change I intended."

I believe -you- review the generated code because you understand the
generators enough to know what a reasonable delta is.  I imagine most
people wouldn't realize that indirect_size.c shouldn't change.

Obviously, if you're making changes to the generator themselves, you
should manually diff the output to make sure it's okay.  If you're
adding new XML descriptions of an API, then I hope it isn't that
catastrophically fragile.

> Note also that some of the generated code is platform-specific and gets
> little testing.  How often does anyone test the C-based dispatch code?
> Or the SPARC dispatch code?  Or even the 32-bit x86 dispatch code?

Regenerating SPARC dispatch incorrectly = fail.  Not regenerating it at
all also = fail.  Untested platforms may break.  That's bad, but
suggests that we need more automated testing across platforms, not that
we need to have a bunch of generated C files tracked in git.

> I know Paul asserts people can diff the files by hand.  I assert that,
> while they can, nobody will do that ever.  People will look for changes
> in the files that they expected to change.  Given the size of the
> potential set of changed files, I expect that change in unexpected files
> will go unnoticed.

Agreed.

> Nobody runs indirect rendering tests.  Therefore we have no testing of
> code that we might not even notice we were changing.  That seems just
> about as awful as possible.
> </broken_record>

So we should do automated regression testing on indirect rendering every
week or so.  This sounds like the perfect thing to throw on a lab, and
actual testing is far more likely to actually find bugs than people
manually diffing piles of unreadable C code.

> I fully agree that the current situation sucks.  However, the new
> situation will also suck.  I reserve the right to say "I informed you
> thusly" the next time one of these kinds of bugs occurs. :p

*100%* of the Khronos ES2 conformance suite fails *right now* and
bisects to a commit that Eric made to add some new content to GL3.xml.
I verified that commit several times in disbelief.  The problem was that
the build system didn't run the generators correctly and some rubbish
got committed that routed glShaderSource to _mesa_LinkProgram.

You're welcome to say "I told you so."  I'm fine with that.  There may
be problems with not including the generated files, but there are
serious problems today that this series claims to address.


More information about the mesa-dev mailing list