Bug 15079 – Fails to build on arm (debian)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mike Hommey]

WebKit fails to build in the JavaScriptCore/kjs directory on arm, with the following error:

g++ -c -g -O2 -Wall -D_REENTRANT -I/usr/include -Wreturn-type -fno-strict-aliasing -g -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/libxml2 -I/usr/include/libxml2 -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -DQT_SHARED -DBUILDING_GDK__=1 -DBUILDING_CAIRO__ -DUSE_SYSTEM_MALLOC -DNDEBUG -DHAVE_STDINT_H -DBUILD_WEBKIT -DENABLE_ICONDATABASE=1 -DENABLE_XPATH=1 -DENABLE_XSLT=1 -DENABLE_SVG=1 -DWTF_CHANGES=1 -DBUILDING_GDK__ -I/usr/share/qt4/mkspecs/linux-g++ -I../../WebCore -I../../WebCore/platform/gdk -I../../WebCore/platform/network/curl -I../../WebCore/platform/graphics/cairo -I../../WebCore/loader/gdk -I../../WebCore/page/gdk -I../../WebKit/gtk/Api -I../../WebKit/gtk/WebCoreSupport -I../../JavaScriptCore -I../../JavaScriptCore/kjs -I../../JavaScriptCore/bindings -I../../JavaScriptCore/bindings/c -I../../JavaScriptCore/wtf -I../../JavaScriptCore/ForwardingHeaders -I../../WebCore -I../../WebCore/ForwardingHeaders -I../../WebCore/platform -I../../WebCore/platform/network -I../../WebCore/platform/graphics -I../../WebCore/loader -I../../WebCore/page -I../../WebCore/css -I../../WebCore/dom -I../../WebCore/bridge -I../../WebCore/editing -I../../WebCore/rendering -I../../WebCore/history -I../../WebCore/xml -I../../WebCore/html -Itmp -Itmp -Itmp -I../../JavaScriptCore -I../../JavaScriptCore/kjs -I../../JavaScriptCore/bindings -I../../JavaScriptCore/bindings/c -I../../JavaScriptCore/wtf -I../../JavaScriptCore/pcre -I/build/buildd/webkit-0~svn25144/build-gdk/JavaScriptCore/kjs/tmp -I../../WebCore/platform/graphics/svg/cairo -I../../WebCore/platform/image-decoders/bmp -I../../WebCore/platform/image-decoders/gif -I../../WebCore/platform/image-decoders/ico -I../../WebCore/platform/image-decoders/jpeg -I../../WebCore/platform/image-decoders/png -I../../WebCore/platform/image-decoders/xbm -I../../WebCore -I../../WebCore/ForwardingHeaders -I../../../webkit-0~svn25144 -I../../JavaScriptCore/kjs -I../../JavaScriptCore/bindings -I../../WebCore/platform -I../../WebCore/platform/network -I../../WebCore/platform/graphics -I../../WebCore/platform/graphics/svg -I../../WebCore/platform/graphics/svg/filters -I../../WebCore/loader -I../../WebCore/loader/icon -I../../WebCore/css -I../../WebCore/dom -I../../WebCore/page -I../../WebCore/bridge -I../../WebCore/editing -I../../WebCore/rendering -I../../WebCore/history -I../../WebCore/xml -I../../WebCore/html -I../../WebCore/bindings/js -I../../WebCore/ksvg2 -I../../WebCore/ksvg2/css -I../../WebCore/ksvg2/svg -I../../WebCore/ksvg2/misc -I../../WebCore/ksvg2/events -I../../WebCore/platform/image-decoders -I../../WebKitQt/WebCoreSupport -I../../WebCore -I. -o tmp/ustring.o ../../JavaScriptCore/kjs/ustring.cpp
../../JavaScriptCore/kjs/ustring.cpp:85: error: size of array 'dummyuchar_is_2_bytes' is negative

It seems the PLATFORM(ARM) test doesn't work properly, at least on debian's arm toolchain. I'll attach a patch that should fix this issue (not tested yet).

[Mike Hommey]

Created attachment 16121 [details]
Patch

Adds a test on __arm__

[Adam Roben (:aroben)]

Comment on attachment 16121 [details]
Patch

Thanks for the patch! A couple of comments.

+#if   defined(arm) \
+   || defined(__arm__)

You should just put these on the same line:

#if defined(arm) || defined(__arm__)

You'll need to create a ChangeLog entry using the prepare-ChangeLog. See <http://webkit.org/coding/contributing.html>.

You should also set the review flag to ? when uploading patches so that people know they need review.

Once you have a patch with a ChangeLog entry, upload it here and I'll r+ it.

[Mike Hommey]

(In reply to comment #2)
> (From update of attachment 16121 [details] [edit])
> Thanks for the patch! A couple of comments.
> 
> +#if   defined(arm) \
> +   || defined(__arm__)
> 
> You should just put these on the same line:
> 
> #if defined(arm) || defined(__arm__)

Well, I would have, if surrounding #if's weren't split over several lines. So please tell me which way you really prefer ;)

> You'll need to create a ChangeLog entry using the prepare-ChangeLog. See
> <http://webkit.org/coding/contributing.html>.
> 
> You should also set the review flag to ? when uploading patches so that people
> know they need review.

Being used to mozilla's bugzilla, I assumed I needed a name for a reviewer, which is why i didn't put the review flag.

[Mike Hommey]

Created attachment 16122 [details]
Patch v2

This includes proper Changelog. Please tell me for the #if (cf. previous comment)

[Mark Rowe (bdash)]

What is "FTBFS" that you mention in your ChangeLog?  The patch looks fine to me except for that cryptic message.

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> What is "FTBFS" that you mention in your ChangeLog?  The patch looks fine to me
> except for that cryptic message.

Failed To Build From Source.  It's a Debian thing.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 16121 [details] [edit] [edit])
> > Thanks for the patch! A couple of comments.
> > 
> > +#if   defined(arm) \
> > +   || defined(__arm__)
> > 
> > You should just put these on the same line:
> > 
> > #if defined(arm) || defined(__arm__)
> 
> Well, I would have, if surrounding #if's weren't split over several lines. So
> please tell me which way you really prefer ;)

Since the rest of the file is formatted with split lines, I think it's fine to leave it the way it is in the patch.

[Mark Rowe (bdash)]

Comment on attachment 16122 [details]
Patch v2

Ok.  I'll r+ this but I'd prefer if whomever commits this would decrypt the ChangeLog message when doing so.  FTBFS isn't a commonly-used acronym in this neck of the woods.

[David Kilzer (:ddkilzer)]

Comment on attachment 16122 [details]
Patch v2

r=me

Whoever lands this patch should change "FTBFS" to "build failure" in the ChangeLog.

[David Kilzer (:ddkilzer)]

Committed revision 25253.