The Xorg/Xprint CVS trunk currently fails to build with |PrintOnlyServer| set to |NO|.
Created attachment 2193 [details] [review] Patch for 2005-03-23-trunk
Patch checked-in... /cvs/xorg/xc/ChangeLog,v <-- xc/ChangeLog new revision: 1.831; previous revision: 1.830 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/Imakefile,v <-- xc/programs/Xserver/Imakefile new revision: 1.30; previous revision: 1.29 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/Xprint/DiPrint.h,v <-- xc/programs/Xserver/Xprint/DiPrint.h new revision: 1.3; previous revision: 1.2 /cvs/xorg/xc/programs/Xserver/Xprint/Imakefile,v <-- xc/programs/Xserver/Xprint/Imakefile new revision: 1.6; previous revision: 1.5 /cvs/xorg/xc/programs/Xserver/Xprint/Init.c,v <-- xc/programs/Xserver/Xprint/Init.c new revision: 1.10; previous revision: 1.9 /cvs/xorg/xc/programs/Xserver/Xprint/ddxInit.c,v <-- xc/programs/Xserver/Xprint/ddxInit.c new revision: 1.5; previous revision: 1.4 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/dix/Imakefile,v <-- xc/programs/Xserver/dix/Imakefile new revision: 1.5; previous revision: 1.4 /cvs/xorg/xc/programs/Xserver/dix/main.c,v <-- xc/programs/Xserver/dix/main.c new revision: 1.6; previous revision: 1.5 /cvs/xorg/xc/programs/Xserver/dix/xpstubs.c,v <-- xc/programs/Xserver/dix/xpstubs.c new revision: 1.3; previous revision: 1.2 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/os/Imakefile,v <-- xc/programs/Xserver/os/Imakefile new revision: 1.8; previous revision: 1.7 /cvs/xorg/xc/programs/Xserver/os/utils.c,v <-- xc/programs/Xserver/os/utils.c new revision: 1.12; previous revision: 1.11 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. Mailing the commit message to xorg-commit@lists.freedesktop.org... ... marking bug as FIXED.
This fix is objectionable. 1. There should be no dependency created from DIX to Xprint headers. 2. It's not acceptable to add calls into Xprint and stuffing them with stubs when not needed. If new calls to Xprint are absolutely necessary then they should be shielded by #ifdefs. There is no explanation why these calls are necessary. 3. xpstubs.c exists to stub calls in the font library which lives outside the Server and therefore needs to be shareable. The unfortunate addition of XprintOptions() that was not properly reviewed at the time it was made doesn't change this siutation. This function should also be properly shielded instead. If you plan to reduce the number of ifdefs and replace them with stubs to make the DIX more universal you should bring up this issue on the development list for discussion outlining your plans.
(In reply to comment #3) > This fix is objectionable. > 1. There should be no dependency created from DIX to Xprint headers. Which dependicy do you mean exactly here (seems we missed that issue in the phone call 10mins ago... ;-( ) ? > 2. It's not acceptable to add calls into Xprint and stuffing them with > stubs when not needed. > If new calls to Xprint are absolutely necessary then they should be shielded > by #ifdefs. There is no explanation why these calls are necessary. This is neccesary as otherwise Xprt cannot be build anymore at the same time together with a Xorg server with loadeable print modules. Additionally the Xorg server needs a way to choose between "video", "print" and "video+print" mode so the change in xc/programs/Xserver/dix/main.c ... -- snip -- InitOutput(&screenInfo, argc, argv); + PrinterInitOutput(&screenInfo, argc, argv); + -- snip -- ... is required to add later a |switch()|-statement to allow a choice here (so adding a wrapper *.o in Xprint/ isn't an option here, |InitOutput()| and |PrinterInitOutput()| need to be seperately callable). > 3. xpstubs.c exists to stub calls in the font library which lives outside the > Server and therefore needs to be shareable. The unfortunate addition of > XprintOptions() that was not properly reviewed at the time it was made > doesn't change this siutation. This function should also be properly > shielded instead. Again, it cannot be shielded as the matching objects are shared within one build between multiple Xservers. > If you plan to reduce the number of ifdefs and replace them with stubs to make > the DIX more universal you should bring up this issue on the development list > for discussion outlining your plans. I've outlined it briefly in xorg@freedesktop.org (and followed exactly the requirements set my Mike Marris (see http://lists.freedesktop.org/pipermail/xorg/2005-February/006290.html)), described it more in detail (actually this isn't the work for the unified video+print Xserver, just a preparation step to get the old |PrintOnlyServer| build option working again and then start the work on a unified Xserver with loadable modules) in the new xorg-arch list and the patches itself cycled for several weeks forth and back in the Xprint mailingslist for review (... and I thought that I did everything right this time... ;-( ).
Created attachment 2294 [details] [review] Patch to decouple the DIX dependicy to the DiPrint.h header Egbert: The attached patch should cure the problem that DIX depends on the xc/programs/Xserver/Xprint/DiPrint.h via introducing a new CPP symbol |DIPRINT| (Device Independent PRINT) to enable/disable the Xprint-specific parts in DIX.
Comment on attachment 2294 [details] [review] Patch to decouple the DIX dependicy to the DiPrint.h header Requesting review...
Created attachment 2300 [details] [review] Alternative patch ro decouple the DIX dependicy from the DtPrint.h header Alternative solution based on Egbert Eich's version (I only hacked it until it was building&&working on all build combinations).
I am taking the alternative patch for now to have a way to conditionalise the usage of the headers...
Created attachment 2526 [details] [review] Patch for 2005-04-23-trunk to make the print headers conditional
attachment #2526 [details] [review] checked-in... /cvs/xorg/xc/ChangeLog,v <-- xc/ChangeLog new revision: 1.898; previous revision: 1.897 /cvs/xorg/xc/config/cf/X11.tmpl,v <-- xc/config/cf/X11.tmpl new revision: 1.47; previous revision: 1.46 /cvs/xorg/xc/programs/Xserver/Xext/Imakefile,v <-- xc/programs/Xserver/Xext/Imakefile new revision: 1.9; previous revision: 1.8 /cvs/xorg/xc/programs/Xserver/dix/Imakefile,v <-- xc/programs/Xserver/dix/Imakefile new revision: 1.6; previous revision: 1.5 /cvs/xorg/xc/programs/Xserver/dix/main.c,v <-- xc/programs/Xserver/dix/main.c new revision: 1.10; previous revision: 1.9 /cvs/xorg/xc/programs/Xserver/dix/xpstubs.c,v <-- xc/programs/Xserver/dix/xpstubs.c new revision: 1.4; previous revision: 1.3 /cvs/xorg/xc/programs/Xserver/mi/miinitext.c,v <-- xc/programs/Xserver/mi/miinitext.c new revision: 1.19; previous revision: 1.18 /cvs/xorg/xc/programs/Xserver/os/utils.c,v <-- xc/programs/Xserver/os/utils.c new revision: 1.14; previous revision: 1.13 Mailing the commit message to xorg-commit@lists.freedesktop.org... ... marking bug as FIXED (again, please reopen if there are any issues left (hopefully not...)).
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.