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)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.4beta

People

(Reporter: netscape, Assigned: roland.mainz)

References

Details

(Keywords: fixed1.4)

Attachments

(3 files, 1 obsolete file)

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.
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 ?
Christopher Seawood: Is moving the code into gfx/src/xprintutil/ OK (which gets build before gfx/src/gtk/ and before gfx/src/xprint/) ?
Fine by me.
Taking myself...
Assignee: katakai → Roland.Mainz
QA Contact: Roland.Mainz → katakai
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.4beta
Attached patch Patch for 2003-03-23-08-trunk (obsolete) — Splinter Review
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)
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?
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 ? ... :-)))))
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 :)
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-
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...
Attachment #120264 - Attachment is obsolete: true
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 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+
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 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+
The patch builds OK on Win32
win32 doesn't use xprint.
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...
Fix checked in to trunk
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) ...
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. :))
All 3 patches checked in to 1.4 branch.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Keywords: fixed1.4
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: