Closed
Bug 751521
Opened 13 years ago
Closed 13 years ago
--enable-system-pixman is broken after Bug 751273 fixed
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: ojab, Assigned: t.matsuu)
References
Details
Attachments
(3 files, 12 obsolete files)
924 bytes,
text/plain
|
Details | |
472 bytes,
patch
|
Details | Diff | Splinter Review | |
20.80 KB,
patch
|
t.matsuu
:
review+
|
Details | Diff | Splinter Review |
mozilla-inbound is broken for me since
changeset: 92892:025285f57616
parent: 92888:5900fe7cd355
parent: 92891:d4e35005f5a9
user: Ehsan Akhgari <ehsan@mozilla.com>
date: Wed May 02 21:52:33 2012 -0400
summary: Merge mozilla-central into mozilla-inbound on a CLOSED TREE
which fixes Bug 750661 and dependencies. Also reproducible with hg tip (-r92982:d7271f499b8b) The actual error is:
make[7]: Leaving directory `/home/ojab/opt2/xulrunner/layout/build'
make -C media export
make[7]: Entering directory `/home/ojab/opt2/xulrunner/layout/media'
/sources/mozilla-inbound/config/rules.mk:745: *** SHARED_LIBRARY_LIBS must contain .a files only. Stop.
make[7]: Leaving directory `/home/ojab/opt2/xulrunner/layout/media'
make[6]: *** [export] Error 2
make[6]: Leaving directory `/home/ojab/opt2/xulrunner/layout'
make[5]: *** [export_tier_platform] Error 2
make[5]: Leaving directory `/home/ojab/opt2/xulrunner'
make[4]: *** [tier_platform] Error 2
And SHARED_LIBRARY_LIBS expands to:
/sources/mozilla-inbound/config/rules.mk:745: *** .../../gfx/ots/src/libmozots.a ../../gfx/qcms/libmozqcms.a ../../gfx/cairo/cairo/src/libmozcairo.a -lpixman-1 ../../gfx/graphite2/src/libmozgraphite2.a ../../media/libvorbis/lib/libvorbis.a ../../media/libogg/src/libogg.a ../../media/libtheora/lib/libtheora.a ../../media/libopus/libopus.a ../../media/libnestegg/src/libnestegg.a ../../media/libsydneyaudio/src/libsydneyaudio.a ../../media/libcubeb/src/libcubeb.a ../../gfx/angle/libangle.a ../../parser/expat/lib/libmozexpat_s.a must contain .a files only. Stop.
Blocks: 751273
Summary: Build failure with `SHARED_LIBRARY_LIBS must contain .a files only` → --enable-system-pixman is broken after Bug 751273 fixed
Assignee | ||
Comment 1•13 years ago
|
||
MOZ_CAIRO_LIBS generated by configure script contains both cairo and pixman libraries:
https://hg.mozilla.org/mozilla-central/annotate/e6228decd18e/configure.in#l8146
This may include "-lpixman-1" in the MOZ_CAIRO_LIBS. And it causes error.
Assignee | ||
Comment 2•13 years ago
|
||
This works with --enable-system-pixman but not without it.
We cannot test the combination of --enable-system-cairo because of the bug 722975.
Any advice or updated patch is welcome.
Comment 3•13 years ago
|
||
> Created attachment 621586 [details] [diff] [review]
> Separate pixman detection from cairo (doesn't work)
>
> This works with --enable-system-pixman but not without it.
>
> We cannot test the combination of --enable-system-cairo because of the bug
> 722975.
>
> Any advice or updated patch is welcome.
You missed a bit. With this patch and the above patch from Takanori MATSUURA (many thanks for that!) I was able to build Firefox with --enable-system-cairo. (I have patched my system cairo with the expose-snapshot patch)
Comment 4•13 years ago
|
||
Comment 6•13 years ago
|
||
Doh! typo is in the combined patch too
Attachment #621808 -
Attachment is obsolete: true
Comment 7•13 years ago
|
||
layout/media/Makefile.in and layout/media/symbols.def.in have changed so the previous patch didn't apply cleanly
Attachment #621818 -
Attachment is obsolete: true
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to Andrew Benton from comment #7)
> Created attachment 622349 [details] [diff] [review]
> Rediffed the patch against the current trunk
Thanks, Andrew.
I found that _moz_pixman* is defined only when both MOZ_TREE_CAIRO and MOZ_TREE_PIXMAN are defined. So I've updated a patch with a bit modified.
Attachment #622349 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #621586 -
Attachment is obsolete: true
Assignee | ||
Comment 9•13 years ago
|
||
This patch now works both with and without --enable-system-pixman.
I suppose attachment 621815 [details] [diff] [review] is not required now.
Before checking this in, we may need try build.
(However I don't have an account.)
Attachment #622358 -
Attachment is obsolete: true
Attachment #622395 -
Flags: review?(ted.mielczarek)
Comment 10•13 years ago
|
||
If you'd like to get access to the try server, file a bug and CC me and I will vouch for you:
http://www.mozilla.org/hacking/commit-access-policy/
Assignee | ||
Comment 11•13 years ago
|
||
Update to fit to the latest m-c.
Attachment #622395 -
Attachment is obsolete: true
Attachment #622395 -
Flags: review?(ted.mielczarek)
Attachment #623495 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 12•13 years ago
|
||
There are two possible solutions:
1. Use os.mkdirs instead of os.mkdir to generate cachedir
xpcom/idl-parser/header.py:
if options.cachedir is not None:
if not os.path.isdir(options.cachedir):
os.mkdir(options.cachedir)
sys.path.append(options.cachedir)
2. "make export" at xpcom/idl-parser before parsing idl files
xpcom/idl-parser/Makefile.in:
# Generate the PLY lexer and parser.
export:: $(PARSER_SRCS) $(PLY_PROGS)
$(PYTHON_PATH) \
$(PLY_INCLUDE) \
-I$(topsrcdir)/xpcom/idl-parser \
$(topsrcdir)/xpcom/idl-parser/header.py --cachedir=$(DEPTH)/xpcom/idl-parser/cache --regen
Assignee | ||
Comment 13•13 years ago
|
||
Please ignore comment 12.
I posted a wrong comment for another bug. :-(
Comment 14•13 years ago
|
||
Comment on attachment 623495 [details] [diff] [review]
Separate pixman detection from cairo v3
The patch does not build with latest trunk & --enabled-system-cairo & --enabled-system-pixman. libxul.so link fails because of missing libXrender.so library:
../../widget/gtk2/nsWindow.o: In function `nsWindow::GetSurfaceForGdkDrawable(_GdkDrawable*, nsIntSize const&)':
/home/komat/tmp646-trunk/src/widget/gtk2/nsWindow.cpp:6000: undefined reference to `XRenderFindStandardFormat'
/home/komat/tmp646-trunk/src/widget/gtk2/nsWindow.cpp:6003: undefined reference to `XRenderFindStandardFormat'
../../dom/plugins/ipc/PluginInstanceChild.o: In function `mozilla::plugins::PluginInstanceChild::CreateOptSurface()':
/home/komat/tmp646-trunk/src/dom/plugins/ipc/PluginInstanceChild.cpp:2708: undefined reference to `XRenderFindStandardFormat'
../../gfx/thebes/gfxXlibSurface.o: In function `gfxXlibSurface::CreateSimilarSurface(gfxASurface::gfxContentType, nsIntSize const&)':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:229: undefined reference to `XRenderFindStandardFormat'
../../gfx/thebes/gfxXlibSurface.o: In function `DisplayTable::GetColormapAndVisual(Screen*, XRenderPictFormat*, Visual*, unsigned long*, Visual**)':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:345: undefined reference to `XRenderFindVisualFormat'
../../gfx/thebes/gfxXlibSurface.o: In function `gfxXlibSurface::FindRenderFormat(_XDisplay*, gfxASurface::gfxImageFormat)':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:507: undefined reference to `XRenderFindStandardFormat'
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:509: undefined reference to `XRenderFindStandardFormat'
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:517: undefined reference to `XRenderFindVisualFormat'
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:520: undefined reference to `XRenderFindStandardFormat'
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibSurface.cpp:522: undefined reference to `XRenderFindStandardFormat'
../../gfx/thebes/gfxXlibNativeRenderer.o: In function `gfxXlibNativeRenderer::DrawDirect(gfxContext*, nsIntSize, unsigned int, Screen*, Visual*)':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibNativeRenderer.cpp:258: undefined reference to `XRenderFindVisualFormat'
../../gfx/thebes/gfxXlibNativeRenderer.o: In function `FormatConversionIsExact':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibNativeRenderer.cpp:301: undefined reference to `XRenderFindVisualFormat'
../../gfx/thebes/gfxXlibNativeRenderer.o: In function `CreateTempXlibSurface':
/home/komat/tmp646-trunk/src/gfx/thebes/gfxXlibNativeRenderer.cpp:370: undefined reference to `XRenderFindVisualFormat'
../../gfx/layers/ShadowLayerUtilsX11.o: In function `mozilla::layers::GetXRenderPictFormatFromId(_XDisplay*, unsigned long)':
/home/komat/tmp646-trunk/src/gfx/layers/ipc/ShadowLayerUtilsX11.cpp:69: undefined reference to `XRenderFindFormat'
collect2: ld returned 1 exit status
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 15•13 years ago
|
||
(In reply to Martin Stránský from comment #14)
> Comment on attachment 623495 [details] [diff] [review]
> Separate pixman detection from cairo v3
>
> The patch does not build with latest trunk & --enabled-system-cairo &
> --enabled-system-pixman. libxul.so link fails because of missing
> libXrender.so library:
Try the patch I posted in comment #5
Comment 16•13 years ago
|
||
I believe the proper way is to fix the MOZ_CAIRO_OSLIBS which is empty now but it's still used for libxul link. It may be something like:
diff --git a/toolkit/library/Makefile.in b/toolkit/library/Makefile.in
--- a/toolkit/library/Makefile.in
+++ b/toolkit/library/Makefile.in
@@ -339,5 +339,5 @@ EXTRA_DSO_LDOPTS += \
$(MOZ_JS_LIBS) \
$(NSS_LIBS) \
- $(MOZ_CAIRO_OSLIBS) \
+ $(MOZ_CAIRO_LIBS) \
$(MOZ_APP_EXTRA_LIBS) \
$(SQLITE_LIBS) \
or so.
Comment 17•13 years ago
|
||
Another problem is when --enable-system-pixman is not set and --enable-system-cairo is set, SHARED_LIBRARY_LIBS contains "@MOZ_PIXMAN_LIBS@" string.
Assignee | ||
Updated•13 years ago
|
Attachment #623495 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 18•13 years ago
|
||
Tested on Fedora 16:
* set --enable-system-pixman and unset --enable-system-cairo
* unset --enable-system-pixman and unset --enable-system-cairo
Not tested:
* set --enable-system-pixman and set --enable-system-cairo
Expected to fail:
* unset --enable-system-pixman and set --enable-system-cairo
This will be failed with the message of "--enable-system-cairo should be set with --enable-system-pixman." because I believe system cairo is built with system libpixman.
Attachment #623495 -
Attachment is obsolete: true
Comment 19•13 years ago
|
||
(In reply to Takanori MATSUURA from comment #18)
> Created attachment 626390 [details] [diff] [review]
> Separate pixman detection from cairo v4
>
> Tested on Fedora 16:
> * set --enable-system-pixman and unset --enable-system-cairo
> * unset --enable-system-pixman and unset --enable-system-cairo
>
> Not tested:
> * set --enable-system-pixman and set --enable-system-cairo
>
> Expected to fail:
> * unset --enable-system-pixman and set --enable-system-cairo
> This will be failed with the message of "--enable-system-cairo should be set
> with --enable-system-pixman." because I believe system cairo is built with
> system libpixman.
AFAIK, we don't use pixman directly, only cairo does. So --enable-system-cairo should just imply --enable-system-pixman (which it somehow does, currently)
Assignee | ||
Comment 20•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #19)
> AFAIK, we don't use pixman directly, only cairo does. So
> --enable-system-cairo should just imply --enable-system-pixman (which it
> somehow does, currently)
Thanks.
Updated patch now implies --enable-system-pixman when --enable-system-cairo is set.
TBPL for this patch:
https://tbpl.mozilla.org/?tree=Try&rev=34699c1d6c21
Attachment #626390 -
Attachment is obsolete: true
Attachment #626486 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•13 years ago
|
Attachment #626486 -
Flags: feedback?(stransky)
Attachment #626486 -
Flags: feedback?(b3nton)
Comment 21•13 years ago
|
||
Comment on attachment 626486 [details] [diff] [review]
Separate pixman detection from cairo v5
Review of attachment 626486 [details] [diff] [review]:
-----------------------------------------------------------------
::: configure.in
@@ +8037,5 @@
> MOZ_TREE_PIXMAN=1 )
>
> +# System cairo depends on system libpixman
> +if test -z "$MOZ_TREE_CAIRO"; then
> + MOZ_TREE_PIXMAN=
Please throw an error if --enable-system-cairo is given with --disable-system-pixman
@@ +8148,2 @@
> if test "$MOZ_X11"; then
> + MOZ_CAIRO_OSLIBS="$MOZ_CAIRO_OSLIBS $XLDFLAGS $XRENDER_LIBS $FT2_LIBS $FONTCONFIG_LIBS"
This is very likely to break building with in-tree freetype (which android builds do)
Attachment #626486 -
Flags: review?(mh+mozilla) → review-
Assignee | ||
Updated•13 years ago
|
Attachment #626486 -
Flags: feedback?(stransky)
Attachment #626486 -
Flags: feedback?(b3nton)
Assignee | ||
Comment 22•13 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=a7a0e98914f3
(In reply to Mike Hommey [:glandium] from comment #21)
> ::: configure.in
> @@ +8037,5 @@
> > MOZ_TREE_PIXMAN=1 )
> >
> > +# System cairo depends on system libpixman
> > +if test -z "$MOZ_TREE_CAIRO"; then
> > + MOZ_TREE_PIXMAN=
>
> Please throw an error if --enable-system-cairo is given with
> --disable-system-pixman
Fixed. I'd like someone to test this because I don't have system cairo ready for building Firefox from m-c.
> @@ +8148,2 @@
> > if test "$MOZ_X11"; then
> > + MOZ_CAIRO_OSLIBS="$MOZ_CAIRO_OSLIBS $XLDFLAGS $XRENDER_LIBS $FT2_LIBS $FONTCONFIG_LIBS"
>
> This is very likely to break building with in-tree freetype (which android
> builds do)
I suppose MOZ_X11 is not set on Android. I don't get the build error in this area on Android with the previous patch.
Attachment #626486 -
Attachment is obsolete: true
Comment 23•13 years ago
|
||
(In reply to Takanori MATSUURA from comment #22)
> > This is very likely to break building with in-tree freetype (which android
> > builds do)
>
> I suppose MOZ_X11 is not set on Android. I don't get the build error in this
> area on Android with the previous patch.
The option still exists to create such builds for X.
Comment 24•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #23)
> (In reply to Takanori MATSUURA from comment #22)
> > > This is very likely to break building with in-tree freetype (which android
> > > builds do)
> >
> > I suppose MOZ_X11 is not set on Android. I don't get the build error in this
> > area on Android with the previous patch.
>
> The option still exists to create such builds for X.
(Note that I don't know if such builds work already. If they don't, then it's fine to error out, then. If they do, it would be better to preserve the functionality)
Assignee | ||
Comment 25•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #24)
> (In reply to Mike Hommey [:glandium] from comment #23)
> > The option still exists to create such builds for X.
>
> (Note that I don't know if such builds work already. If they don't, then
> it's fine to error out, then. If they do, it would be better to preserve the
> functionality)
I remove this part from the patch because it is not critical to fix this bug.
Try: https://tbpl.mozilla.org/?tree=Try&rev=22dec7021309
I'm not sure why libraries are hard-coded here. But the issue should be discussed as another bug.
Attachment #626766 -
Attachment is obsolete: true
Attachment #627007 -
Flags: review?(mh+mozilla)
Comment 26•13 years ago
|
||
Comment on attachment 627007 [details] [diff] [review]
Separate pixman detection from cairo v7
Review of attachment 627007 [details] [diff] [review]:
-----------------------------------------------------------------
::: configure.in
@@ +8029,5 @@
>
> +# System cairo depends on system pixman
> +if test "${MOZ_TREE_PIXMAN}" = "force"; then
> + if test -z "${MOZ_TREE_CAIRO}"; then
> + AC_MSG_ERROR([System cairo should be used with system pixman.])
Nit: This is kind of a confusing message. It sounds as if you require --enable-system-pixman when you use --enable-system-cairo, when it's actually about the opposite. Reverting is not going to make things much clearer. I'd go for "--disable-system-pixman is incompatible with --enable-system-cairo"
Attachment #627007 -
Flags: review?(mh+mozilla) → review+
Updated•13 years ago
|
Assignee: nobody → t.matsuu
Assignee | ||
Comment 27•13 years ago
|
||
Change only one message string addressed in comment #26.
Carry over r+.
Attachment #627007 -
Attachment is obsolete: true
Attachment #627182 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•13 years ago
|
Status: NEW → ASSIGNED
Comment 28•13 years ago
|
||
Unfortunately, building with --enable-system-cairo fails with:
Exception: File not found: ../../gfx/cairo/libpixman/src/libmozlibpixman.a
when building libgkmedias.a.desc.
Keywords: checkin-needed
Comment 29•13 years ago
|
||
This hunk from v5 is needed before "if test "$MOZ_TREE_PIXMAN"; then":
# System cairo depends on system libpixman
if test -z "$MOZ_TREE_CAIRO"; then
MOZ_TREE_PIXMAN=
fi
Comment 30•13 years ago
|
||
Landed a fixed up version.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a64fac52f708
Target Milestone: --- → mozilla15
Comment 31•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•