Bug 97419

Summary:

[CMake] Change hardcoded 'lib' and 'bin' in CMakeLists.txt to 'LIB_INSTALL_DIR' and 'EXEC_INSTALL_DIR'

Product:

WebKit

Reporter:

Byungwoo Lee <bw80.lee>

Component:

WebKit Misc.

Assignee:

Byungwoo Lee <bw80.lee>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, mxie, paroga, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Byungwoo Lee

Reported 2012-09-23 19:06:03 PDT

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'.

Attachments
Patch (2.40 KB, patch) 2012-09-24 02:38 PDT, Byungwoo Lee	no flags	Details Formatted Diff Diff
Patch (2.30 KB, patch) 2012-09-24 08:57 PDT, Byungwoo Lee	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Byungwoo Lee

Comment 1 2012-09-23 21:26:19 PDT

I tested on Debug/Release and it works ok.

Byungwoo Lee

Comment 2 2012-09-24 02:38:41 PDT

Created attachment 165337 [details] Patch

Patrick R. Gansterer

Comment 3 2012-09-24 08:35:13 PDT

Comment on attachment 165337 [details] Patch look ok to me

Raphael Kubo da Costa (:rakuco)

Comment 4 2012-09-24 08:36:13 PDT

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.

Byungwoo Lee

Comment 5 2012-09-24 08:47:24 PDT

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 :)

Ming Xie

Comment 6 2012-09-24 08:53:07 PDT

Sorry, why do you want to remove this line? -INCLUDE(OptionsCommon)

Byungwoo Lee

Comment 7 2012-09-24 08:57:30 PDT

Created attachment 165393 [details] Patch

Byungwoo Lee

Comment 8 2012-09-24 09:23:36 PDT

(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).

Raphael Kubo da Costa (:rakuco)

Comment 9 2012-09-24 10:10:55 PDT

Comment on attachment 165393 [details] Patch Looks fine, thanks.

Ming Xie

Comment 10 2012-09-24 10:28:50 PDT

(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! :(

Byungwoo Lee

Comment 11 2012-09-24 18:31:18 PDT

(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 :)

WebKit Review Bot

Comment 12 2012-09-24 20:21:54 PDT

Comment on attachment 165393 [details] Patch Clearing flags on attachment: 165393 Committed r129447: <http://trac.webkit.org/changeset/129447>

WebKit Review Bot

Comment 13 2012-09-24 20:21:58 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.