Note: There are a few cases of duplicates in user autocompletion which are being worked on.

--enable-system-pixman is broken after Bug 751273 fixed

RESOLVED FIXED in mozilla15

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ojab, Assigned: Takanori MATSUURA)

Tracking

Other Branch
mozilla15
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 12 obsolete attachments)

924 bytes, text/plain
Details
472 bytes, patch
Details | Diff | Splinter Review
20.80 KB, patch
Takanori MATSUURA
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Created attachment 620643 [details]
my mozconfig

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.
(Reporter)

Updated

5 years ago
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

5 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

5 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.

Comment 3

5 years ago
Created attachment 621800 [details] [diff] [review]
add pixman and xrender to the MOZ_GTK2 flags

> 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

5 years ago
Created attachment 621808 [details] [diff] [review]
combined patch

Comment 5

5 years ago
Created attachment 621815 [details] [diff] [review]
add pixman and xrender to the MOZ_GTK2 flags

Doh! typo
Attachment #621800 - Attachment is obsolete: true

Comment 6

5 years ago
Created attachment 621818 [details] [diff] [review]
combined patch

Doh! typo is in the combined patch too
Attachment #621808 - Attachment is obsolete: true

Comment 7

5 years ago
Created attachment 622349 [details] [diff] [review]
Rediffed the patch against the current trunk

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

5 years ago
Created attachment 622358 [details] [diff] [review]
Separate pixman detection from cairo v1

(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

5 years ago
Attachment #621586 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
Created attachment 622395 [details] [diff] [review]
Separate pixman detection from cairo v2

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)
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

5 years ago
Created attachment 623495 [details] [diff] [review]
Separate pixman detection from cairo v3

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

5 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

5 years ago
Please ignore comment 12.
I posted a wrong comment for another bug. :-(

Comment 14

5 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

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 15

5 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

5 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

5 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

5 years ago
Attachment #623495 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 18

5 years ago
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.
Attachment #623495 - Attachment is obsolete: true
(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

5 years ago
Created attachment 626486 [details] [diff] [review]
Separate pixman detection from cairo v5

(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

5 years ago
Attachment #626486 - Flags: feedback?(stransky)
Attachment #626486 - Flags: feedback?(b3nton)
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

5 years ago
Attachment #626486 - Flags: feedback?(stransky)
Attachment #626486 - Flags: feedback?(b3nton)
(Assignee)

Comment 22

5 years ago
Created attachment 626766 [details] [diff] [review]
Separate pixman detection from cairo v6

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
(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.
(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

5 years ago
Created attachment 627007 [details] [diff] [review]
Separate pixman detection from cairo v7

(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 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+
Assignee: nobody → t.matsuu
(Assignee)

Comment 27

5 years ago
Created attachment 627182 [details] [diff] [review]
Separate pixman detection from cairo v7.1

Change only one message string addressed in comment #26.

Carry over r+.
Attachment #627007 - Attachment is obsolete: true
Attachment #627182 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
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
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
Landed a fixed up version.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a64fac52f708
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/a64fac52f708
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.