Bug 691510

Summary: issues with x11 device as dynamic module
Product: Ghostscript Reporter: Hin-Tak Leung <htl10>
Component: Build ProcessAssignee: Default assignee <ghostpdl-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: till.kamppeter
Priority: P4    
Version: master   
Hardware: Other   
OS: Linux   
Customer: Word Size: ---
Attachments: patch addressing those issues mentioned.

Description Hin-Tak Leung 2010-07-30 01:00:40 UTC
Created attachment 6579 [details]
patch addressing those issues mentioned.

While looking at bug 690009, I noted a few issues with building the x11 device as dynamic module:

all object files destined for shared libraries
 needed to be compiled with -fPIC but this is not the case for gdevxcmp, 
gsparamx and gdevemap. The first one is easiest to fix, just switching from
$(GLCC) to $(GLCCSHARED). The other two are more difficult as they
are required by other parts to be built as standard.

http://bugs.ghostscript.com/show_bug.cgi?id=690009#c10
http://bugs.ghostscript.com/show_bug.cgi?id=690009#c11
http://bugs.ghostscript.com/show_bug.cgi?id=690009#c12

Also, all used of $(CC_SHARED) are for object files, rather than linking, so  DYNAMIC_CFLAGS is more appropiate than DYNAMIC_LDLAGS.

The attached patch is one way of addressing the problems with gsparamx and gdevemap - relying on the first one being available as part of psdf and inserting the 2nd one bodily into where it is used for the 2nd; there are probably better ways of doing the same thing.
Comment 1 Till Kamppeter 2010-07-30 07:06:50 UTC
The patch does not delete the original gdevemap. We should avoid that the same code piece exists more than once in the source code. Imaging someone fixes a bug in it and forgets the second incarnation.
Comment 2 Till Kamppeter 2010-07-30 07:23:40 UTC
Can you also add a patch for the "easiest" gdevxcmp fix? Thanks.
Comment 3 Till Kamppeter 2010-07-30 08:25:05 UTC
Hin-Tak, forget my comment #2, I have seen now that you have already addressed gdevxcmp in the patch.
Comment 4 Till Kamppeter 2010-07-30 08:28:47 UTC
Found the comment about gdevemap in the other bug. I will test the patch now on an actual Linux box and if it works commit it.
Comment 5 Till Kamppeter 2010-07-30 08:58:51 UTC
Fixed in SVN rev 11555. Thanks for the patch.
Comment 6 Hin-Tak Leung 2010-07-30 15:12:11 UTC
(In reply to comment #1)
> The patch does not delete the original gdevemap. We should avoid that the same
> code piece exists more than once in the source code. Imaging someone fixes a
> bug in it and forgets the second incarnation.

It shouldn't be deleted - as it is used by mac classic build and windows build to load xfonts. The mapping has not changed for 10 years and unlike to change, but I agree it is not elegant.

I have thought of another alternative of addressing gsparamx and gdevemap being shared with non-unix builds in core. One can have a smaller list of objects for the shared x11 target than the build-in x11 target. e.g.

x11_shared=<most of OBJ>
x11_=$(x11_shared) $(GLOBJ)gdevemap.$(OBJ) $(GLOBJ)gsparamx.$(OBJ)

$(GLOBJ)X11.so : $(x11alt_) $(x11_shared)
$(CCLD) $(LDFLAGS) -shared -o $(GLOBJ)X11.so $(x11alt_) $(x11_shared) \
-L/usr/X11R6/lib -lXt -lSM -lICE -lXext -lX11 $(XLIBDIRS)

It is marginally better in that it does't involve duplicating the body of gdevemap, but make the makefile a bit more complicated.