libshadow + fb fix

[Adam Jackson]

On Tuesday 23 May 2006 14:59, Enrico Weigelt wrote:
> * Adam Jackson <ajax at nwnk.net> wrote:
> > On Wednesday 17 May 2006 10:39, Enrico Weigelt wrote:
> > > This fix adds some (non-compiled) file from libfd to libshadow,
> > > so the misssing symbols can be resolved.
> >
> > Absolutely not.
> >
> > Notice the big chunks of fbcmap.c under conditional compilation.
> > It has to be compiled once per DDX to match the #defines for each one.
>
> hmm, so it has to be added to the individual DDX'es instead of libfb ?

Which as Daniel pointed out a while ago [1] is exactly what is done, yes.  
He's being a bit short with you but he's also correct.  Given that he did the 
initial modular XFree86 DDX build system bringup basically unassisted, he 
generally knows what he's talking about here.

So when we say "no" with no explanation, it's generally because there's a 
fundamentally flawed assumption at work and we're trying to avoid people 
going down that route again.  In this case:

> AFAICS there are exactly two cases in fbcmap.c:
> a) XFree86Server is defined
> b) -''- not defined
>
> So we could easily split it off into two files and add just the right
> file to the individual DDX'es.

No.

That would mask the real problem, which is that the #defines shouldn't exist.  
That they _do_ exist, is because the different DDXes treat colormap 
initialization differently, and someone needs to figure out why and fix it.

Simply picking one of the #ifdef branches is not good enough.  We tried that 
several times, unintentionally, first in debrix and then in the 7.0 bringup, 
and it doesn't work.  The code surrounding the ifdefs needs to be fixed too.

In general, the stupid ifdefs are there for a reason.  That reason might not 
be a _pleasant_ reason, and it's likely that code fixes are required, but we 
didn't just make the build system cracked out because we're dorks like that.  
The code has rotted significantly over the past 20 years and it's really 
really hard to both a) fix it, and b) keep it working while doing so.  N-way 
compilation of code in the DIX layer is a smell, and in this case we've 
reflected the code smell (varying DDX handling of colormaps) in the build 
smell (N-way compilation).  This was chosen because in the absence of code 
fixes it's the Correct thing to do.  Really.

So, yeah, fbcmap is absolutely not going to get built into libshadow.

[1] http://lists.freedesktop.org/archives/xorg-modular/2006-May/001043.html

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.x.org/archives/xorg-modular/attachments/20060525/801d94ae/attachment.pgp