On 02/22/2012 04:14 PM, Paul Berry wrote:
On 22 February 2012 16:06, Chad Versace <chad.vers...@linux.intel.com <mailto:chad.vers...@linux.intel.com>> 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 <i...@freedesktop.org <mailto:i...@freedesktop.org> >> <mailto:i...@freedesktop.org <mailto:i...@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.roman...@intel.com <mailto:ian.d.roman...@intel.com> >> <mailto:ian.d.roman...@intel.com <mailto:ian.d.roman...@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? > > 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. > 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. I agree with this. In my experience, including build outputs in source control is nearly always a mistake; as a rule, I'd rather exclude build artifacts from source control except in situations where there is no feasible alternative. IMHO this doesn't seem like one of those situations.
I am strongly opposed to not including generated code in source control. A lot of the generated code doesn't get a lot of testing (e.g., GLX protocol code), so subtle breaks in generated code may not be noticed for too long. We've had several cases over the years where seemingly innocuous changes to generator scripts or XML cause, literally, dozens of functions to be generated completely broken. Some cases have included calculated packet sizes being wrong in all functions of a certain category, all functions of a certain category having empty bodies, and dispatch code for other CPUs being broken or empty.
Having the generated files tracked by GIT at least makes it a lot easier to find the break. When someone notices that src/glx/indirect.c has errors, they can git-log that file instead of having to either bisect or view the git-log of all the files that contribute to that file being built (I'd estimate that there are about a dozen).
Having the person making the commits see the changes also helps prevent bad changes from ever landing. If you haven't added any new functions, it should set of alarm bells when you're committing changes to src/mapi/glapi/glapi_sparc.S. That extra safeguard alone has saved my ass several times.
I'm not willing to give up that safety net without something to replace it. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
