The bug was this:

if((ARCH MATCHES amd64) AND ($ENV{VCINSTALLDIR}))

must be transformed into :

if((ARCH MATCHES amd64) AND (DEFINED ENV{VCINSTALLDIR}))

I saw this bug when I tried to compile ReactOS with the x64 build environment 
of Visual Studio. When trying to compile the boot sectors, the compilation 
failed because ml.exe couldn't been executed. After checking, I found that only 
ml64.exe (the x64 version of the Macro Assembler) was available under the x64 
build environment, and not the x86 version (ml.exe).
In msvc.cmake, the previous mentioned line was here to say that, if one was 
under x64 with VS, then one must use the x86 ml.exe for the build sectors. 
Since ml.exe was not directly available through the command line (i.e. by 
directing executing "ml.exe"), one had to execute it by specifying its full 
path. But clearly this was not the case, the compilation wanted to use "ml.exe" 
directly (verified by seeing the executed command line in verbose mode), and 
therefore was not found. Thus one should ask, why the CMake test always failed.
Then I found (references: 
http://www.cmake.org/pipermail/cmake/2011-October/046698.html and 
http://www.cmake.org/pipermail/cmake/2011-October/046704.html) that, checking 
the definition of an environment variable, must be done by checking for 
"DEFINED ENV{variable}" and not for "$ENV{variable}" only, as it was done 
before.

Regards,
Hermès BÉLUSCA - MAÏTO


-----Message d'origine-----
De : [email protected] [mailto:[email protected]] De la 
part de Timo Kreuzer
Envoyé : vendredi 24 août 2012 22:15
À : [email protected]
Objet : Re: [ros-dev] [ros-diffs] [akhaldi] 57151: [CMAKE/MSVC] Hermès Bélusca: 
* Fix a bug in the way we compiled the boot sectors on the x64 build. * 
Consistently set the mc compiler, like gcc builds. * Use proper variables when 
...


What was the bug?
I can't figure it out from the changes.

Am 24.08.2012 17:36, schrieb [email protected]:
> Author: akhaldi
> Date: Fri Aug 24 15:36:17 2012
> New Revision: 57151
>
> URL: http://svn.reactos.org/svn/reactos?rev=57151&view=rev
> Log:
> [CMAKE/MSVC]
> Hermès Bélusca:
> * Fix a bug in the way we compiled the boot sectors on the x64 build.
> * Consistently set the mc compiler, like gcc builds.
> * Use proper variables when referring to the compilers.
> See issue #7297 for more details.
>
> Modified:
>      trunk/reactos/cmake/msvc.cmake
>      trunk/reactos/toolchain-msvc.cmake
>
> Modified: trunk/reactos/cmake/msvc.cmake
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/cmake/msvc.cmake?rev=
> 57151&r1=57150&r2=57151&view=diff 
> ======================================================================
> ========
> --- trunk/reactos/cmake/msvc.cmake [iso-8859-1] (original)
> +++ trunk/reactos/cmake/msvc.cmake [iso-8859-1] Fri Aug 24 15:36:17 
> +++ 2012
> @@ -53,12 +53,12 @@
>       # We may temporarily use just the global defines, but this is not a 
> solution as some modules (minihal for example) apply additional definitions 
> to source files, so we get an incorrect build of such targets.
>       get_directory_property(definitions DEFINITIONS)
>       set(CMAKE_ASM_COMPILE_OBJECT
> -        "cl /nologo /X /I${REACTOS_SOURCE_DIR}/include/asm 
> /I${REACTOS_BINARY_DIR}/include/asm <FLAGS> ${definitions} /D__ASM__ 
> /D_USE_ML /EP /c <SOURCE> > <OBJECT>.tmp"
> +        "<CMAKE_C_COMPILER> /nologo /X /I${REACTOS_SOURCE_DIR}/include/asm 
> /I${REACTOS_BINARY_DIR}/include/asm <FLAGS> ${definitions} /D__ASM__ 
> /D_USE_ML /EP /c <SOURCE> > <OBJECT>.tmp"
>           "<CMAKE_ASM_COMPILER> /nologo /Cp /Fo<OBJECT> /c /Ta <OBJECT>.tmp")
>   else()
>       # NMake Makefiles
>       set(CMAKE_ASM_COMPILE_OBJECT
> -        "cl /nologo /X /I${REACTOS_SOURCE_DIR}/include/asm 
> /I${REACTOS_BINARY_DIR}/include/asm <FLAGS> <DEFINES> /D__ASM__ /D_USE_ML /EP 
> /c <SOURCE> > <OBJECT>.tmp"
> +        "<CMAKE_C_COMPILER> /nologo /X /I${REACTOS_SOURCE_DIR}/include/asm 
> /I${REACTOS_BINARY_DIR}/include/asm <FLAGS> <DEFINES> /D__ASM__ /D_USE_ML /EP 
> /c <SOURCE> > <OBJECT>.tmp"
>           "<CMAKE_ASM_COMPILER> /nologo /Cp /Fo<OBJECT> /c /Ta <OBJECT>.tmp")
>   endif()
>   
> @@ -67,14 +67,14 @@
>       add_compile_flags("/analyze")
>   elseif(_PREFAST_)
>       message("PREFAST enabled!")
> -    set(CMAKE_C_COMPILE_OBJECT "prefast cl ${CMAKE_START_TEMP_FILE} 
> ${CMAKE_CL_NOLOGO} <FLAGS> <DEFINES> /Fo<OBJECT> -c 
> <SOURCE>${CMAKE_END_TEMP_FILE}"
> +    set(CMAKE_C_COMPILE_OBJECT "prefast <CMAKE_C_COMPILER> 
> ${CMAKE_START_TEMP_FILE} ${CMAKE_CL_NOLOGO} <FLAGS> <DEFINES> /Fo<OBJECT> -c 
> <SOURCE>${CMAKE_END_TEMP_FILE}"
>       "prefast LIST")
> -    set(CMAKE_CXX_COMPILE_OBJECT "prefast cl ${CMAKE_START_TEMP_FILE} 
> ${CMAKE_CL_NOLOGO} <FLAGS> <DEFINES> /TP /Fo<OBJECT> -c 
> <SOURCE>${CMAKE_END_TEMP_FILE}"
> +    set(CMAKE_CXX_COMPILE_OBJECT "prefast <CMAKE_CXX_COMPILER> 
> ${CMAKE_START_TEMP_FILE} ${CMAKE_CL_NOLOGO} <FLAGS> <DEFINES> /TP /Fo<OBJECT> 
> -c <SOURCE>${CMAKE_END_TEMP_FILE}"
>       "prefast LIST")
>       set(CMAKE_C_LINK_EXECUTABLE
> -    "cl ${CMAKE_CL_NOLOGO} <OBJECTS> ${CMAKE_START_TEMP_FILE} <FLAGS> 
> /Fe<TARGET> -link /implib:<TARGET_IMPLIB> 
> /version:<TARGET_VERSION_MAJOR>.<TARGET_VERSION_MINOR> <CMAKE_C_LINK_FLAGS> 
> <LINK_FLAGS> <LINK_LIBRARIES>${CMAKE_END_TEMP_FILE}")
> +    "<CMAKE_C_COMPILER> ${CMAKE_CL_NOLOGO} <OBJECTS> 
> + ${CMAKE_START_TEMP_FILE} <FLAGS> /Fe<TARGET> -link 
> + /implib:<TARGET_IMPLIB> 
> + /version:<TARGET_VERSION_MAJOR>.<TARGET_VERSION_MINOR> 
> + <CMAKE_C_LINK_FLAGS> <LINK_FLAGS> 
> + <LINK_LIBRARIES>${CMAKE_END_TEMP_FILE}")
>       set(CMAKE_CXX_LINK_EXECUTABLE
> -    "cl ${CMAKE_CL_NOLOGO} <OBJECTS> ${CMAKE_START_TEMP_FILE} <FLAGS> 
> /Fe<TARGET> -link /implib:<TARGET_IMPLIB> 
> /version:<TARGET_VERSION_MAJOR>.<TARGET_VERSION_MINOR> <CMAKE_CXX_LINK_FLAGS> 
> <LINK_FLAGS> <LINK_LIBRARIES>${CMAKE_END_TEMP_FILE}")
> +    "<CMAKE_CXX_COMPILER> ${CMAKE_CL_NOLOGO} <OBJECTS> 
> + ${CMAKE_START_TEMP_FILE} <FLAGS> /Fe<TARGET> -link 
> + /implib:<TARGET_IMPLIB> 
> + /version:<TARGET_VERSION_MAJOR>.<TARGET_VERSION_MINOR> 
> + <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS> 
> + <LINK_LIBRARIES>${CMAKE_END_TEMP_FILE}")
>   endif()
>   
>   set(CMAKE_RC_CREATE_SHARED_LIBRARY ${CMAKE_C_CREATE_SHARED_LIBRARY}) 
> @@ -143,7 +143,7 @@
>           # Compile the generated asm stub file
>           add_custom_command(
>               OUTPUT ${_asm_stubs_file}.obj
> -            COMMAND ml /Cp /Fo${_asm_stubs_file}.obj /c /Ta 
> ${_asm_stubs_file}
> +            COMMAND ${CMAKE_ASM_COMPILER} /Cp 
> + /Fo${_asm_stubs_file}.obj /c /Ta ${_asm_stubs_file}
>               DEPENDS ${_asm_stubs_file})
>       else()
>           # be clear about the "language"
> @@ -202,27 +202,28 @@
>   endfunction()
>   
>   macro(macro_mc FLAG FILE)
> -    set(COMMAND_MC mc ${FLAG} -r ${REACTOS_BINARY_DIR}/include/reactos -h 
> ${REACTOS_BINARY_DIR}/include/reactos ${CMAKE_CURRENT_SOURCE_DIR}/${FILE}.mc)
> +    set(COMMAND_MC ${CMAKE_MC_COMPILER} ${FLAG} -r 
> + ${REACTOS_BINARY_DIR}/include/reactos -h 
> + ${REACTOS_BINARY_DIR}/include/reactos 
> + ${CMAKE_CURRENT_SOURCE_DIR}/${FILE}.mc)
>   endmacro()
>   
>   #pseh workaround
>   set(PSEH_LIB "pseh")
>   
> -# Use full path for ml when using x64 VS -if((ARCH MATCHES amd64) AND 
> ($ENV{VCINSTALLDIR}))
> +# Use a full path for the x86 version of ml when using x64 VS.
> +# It's not a problem when using the DDK/WDK because, in x64 mode, # 
> +both the x86 and x64 versions of ml are available.
> +if((ARCH MATCHES amd64) AND (DEFINED ENV{VCINSTALLDIR}))
>       set(CMAKE_ASM16_COMPILER $ENV{VCINSTALLDIR}/bin/ml.exe)
>   else()
>       set(CMAKE_ASM16_COMPILER ml.exe)
>   endif()
>   
>   function(CreateBootSectorTarget _target_name _asm_file _binary_file 
> _base_address)
> -
>       set(_object_file ${_binary_file}.obj)
>       set(_temp_file ${_binary_file}.tmp)
>   
>       add_custom_command(
>           OUTPUT ${_temp_file}
> -        COMMAND cl /nologo /X /I${REACTOS_SOURCE_DIR}/include/asm 
> /I${REACTOS_BINARY_DIR}/include/asm /D__ASM__ /D_USE_ML /EP /c ${_asm_file} > 
> ${_temp_file}
> +        COMMAND ${CMAKE_C_COMPILER} /nologo /X 
> + /I${REACTOS_SOURCE_DIR}/include/asm 
> + /I${REACTOS_BINARY_DIR}/include/asm /D__ASM__ /D_USE_ML /EP /c 
> + ${_asm_file} > ${_temp_file}
>           DEPENDS ${_asm_file})
>   
>       add_custom_command(
>
> Modified: trunk/reactos/toolchain-msvc.cmake
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/toolchain-msvc.cmake?
> rev=57151&r1=57150&r2=57151&view=diff
> ======================================================================
> ========
> --- trunk/reactos/toolchain-msvc.cmake [iso-8859-1] (original)
> +++ trunk/reactos/toolchain-msvc.cmake [iso-8859-1] Fri Aug 24 
> +++ 15:36:17 2012
> @@ -6,6 +6,7 @@
>   # which compilers to use for C and C++
>   set(CMAKE_C_COMPILER cl)
>   set(CMAKE_CXX_COMPILER cl)
> +set(CMAKE_MC_COMPILER mc)
>   set(CMAKE_RC_COMPILER rc)
>   if(${ARCH} MATCHES amd64)
>       set(CMAKE_ASM_COMPILER ml64)
>
>
>


_______________________________________________
Ros-dev mailing list
[email protected]
http://www.reactos.org/mailman/listinfo/ros-dev

_______________________________________________
Ros-dev mailing list
[email protected]
http://www.reactos.org/mailman/listinfo/ros-dev

Reply via email to