Bug 153528 - [EFL] All API tests are broken on 15.10
Summary: [EFL] All API tests are broken on 15.10
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords:
Depends on: 153211
Blocks: 150619
  Show dependency treegraph
 
Reported: 2016-01-26 19:02 PST by Gyuyoung Kim
Modified: 2016-02-01 06:51 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2016-01-31 06:40 PST, Joonghun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2016-01-26 19:02:02 PST
When running EFL API tests on Ubuntu 15.10, all tests are failed. Need to fix it to bump Ubuntu version.
Comment 1 Joonghun Park 2016-01-29 21:48:32 PST
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.
Comment 2 Joonghun Park 2016-01-31 06:40:01 PST
Created attachment 270340 [details]
Patch
Comment 3 Gyuyoung Kim 2016-01-31 16:23:02 PST
Comment on attachment 270340 [details]
Patch

rs=me.
Comment 4 WebKit Commit Bot 2016-01-31 17:21:01 PST
Comment on attachment 270340 [details]
Patch

Clearing flags on attachment: 270340

Committed r195945: <http://trac.webkit.org/changeset/195945>
Comment 5 WebKit Commit Bot 2016-01-31 17:21:05 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Michael Catanzaro 2016-01-31 19:48:30 PST
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.
Comment 7 Gyuyoung Kim 2016-01-31 20:57:11 PST
(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.
Comment 8 Joonghun Park 2016-01-31 20:59:36 PST
(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.
Comment 9 Gyuyoung Kim 2016-01-31 21:48:22 PST
(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 ?
Comment 10 Michael Catanzaro 2016-02-01 06:51:22 PST
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.