Bug 2792 - Build fails with |PrintOnlyServer| set to |NO|
Summary: Build fails with |PrintOnlyServer| set to |NO|
Status: RESOLVED FIXED
Alias: None
Product: xprint
Classification: Unclassified
Component: Build config (show other bugs)
Version: unspecified
Hardware: All All
: high major
Assignee: Roland Mainz
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-23 02:00 UTC by Roland Mainz
Modified: 2005-04-23 01:11 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch for 2005-03-23-trunk (30.52 KB, patch)
2005-03-23 11:56 UTC, Roland Mainz
no flags Details | Splinter Review
Patch to decouple the DIX dependicy to the DiPrint.h header (9.48 KB, patch)
2005-04-02 19:46 UTC, Roland Mainz
no flags Details | Splinter Review
Alternative patch ro decouple the DIX dependicy from the DtPrint.h header (15.70 KB, patch)
2005-04-03 18:24 UTC, Roland Mainz
no flags Details | Splinter Review
Patch for 2005-04-23-trunk to make the print headers conditional (17.84 KB, patch)
2005-04-23 18:02 UTC, Roland Mainz
no flags Details | Splinter Review

Description Roland Mainz 2005-03-23 02:00:28 UTC
The Xorg/Xprint CVS trunk currently fails to build with |PrintOnlyServer| set to
|NO|.
Comment 1 Roland Mainz 2005-03-23 11:56:25 UTC
Created attachment 2193 [details] [review]
Patch for 2005-03-23-trunk
Comment 2 Roland Mainz 2005-03-23 11:59:59 UTC
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.
Comment 3 Egbert Eich 2005-03-31 09:33:02 UTC
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.


Comment 4 Roland Mainz 2005-03-31 11:55:54 UTC
(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... ;-( ).
Comment 5 Roland Mainz 2005-04-02 19:46:50 UTC
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 6 Roland Mainz 2005-04-02 19:48:10 UTC
Comment on attachment 2294 [details] [review]
Patch to decouple the DIX dependicy to the DiPrint.h header

Requesting review...
Comment 7 Roland Mainz 2005-04-03 18:24:36 UTC
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).
Comment 8 Roland Mainz 2005-04-23 18:01:05 UTC
I am taking the alternative patch for now to have a way to conditionalise the
usage of the headers...
Comment 9 Roland Mainz 2005-04-23 18:02:26 UTC
Created attachment 2526 [details] [review]
Patch for 2005-04-23-trunk to make the print headers conditional
Comment 10 Roland Mainz 2005-04-23 18:11:45 UTC
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.