Bug 97302 – [EFL][WK2] Change the install path for libewk2UnitTestInjectedBundleSample.so

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Byungwoo Lee]

After building, libewk2UnitTestInjectedBundleSample.so library is created in the Source/WebKit2/UIProcess/API/efl/tests/resources/.
But the path was in the Source directory, so it will be better to move it under the build directory.

[Byungwoo Lee]

Created attachment 165067 [details]
Patch

[Kangil Han]

Comment on attachment 165067 [details]
Patch

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

> Source/WebKit2/PlatformEfl.cmake:307
> +    INSTALL(TARGETS ewk2UnitTestInjectedBundleSample DESTINATION "${LIB_INSTALL_DIR}")

We don't have to install test library. :-)

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165067 [details]
Patch

Yeah, INSTALLing that library is the wrong approach. You just need to adjust the LIBRARY_OUTPUT_DIRECTORY.

[Byungwoo Lee]

(In reply to comment #3)
> (From update of attachment 165067 [details])
> Yeah, INSTALLing that library is the wrong approach. You just need to adjust the LIBRARY_OUTPUT_DIRECTORY.

Yes you are right. I'm preparing like above. Thanks Kangil and Kubo :)

[Byungwoo Lee]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 165067 [details] [details])
> > Yeah, INSTALLing that library is the wrong approach. You just need to adjust the LIBRARY_OUTPUT_DIRECTORY.
> 
> Yes you are right. I'm preparing like above. Thanks Kangil and Kubo :)

I found some unnecessary dependency for ewk2UnitTestInjectedBundleSample with ewk2UnitTestUtils, so I'll fix this also.

Before removing the dependency, I met a SegFault on unit test at test_ewk2_context some times, but with removing this, the SegFault not found.

[Byungwoo Lee]

Created attachment 165105 [details]
Patch

[Ryuan Choi]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (From update of attachment 165067 [details] [details] [details])
> > > Yeah, INSTALLing that library is the wrong approach. You just need to adjust the LIBRARY_OUTPUT_DIRECTORY.
> > 
> > Yes you are right. I'm preparing like above. Thanks Kangil and Kubo :)
> 
> I found some unnecessary dependency for ewk2UnitTestInjectedBundleSample with ewk2UnitTestUtils, so I'll fix this also.
> 
> Before removing the dependency, I met a SegFault on unit test at test_ewk2_context some times, but with removing this, the SegFault not found.

BTW, why you move the code?

Is it also necessary?

[Byungwoo Lee]

+(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > (From update of attachment 165067 [details] [details] [details] [details])
> > > > Yeah, INSTALLing that library is the wrong approach. You just need to adjust the LIBRARY_OUTPUT_DIRECTORY.
> > > 
> > > Yes you are right. I'm preparing like above. Thanks Kangil and Kubo :)
> > 
> > I found some unnecessary dependency for ewk2UnitTestInjectedBundleSample with ewk2UnitTestUtils, so I'll fix this also.
> > 
> > Before removing the dependency, I met a SegFault on unit test at test_ewk2_context some times, but with removing this, the SegFault not found.
> 
> BTW, why you move the code?
> 
> Is it also necessary?
I moved it to add definition for the library path.
But As you pointed, removing library dependency or moving some code is not necessary for changing library path.
I'll just change the library path.
Thank you :)

[Byungwoo Lee]

Created attachment 165256 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 165256 [details]
Patch

Looks fine. Please land after verifying if this patch doesn't influence unit test on debug and release build.

[Byungwoo Lee]

Comment on attachment 165256 [details]
Patch

I tested on debug/release and ok.
Thank you :)

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165256 [details]
Patch

This still looks wrong.

The library is being automatically put into ${CMAKE_BINARY_DIR}/lib because of the way we set the CMAKE_LIBRARY_OUTPUT_DIRECTORY variable in the top-level CMakeLists.txt; using ${LIB_INSTALL_DIR} like you did does not make much sense, as LIB_SUFFIX is not used by CMAKE_LIBRARY_OUTPUT_DIRECTORY.

Just setting TEST_LIB_DIR to ${CMAKE_LIBRARY_OUTPUT_DIRECTORY} should be fine.

[Byungwoo Lee]

(In reply to comment #12)
> (From update of attachment 165256 [details])
> This still looks wrong.
> 
> The library is being automatically put into ${CMAKE_BINARY_DIR}/lib because of the way we set the CMAKE_LIBRARY_OUTPUT_DIRECTORY variable in the top-level CMakeLists.txt; using ${LIB_INSTALL_DIR} like you did does not make much sense, as LIB_SUFFIX is not used by CMAKE_LIBRARY_OUTPUT_DIRECTORY.
> 
> Just setting TEST_LIB_DIR to ${CMAKE_LIBRARY_OUTPUT_DIRECTORY} should be fine.

Ok, It will be more clear and safe. Thanks for pointing it.

[Byungwoo Lee]

Created attachment 165307 [details]
Patch

[Byungwoo Lee]

(In reply to comment #14)
> Created an attachment (id=165307) [details]
> Patch

I tested on Debug/Release and it works ok.

[Byungwoo Lee]

Created attachment 165308 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165308 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 165308 [details]
Patch

Clearing flags on attachment: 165308

Committed r129352: <http://trac.webkit.org/changeset/129352>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.