Re: [ros-dev] [ros-diffs] [spetreolle] 67053: [FREELDR] In a quest to better registry, don't break VSSolution builds. freeldr_pe is not in the same directory and copy doesn't care if you ask to concatenate C:\tomatoes, it a...

[Timo Kreuzer]

The comment "# Retrieve the full path to the generated file of the 
'freeldr_pe' target" Doesn't explain why it is done.
It rather looks like someone is doing something pointless and 
documenting in a comment that he's doing the pointless.

${CMAKE_CURRENT_BINARY_DIR} should be the "full path to the generated file". So
it seems completely pointless to add extra magic to get that file path. Your commit
message implies that this change fixes VSSolution builds, but it also doesn't explain
it. In fact I don't even understand that sentence.

If you don't add a proper comment to that thing, why it is needed, I 
promise I will have forgotten about this in a year and remove it again. :)

Thanks,
Timo

Am 04.04.2015 um 22:33 schrieb spetreolle(a)svn.reactos.org:
>   Author: spetreolle
>  Date: Sat Apr  4 20:33:18 2015
>  New Revision: 67053
>
>  URL: http://svn.reactos.org/svn/reactos?rev=67053&view=rev
>  Log:
>  [FREELDR]
>  In a quest to better registry,
>  don't break VSSolution builds.
>  freeldr_pe is not in the same directory and copy doesn't care if you ask to
> concatenate C:\tomatoes, it already has the first file.
>
>  Modified:
>       trunk/reactos/boot/freeldr/freeldr/CMakeLists.txt
>
>  Modified: trunk/reactos/boot/freeldr/freeldr/CMakeLists.txt
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/CMake…
>  ==============================================================================
>  --- trunk/reactos/boot/freeldr/freeldr/CMakeLists.txt	[iso-8859-1] (original)
>  +++ trunk/reactos/boot/freeldr/freeldr/CMakeLists.txt	[iso-8859-1] Sat Apr  4 20:33:18
> 2015
>  @@ -226,10 +226,13 @@
>    add_dependencies(freeldr_pe asm)
>    add_dependencies(freeldr_pe_dbg asm)
>    
>  +# Retrieve the full path to the generated file of the 'freeldr_pe' target
>  +get_target_property(_freeldr_pe_output_file freeldr_pe LOCATION)
>  +
>    concatenate_files(
>        ${CMAKE_CURRENT_BINARY_DIR}/freeldr.sys
>        ${CMAKE_CURRENT_BINARY_DIR}/frldr16.bin
>  -    ${CMAKE_CURRENT_BINARY_DIR}/freeldr_pe.dll)
>  +    ${_freeldr_pe_output_file})
>    
>    add_custom_target(freeldr ALL DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/freeldr.sys)
>    
>  @@ -240,7 +243,8 @@
>    concatenate_files(
>        ${CMAKE_CURRENT_BINARY_DIR}/setupldr.sys
>        ${CMAKE_CURRENT_BINARY_DIR}/frldr16.bin
>  -    ${CMAKE_CURRENT_BINARY_DIR}/freeldr_pe.dll)
>  +    ${_freeldr_pe_output_file})
>    
>    add_custom_target(setupldr ALL DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/setupldr.sys)
>    add_cd_file(TARGET setupldr FILE ${CMAKE_CURRENT_BINARY_DIR}/setupldr.sys DESTINATION
> loader NO_CAB FOR bootcd regtest)
>  +
>
>
>