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

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Byungwoo Lee]

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

[Byungwoo Lee]

I tested on Debug/Release and it works ok.

[Byungwoo Lee]

Created attachment 165337 [details]
Patch

[Patrick R. Gansterer]

Comment on attachment 165337 [details]
Patch

look ok to me

[Raphael Kubo da Costa (:rakuco)]

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 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]

Sorry, why do you want to remove this line?

-INCLUDE(OptionsCommon)

[Byungwoo Lee]

Created attachment 165393 [details]
Patch

[Byungwoo Lee]

(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 on attachment 165393 [details]
Patch

Looks fine, thanks.

[Ming Xie]

(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]

(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 on attachment 165393 [details]
Patch

Clearing flags on attachment: 165393

Committed r129447: <http://trac.webkit.org/changeset/129447>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.