Bug 69997 – [Chromium] Reduce dependencies of ImageDiff to compile it for Android.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Hao Zheng]

Make ImageDiff depend on webkit_support_gfx only. Build it as toolsets:host for Android.

This change is required to make ImageDiff for Android, and can benefit other platforms, too. On Android, most targets are built as toolsets:target. while ImageDiff needs to be built as toolsets:host. Currently, building the standalone target of ImageDiff depends on about 85 other targets (try  make ImageDiff'), which is inefficient. After the change, ImageDiff only depends on webkit_support_gfx, which in turn depends on libpng and zlib.

Related to chromium change: http://codereview.chromium.org/8226018/

[Hao Zheng]

Created attachment 111376 [details]
Proposed patch.

[WebKit Review Bot]

Comment on attachment 111376 [details]
Proposed patch.

Rejecting attachment 111376 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
iff] Error 1
make: *** Waiting for unfinished jobs....

Failed to run "['Tools/Scripts/build-webkit', '--debug', '--chromium', '--update-chromium']" exit_code: 2
target/ImageDiff/Tools/DumpRenderTree/chromium/ImageDiff.o: in function Image::setPixelAt(int, int, unsigned int) const:Tools/DumpRenderTree/chromium/ImageDiff.cpp:153: error: undefined reference to 'WTFReportBacktrace'
collect2: ld returned 1 exit status
make: *** [out/Debug/ImageDiff] Error 1
make: *** Waiting for unfinished jobs....

Full output: http://queues.webkit.org/results/10124291

[Hao Zheng]

Created attachment 111557 [details]
Proposed patch 2.

Sorry, I didn't try Debug build. Now I completely removed code dependent on JavaScriptCore.

[Hao Zheng]

Created attachment 111558 [details]
Proposed patch 3.

[WebKit Review Bot]

Attachment 111557 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1

Tools/DumpRenderTree/chromium/ImageDiff.cpp:43:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 111558 [details]
Proposed patch 3.

Attachment 111558 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10142236

[Hao Zheng]

Created attachment 111580 [details]
Proposed patch 4.

[Tony Chang]

Comment on attachment 111580 [details]
Proposed patch 4.

View in context: https://bugs.webkit.org/attachment.cgi?id=111580&action=review

I think we should just depend on wtf because it's pretty small.  Or are you trying to avoid having host and target in JavaScriptCore.gyp?

> Tools/DumpRenderTree/chromium/ImageDiff.cpp:51
> +#include <limits.h>

I would just include this on all platforms if a single platform needs it.

[Hao Zheng]

Created attachment 111711 [details]
Proposed patch 5.

[Hao Zheng]

(In reply to comment #8)
> (From update of attachment 111580 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111580&action=review
> 
> I think we should just depend on wtf because it's pretty small.  Or are you trying to avoid having host and target in JavaScriptCore.gyp?

No, it's because wtf would depend on icu, which in turn would depend on others...

> 
> > Tools/DumpRenderTree/chromium/ImageDiff.cpp:51
> > +#include <limits.h>
> 
> I would just include this on all platforms if a single platform needs it.

Done.

[WebKit Review Bot]

Comment on attachment 111711 [details]
Proposed patch 5.

Clearing flags on attachment: 111711

Committed r97998: <http://trac.webkit.org/changeset/97998>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.