Closed
Bug 201633
Opened 22 years ago
Closed 22 years ago
xprintutil.c needs to be moved into a utillity library
Categories
(Core Graveyard :: Printing: Xprint, defect)
Core Graveyard
Printing: Xprint
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.4beta
People
(Reporter: netscape, Assigned: roland.mainz)
References
Details
(Keywords: fixed1.4)
Attachments
(3 files, 1 obsolete file)
153.24 KB,
patch
|
leaf
:
review+
asa
:
approval1.4+
|
Details | Diff | Splinter Review |
977 bytes,
patch
|
Details | Diff | Splinter Review | |
2.16 KB,
patch
|
netscape
:
review+
|
Details | Diff | Splinter Review |
xprintutil.c is linked into both libgfx_gtk.so & libgfx_xprint.so. This causes
problems for the new static build option (bug 201602). Common code like that
needs to be linked into a separate library that the components link against.
Assignee | ||
Comment 1•22 years ago
|
||
Christopher Seawood wrote:
> Common code like that
> needs to be linked into a separate library that the components link against.
I guess we can simply build it as static library, right ?
Reporter | ||
Comment 2•22 years ago
|
||
Yes.
Assignee | ||
Comment 3•22 years ago
|
||
Christopher Seawood:
Is moving the code into gfx/src/xprintutil/ OK (which gets build before
gfx/src/gtk/ and before gfx/src/xprint/) ?
Reporter | ||
Comment 4•22 years ago
|
||
Fine by me.
Assignee | ||
Comment 5•22 years ago
|
||
Taking myself...
Assignee: katakai → Roland.Mainz
QA Contact: Roland.Mainz → katakai
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.4beta
Assignee | ||
Comment 6•22 years ago
|
||
Assignee | ||
Comment 7•22 years ago
|
||
Comment on attachment 120264 [details] [diff] [review]
Patch for 2003-03-23-08-trunk
Requesting r= and checkin.
Note that
gfx/src/xprintutil/xprintutil*.{c|h} are identical to
gfx/src/xprint/xprintutil*.{c|h} - they have only been moved and not been
modified.
Attachment #120264 -
Flags: review?(seawood)
Reporter | ||
Comment 8•22 years ago
|
||
Sorry, I just realized that we also have libgfxshared_s.a which is also a
utility library that's linked against multiple components. Why can't the
xprintutil code go into that?
Assignee | ||
Comment 9•22 years ago
|
||
Christopher Seawood wrote:
> Sorry, I just realized that we also have libgfxshared_s.a which is also a
> utility library that's linked against multiple components.
Erm... because that stuff is more for crossplatform gfx code ?
xprintutil is just a utility library which sits on top of libXp.so to make it's
useable easier, however it's X11-specific and does not fit into the existing
categories ...
> Why can't the xprintutil code go into that?
It would be nice to keep the platform-/module-spefic stuff logically seperate
from platform-/module-independent stuff (otherwise we could just stick
everything into libmozilla.a and create "mozilla-bin" from that, right ? ...
:-)))))
Assignee | ||
Comment 10•22 years ago
|
||
BTW: gfx/src/x11shared/ is also the wrong (new) location for the code (unless we
move the whole code from gfx/src/xlibrgb/ to gfx/src/x11shared/ , too :)
Reporter | ||
Comment 11•22 years ago
|
||
Comment on attachment 120264 [details] [diff] [review]
Patch for 2003-03-23-08-trunk
In gfx/src/xprintutil/Makefile.in, there is no reason why xprintutil should be
setting XPIDL_MODULE and it should definitely *not* be setting it to 'layout'
which is almost certainly already used. Also, is xprintutil.h really a public
header? If not, it shouldn't be added to EXPORTS and exported into
$(DIST)/include. I do not see any reference to tooklit specific includes so I
don't see why TK_CFLAGS is being added to INCLUDES.
In gfx/src/gtk/Makefile.in, there's no need to add xprintutil to both REQUIRES
and LOCAL_INCLUDES. Likewise for the other Makefile.ins.
Attachment #120264 -
Flags: review?(seawood) → review-
Assignee | ||
Comment 12•22 years ago
|
||
Christopher Seawood wrote:
> (From update of attachment 120264 [details] [diff] [review])
> In gfx/src/xprintutil/Makefile.in, there is no reason why xprintutil should be
> setting XPIDL_MODULE and it should definitely *not* be setting it to 'layout'
> which is almost certainly already used.
Ugh... looks the idea to cut&paste the code from other Makefile.in stuff wasn't
a smart idea... ;-/
> Also, is xprintutil.h really a public
> header?
Yes - it must be exported, otherwise it does not make sense to put "xprintutil"
into the REQUIRES section. Either we use the LOCAL_INCLUDES-hack (ewwwwww :) or
use REQUIRES (which I would prefer...).
New patch coming up in a few secs...
Assignee | ||
Comment 13•22 years ago
|
||
Attachment #120264 -
Attachment is obsolete: true
Assignee | ||
Comment 14•22 years ago
|
||
Comment on attachment 123563 [details] [diff] [review]
New patch per cls's comments
Requesting r= (from leaf since cls seems to be offline currently... ;-( ) ...
Attachment #123563 -
Flags: review?(leaf)
Comment 15•22 years ago
|
||
Comment on attachment 123563 [details] [diff] [review]
New patch per cls's comments
it looks like cls's concerns were addressed, r=leaf
Attachment #123563 -
Flags: review?(leaf) → review+
Assignee | ||
Comment 16•22 years ago
|
||
Comment on attachment 123563 [details] [diff] [review]
New patch per cls's comments
Requesting a= for 1.4final - super-low-risk patch - we only move sources around
and the only kind of issues which _may_ occur in theory are build bustage
issues which are catched by the tinderboxen...
Attachment #123563 -
Flags: approval1.4?
Comment 17•22 years ago
|
||
Comment on attachment 123563 [details] [diff] [review]
New patch per cls's comments
a=asa (on behalf of drivers) for checkin to 1.4
Attachment #123563 -
Flags: approval1.4? → approval1.4+
Comment 18•22 years ago
|
||
The patch builds OK on Win32
Reporter | ||
Comment 19•22 years ago
|
||
win32 doesn't use xprint.
Assignee | ||
Comment 20•22 years ago
|
||
Christopher Seawood wrote:
> win32 doesn't use xprint.
Sure, we only wanted to make sure that we don't blow up the win32 tinderboxen or
any other platform which does _not_ build Xprint...
Comment 21•22 years ago
|
||
Fix checked in to trunk
Assignee | ||
Comment 22•22 years ago
|
||
Assignee | ||
Comment 23•22 years ago
|
||
Comment on attachment 124414 [details] [diff] [review]
Patch to fix the Xlib port bustage
Patch checked-in into trunk
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&
cvsroot=/cvsroot&date=explicit&mindate=1054174020&maxdate=1054174500&who=jst%25
netscape.com) ...
Assignee | ||
Comment 24•22 years ago
|
||
Patch to fix the bustage on platforms which enforce a specific order of -L and
-l linker arguments and to cure the StaticBuild bustage.
Patch already checked-in, now all tinderboxen are GREEN. :))
Reporter | ||
Updated•22 years ago
|
Attachment #124431 -
Flags: review+
Comment 25•22 years ago
|
||
All 3 patches checked in to 1.4 branch.
Updated•17 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•