Bug 153528 – [EFL] All API tests are broken on 15.10

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Gyuyoung Kim]

When running EFL API tests on Ubuntu 15.10, all tests are failed. Need to fix it to bump Ubuntu version.

[Joonghun Park]

It seems that this issue is caused by https://bugs.webkit.org/show_bug.cgi?id=153211.
When I revert it, I found that API tests execution works well in 15.10 as well as 15.04.

[Joonghun Park]

Created attachment 270340 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 270340 [details]
Patch

rs=me.

[WebKit Commit Bot]

Comment on attachment 270340 [details]
Patch

Clearing flags on attachment: 270340

Committed r195945: <http://trac.webkit.org/changeset/195945>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Hm, this defeats the point of putting those statements in conditionals, and will break Windows. See the bottom of bug #153211.

We should think about how to make this work for both EFL and Windows. It's also rather fragile to have code that depends on include order. Maybe we should just set the directories for Windows in the toplevel CMakeLists.txt, to make this more clear.

[Gyuyoung Kim]

(In reply to comment #6)
> Hm, this defeats the point of putting those statements in conditionals, and
> will break Windows. See the bottom of bug #153211.

Now I don't know this change still causes to copy files to bin instead of bin64 on win port. Now I'm checking it.

[Joonghun Park]

(In reply to comment #6)
> Hm, this defeats the point of putting those statements in conditionals, and
> will break Windows. See the bottom of bug #153211.
> 
> We should think about how to make this work for both EFL and Windows. It's
> also rather fragile to have code that depends on include order. Maybe we
> should just set the directories for Windows in the toplevel CMakeLists.txt,
> to make this more clear.

(In reply to comment #6)
> Hm, this defeats the point of putting those statements in conditionals, and
> will break Windows. See the bottom of bug #153211.
> 
> We should think about how to make this work for both EFL and Windows. It's
> also rather fragile to have code that depends on include order. Maybe we
> should just set the directories for Windows in the toplevel CMakeLists.txt,
> to make this more clear.

First of all, OptionsEfl.cmake:46's statement
add_definitions(-DWEBKIT-EXEC_PATH=\"${CMAKE_RUNTIME_OUTPUT_DIRECTORY}\")
uses the value otherwise all the api tests goes to be blocked, so I put this change in.

I have seen the bottom of the link you had left, https://bugs.webkit.org/show_bug.cgi?id=153211. 
And as you said, this change makes the conditional meaningless truly, but it seems that current cmake working is fine because the CMAKE_FOO_OUTPUT_DIRECTORY will be overwritten by OptionsWin.cmake and then the values will be used. 
So I think this change is not breaking Windows, but if I missed something please let me know.

And as you said some changes would be needed including removing the conditionals or the ordering dependency causing the code fragile.

[Gyuyoung Kim]

(In reply to comment #7)
> (In reply to comment #6)
> > Hm, this defeats the point of putting those statements in conditionals, and
> > will break Windows. See the bottom of bug #153211.
> 
> Now I don't know this change still causes to copy files to bin instead of
> bin64 on win port. Now I'm checking it.

In my local win port build is fine after merging this change. Besides MiniBrowser works well. But unfortunately my local windows box is 32bit. So I'm not sure if my test is fine or not.

Alex, could you let check if this changes breaks something on win port ?

[Michael Catanzaro]

I think Joonghun is right, Windows should be fine since it will overwrite these values after they're set.

We should still clean this up, though.