| Summary: | [EFL] All API tests are broken on 15.10 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||
| Component: | WebKit EFL | Assignee: | Joonghun Park <jh718.park> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, commit-queue, jh718.park, lucas.de.marchi, mcatanzaro | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Local Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | 153211 | ||||||
| Bug Blocks: | 150619 | ||||||
| Attachments: |
|
||||||
|
Description
Gyuyoung Kim
2016-01-26 19:02:02 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. Created attachment 270340 [details]
Patch
Comment on attachment 270340 [details]
Patch
rs=me.
Comment on attachment 270340 [details] Patch Clearing flags on attachment: 270340 Committed r195945: <http://trac.webkit.org/changeset/195945> All reviewed patches have been landed. Closing bug. 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. 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 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. (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 ? 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. |