There is hardcoded 'lib' and 'bin' in CMakeLists.txt, and this might be better to be changed to 'LIB_INSTALL_DIR' and 'EXEC_INSTALL_DIR'.
I tested on Debug/Release and it works ok.
Created attachment 165337 [details] Patch
Comment on attachment 165337 [details] Patch look ok to me
Comment on attachment 165337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165337&action=review > ChangeLog:3 > + Change hardcoded 'lib' and 'bin' in CMakeLists.txt to 'LIB_INSTALL_DIR' and 'EXEC_INSTALL_DIR'. Please use the [CMake] prefix for CMake-related bugs. > ChangeLog:11 > + Currently, 'LIB_INSTALL_DIR' and 'EXEC_INSTALL_DIR' is used for > + hardcoded 'lib' and 'bin' directory. > + Apply this to set the below variables. > + CMAKE_ARCHIVE_OUTPUT_DIRECTORY, CMAKE_LIBRARY_OUTPUT_DIRECTORY, CMAKE_RUNTIME_OUTPUT_DIRECTORY This explanation is hard to understand. Just saying something like "Use the value of LIB_INSTALL_DIR and EXEC_INSTALL_DIR instead of hardcoding lib and bin for CMAKE_{ARCHIVE,LIBRARY,RUNTIME}_OUTPUT_DIRECTORY" would be easier to grasp.
Comment on attachment 165337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165337&action=review >> ChangeLog:3 >> + Change hardcoded 'lib' and 'bin' in CMakeLists.txt to 'LIB_INSTALL_DIR' and 'EXEC_INSTALL_DIR'. > > Please use the [CMake] prefix for CMake-related bugs. Ok~ I'll add it. >> ChangeLog:11 >> + CMAKE_ARCHIVE_OUTPUT_DIRECTORY, CMAKE_LIBRARY_OUTPUT_DIRECTORY, CMAKE_RUNTIME_OUTPUT_DIRECTORY > > This explanation is hard to understand. Just saying something like "Use the value of LIB_INSTALL_DIR and EXEC_INSTALL_DIR instead of hardcoding lib and bin for CMAKE_{ARCHIVE,LIBRARY,RUNTIME}_OUTPUT_DIRECTORY" would be easier to grasp. Ok~ it'll be more clear :)
Sorry, why do you want to remove this line? -INCLUDE(OptionsCommon)
Created attachment 165393 [details] Patch
(In reply to comment #6) > Sorry, why do you want to remove this line? > > -INCLUDE(OptionsCommon) The line is not removed. it is moved to the above (#132).
Comment on attachment 165393 [details] Patch Looks fine, thanks.
(In reply to comment #8) > (In reply to comment #6) > > Sorry, why do you want to remove this line? > > > > -INCLUDE(OptionsCommon) > The line is not removed. it is moved to the above (#132). I don't know how I missed that. Sorry! :(
(In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #6) > > > Sorry, why do you want to remove this line? > > > > > > -INCLUDE(OptionsCommon) > > The line is not removed. it is moved to the above (#132). > > I don't know how I missed that. Sorry! :( No problem :)
Comment on attachment 165393 [details] Patch Clearing flags on attachment: 165393 Committed r129447: <http://trac.webkit.org/changeset/129447>
All reviewed patches have been landed. Closing bug.