Bug 171788 – REGRESSION: ImageDiff not building with make

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

[Alexey Proskuryakov]

I built WebKit with make, and tried to run tests:

ap$ run-webkit-tests --no-build
Using port 'mac-sierra-wk2'
Test configuration: <sierra, x86_64, debug>
...

ImageDiff was not found at /Users/ap/Safari/OpenSource/WebKitBuild/Debug/ImageDiff
Build check failed

[Alexey Proskuryakov]

I get this message even when not passing --no-build, which is a separate issue, but may be easiest to address at the same time.

ImageDiff was not found at /Users/ap/Safari/OpenSource/WebKitBuild/Debug/ImageDiff

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #1)
> I get this message even when not passing --no-build, which is a separate
> issue, but may be easiest to address at the same time.
> 
> ImageDiff was not found at
> /Users/ap/Safari/OpenSource/WebKitBuild/Debug/ImageDiff

Is this an iOS Simulator build?  Or Mac?

It's definitely looking in the correct spot, it's just failing to actually build ImageDiff.

[Jonathan Bedard]

Created attachment 309375 [details]
Patch

[Jonathan Bedard]

attachment 309375 [details] should do the trick.

Actually, no Apple platforms were building the correct ImageDiff through Make.

[Alexey Proskuryakov]

Comment on attachment 309375 [details]
Patch

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

> Tools/ChangeLog:8
> +        * ImageDiff/Makefile: Use Mac SDK when building for iOS Simulator or iOS device.

Please explain that this is fixing a separate issue that you found while looking into this.

[Jonathan Bedard]

Created attachment 309420 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 309420 [details]
Patch

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

> Tools/ChangeLog:3
> +        REGRESSION: ImageDiff not building with make

I'm not sure if this is even a bug. In the past, we explicitly disabled LayoutTestRelay building with DO_NOT_BUILD_LAYOUT_TEST_RELAY variable (it's actually still in place in trunk). The rationale was that we wanted to maintain symmetry with production builds.

But then I'm not sure about how testers would work. Really confused about the overall plan.

> Tools/ChangeLog:14
> +        * ImageDiff/Makefile: Use Mac SDK when building for iOS Simulator or iOS device.

It's probably best to file a new bug about this, as it really seems like a separate issue.

> Tools/ImageDiff/Makefile:5
> +	OVERRIDE_SDKROOT = macosx
> +	OVERRIDE_ARCHS = "i386 x86_64"

Here is what we used to have in LayoutTestRelay/Makefile, added in <https://trac.webkit.org/r192106>:

OVERRIDE_ARCHS = default
OVERRIDE_SDKROOT = default

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #7)
> Comment on attachment 309420 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309420&action=review
> 
> > Tools/ChangeLog:3
> > +        REGRESSION: ImageDiff not building with make
> 
> I'm not sure if this is even a bug. In the past, we explicitly disabled
> LayoutTestRelay building with DO_NOT_BUILD_LAYOUT_TEST_RELAY variable (it's
> actually still in place in trunk). The rationale was that we wanted to
> maintain symmetry with production builds.

I think the difference with LayoutTestRelay and ImageDiff is that every platform needs an ImageDiff, it's just not always the one built with the target SDK.
 
> It's probably best to file a new bug about this, as it really seems like a
> separate issue.

I can file a separate bug about the iOS Simulator/Device case

[Jonathan Bedard]

<https://bugs.webkit.org/show_bug.cgi?id=171835> is tracking the iOS device and iOS simulator bits.

[Jonathan Bedard]

Created attachment 309433 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/32062947>

[Alex Christensen]

https://bugs.webkit.org/show_bug.cgi?id=171847 fixes a similar problem on Linux and Windows.

[WebKit Commit Bot]

Comment on attachment 309433 [details]
Patch

Clearing flags on attachment: 309433

Committed r216505: <http://trac.webkit.org/changeset/216505>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Talking about this more, Jonathan and I think that this should be conditional, to match production builds. It's not right that release/debug builds ImageDiff, but production does not.