[Mesa-dev] [PATCH] glapi: Fix incorrect enum value.

[Chad Versace]

On 02/22/2012 04:29 PM, Kenneth Graunke wrote:
> On 02/22/2012 04:06 PM, Chad Versace wrote:
>> On 02/22/2012 02:22 PM, Ian Romanick wrote:
>>> On 02/22/2012 02:17 PM, Paul Berry wrote:
>>>> On 22 February 2012 13:52, Ian Romanick<idr at freedesktop.org
>>>> <mailto:idr at freedesktop.org>>  wrote:
>>>>
>>>>      On 02/22/2012 01:41 PM, Paul Berry wrote:
>>>>
>>>>            From
>>>>          http://www.opengl.org/__registry/specs/ARB/seamless___cube_map.txt
>>>>          <http://www.opengl.org/registry/specs/ARB/seamless_cube_map.txt>:
>>>>
>>>>               Accepted by the<cap>   parameter of Enable, Disable and
>>>>          IsEnabled,
>>>>               and by the<pname>   parameter of GetBooleanv, GetIntegerv,
>>>>          GetFloatv
>>>>               and GetDoublev:
>>>>
>>>>               TEXTURE_CUBE_MAP_SEAMLESS                   0x884F
>>>>
>>>>
>>>>      Oops.  That was my typo.  You'll also have to regenerate the various
>>>>      files that depend on the XML definitions.  I think this change
>>>>      should only cause changes in src/mesa/main/enums.c.
>>>>
>>>>      Reviewed-by: Ian Romanick<ian.d.romanick at intel.com
>>>>      <mailto:ian.d.romanick at intel.com>>
>>>>
>>>>
>>>> Oops.  I didn't realize that those files weren't built automatically.
>>>> I'll send an updated patch.
>>>
>>> Dear god, no!  The patch will be huge.
>>
>>>> Is there any good reason why we don't automatically generate files like
>>>> enum.c as part of the mesa build process?  The comment at the top of
>>>> src/mapi/glapi/gen/Makefile says "This file isn't used during a normal
>>>> compilation since we don't want to require Python in order to compile
>>>> Mesa."  But I don't think that makes sense anymore, because Python is
>>>> required to build files like src/mapi/es2api/glapi_mapi_tmp.h, as well
>>>> as some files in src/glsl.
>>>>
>>>> In point of fact, it seems really strange that a file like
>>>> src/mapi/es2api/glapi_mapi_tmp.h is autogenerated during the build
>>>> process, but src/mesa/main/enums.c isn't, since both files are built
>>>> from the same set of xml sources.
>>>
>>> A couple reasons:
>>
>> I really want to see all *build* artifacts removed from Mesa's *source* control. I recall the
>> pain of having the bison and flex output managed by git, and I don't like continuing that fallacy.
>>
>>> 1. The generated files really, really, really should be in git so that accidental changes will be noticed.  This has bitten us a couple times.
>>
>> Wouldn't `git log *.xml *.py` also alert you that the generated headers have changed?
> 
> Not if you remove them from git...

Since the generated headers are exclusively generated by the xml and python scripts in src/mapi, I insist that
`git log *.xml *.py` is a very good indicator that the generated headers have changed.

>>> 2. Changes to the XML files frequently precipitate changes to files in the xserver.  There's no way to automatically handle that.
>>
>> I don't understand how 2 affects this discussion. Is it that the xserver build relies on these generated headers? If so, then
>> we can move the generated headers to the xserver repo and merrily remove these build artifacts from Mesa's source control.
>> Afterwards, it would be the xserver's devs responsibility to manually update their headers, or fix their makefiles to gen them
>> at build time.
> 
> I'll let Ian explain it, but essentially two things are generated from these XML files:
> 
> 1. Mesa headers/code
> 2. X server headers/code
> 
> So if you change them, you still need to manually regenerate the X server code and commit those to the xserver repository.  In other words, integrating this into the build system doesn't eliminate manual steps (and perhaps gives you the illusion that everything happens automatically).
> 
> It would be nice to solve this problem, but I'm not sure how.

I don't understand your rebuttal to my point 2. Actually, it seems that you agree with my proposal.
To "manually regenerate the X server code and commit those to the xserver repository" is exactly what I suggested.
Is there any reason why this proposal is unacceptable? It just moves the burden of manually updating the headers
to the project that will consume those headers.
 
>>> 3. Several of the scripts take a really, really long time to run.  I'm not eager to add a few minutes to my clean-build times.
>>
>> $ cd src/mapi/glapi/gen
>> $ make mesa
>> real    0m5.634s
>>
>> I don't think an adding 5 seconds to a clean build is cause for concern if the addition allows us
>> to remove build artifacts from source control.