Closed
Bug 281950
Opened 19 years ago
Closed 19 years ago
Landing of cairo/libpixman
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: tor, Assigned: tor)
Details
Attachments
(1 file, 1 obsolete file)
7.85 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
Both SVG and <canvas> need cairo, which we need to ship as it's not yet a standard part of linux distributions and will not be on platforms. This patch defaults to the in-tree cairo, allowing a system cairo to be used with --enable-system-cairo. The patch uses unmodified libpixman and cairo source trees located in the directories gfx/cairo/{libpixman,cairo}. It works, but I could use input from autoconf and mozilla build system experts about the appropriate ways of doing the things I'm attempting.
Comment 2•19 years ago
|
||
Comment on attachment 174053 [details] [diff] [review] cairo landing patch Chris, bounce this back to me if you don't have time.
Attachment #174053 -
Flags: review?(cls)
Attachment #174053 -
Flags: review?(cls) → review?(benjamin)
Comment 3•19 years ago
|
||
Comment on attachment 174053 [details] [diff] [review] cairo landing patch >Index: configure.in >+MOZ_CAIRO_LIBS=-"lcairo -lpixman" Ick, try "-lcairo -lpixman" ;) >+ [ --enable-system-cairo Use system cairo (located with pkgconfig)], Would it make sense to support an explicit path in addition to pkgconfig (--with-system-cairo=/usr/lib/cairo) or do we need pkgconfig to pick up the correct CFLAGS? >Index: Makefile.in > clean clobber realclean clobber_all distclean:: [snip] >+ifndef MOZ_SYSTEM_CAIRO >+ $(MAKE) -C gfx/cairo/libpixman $@ >+ $(MAKE) -C gfx/cairo/cairo $@ >+endif Do you care that the realclean and clobber_all target probably aren't supported? I think that tinderboxen may still use them. >+cairo: >+ifndef MOZ_SYSTEM_CAIRO >+ $(MAKE) -C gfx/cairo/libpixman install >+ $(MAKE) -C gfx/cairo/cairo install I see the "make install" but I don't ever see the "make"... marking r- for this, re-request r? if there is something I'm missing. >+ mkdir -p $(DIST)/bin This shouldn't be needed, but if it is, don't use "-p". >+ifneq ($(OS_ARCH),Darwin) >+ -cp $(DIST)/lib/libpixman.so.1 $(DIST)/bin >+ -cp $(DIST)/lib/libcairo.so.1 $(DIST)/bin >+else >+ -cp $(DIST)/lib/libpixman.1.dylib $(DIST)/bin >+ -cp $(DIST)/lib/libcairo.1.dylib $(DIST)/bin >+endif >+endif Why "cp" instead of $(INSTALL) ?... we've already built nsinstall at this point, and it will save a lot of dependency overhead later in depend builds.
Attachment #174053 -
Flags: review?(benjamin) → review-
(In reply to comment #3) > >+MOZ_CAIRO_LIBS=-"lcairo -lpixman" > > Ick, try "-lcairo -lpixman" ;) Heh, a typo that actually worked. > >+ [ --enable-system-cairo Use system cairo (located with pkgconfig)], > > Would it make sense to support an explicit path in addition to pkgconfig > (--with-system-cairo=/usr/lib/cairo) or do we need pkgconfig to pick up the > correct CFLAGS? The cairo header files don't seem to carry a version number, so the only way to check if the version is new enough is what pkgconfig tells us. > > clean clobber realclean clobber_all distclean:: > [snip] > >+ifndef MOZ_SYSTEM_CAIRO > >+ $(MAKE) -C gfx/cairo/libpixman $@ > >+ $(MAKE) -C gfx/cairo/cairo $@ > >+endif > > Do you care that the realclean and clobber_all target probably aren't > supported? I think that tinderboxen may still use them. I split the targets that can be passed through and those that need to be mapped - not sure that's the most elegant way of doing things. > >+cairo: > >+ifndef MOZ_SYSTEM_CAIRO > >+ $(MAKE) -C gfx/cairo/libpixman install > >+ $(MAKE) -C gfx/cairo/cairo install > > I see the "make install" but I don't ever see the "make"... marking r- for > this, re-request r? if there is something I'm missing. The rules for the install target of libpixman and cairo cause them to build the targets - calling make twice on each directory seemed redundant. > >+ mkdir -p $(DIST)/bin > > This shouldn't be needed, but if it is, don't use "-p". Leftover from development - removed. > >+ifneq ($(OS_ARCH),Darwin) > >+ -cp $(DIST)/lib/libpixman.so.1 $(DIST)/bin > >+ -cp $(DIST)/lib/libcairo.so.1 $(DIST)/bin > >+else > >+ -cp $(DIST)/lib/libpixman.1.dylib $(DIST)/bin > >+ -cp $(DIST)/lib/libcairo.1.dylib $(DIST)/bin > >+endif > >+endif > > Why "cp" instead of $(INSTALL) ?... we've already built nsinstall at this > point, and it will save a lot of dependency overhead later in depend builds. No reason - changed.
Attachment #174053 -
Attachment is obsolete: true
Attachment #175380 -
Flags: review?(benjamin)
Updated•19 years ago
|
Attachment #175380 -
Flags: review?(benjamin) → review+
Landed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•