On 29/07/2015 14:37, Brad King wrote:
Hi Brad,
On 07/28/2015 06:02 PM, Bill Somerville wrote:
...
As there doesn't seem to be  a reliable way of detecting MinGW callers
of fixup_bundle() may have to set the variable gp_tool to "objdump" if
dumpbin.exe  is  installed on  the  build  machine  to stop  it  using
dumpbin.exe for library dependency walking.
Is there a reason not to look for objdump before dumpbin?
I was under the impression that dumpbin was selected first for non-MinGW Windows builds, I have no idea if objdump works with binaries produced by compilers other than GNU/CLang. This is why I commented about detecting MinGW, there are mentions of a MINGW CMake system variable but it doesn't seem to exist.

This  patch  also   adds  command  status  checking   to  the  various
execute_process() invocations  in GetPrerequisites.cmake so  that they
don't fail silently producing invalid install packages.
If you're comfortable enough with Git, please split this part out into
a preceding patch with its own explanation in its commit message.  That
will clarify which hunks in the patch belong to which change.
No problem with that, I will do that shortly.

+  if(gp_cmd_filter)             # filter is for performance
+    execute_process(
+      COMMAND ${gp_cmd} ${gp_cmd_args} ${target}
+      COMMAND ${gp_grep_cmd} ${gp_cmd_filter}
+      RESULT_VARIABLE gp_rv
+      OUTPUT_VARIABLE gp_cmd_ov
+      ERROR_VARIABLE gp_ev
+      )
+  else()
+    execute_process(
+      COMMAND ${gp_cmd} ${gp_cmd_args} ${target}
+      RESULT_VARIABLE gp_rv
+      OUTPUT_VARIABLE gp_cmd_ov
+      ERROR_VARIABLE gp_ev
+      )
+  endif()
This can be simplified as:

     execute_process(
       COMMAND ${gp_cmd} ${gp_cmd_args} ${target}
       ${gp_cmd_maybe_filter}
       RESULT_VARIABLE gp_rv
       OUTPUT_VARIABLE gp_cmd_ov
       ERROR_VARIABLE gp_ev
       )

where ``gp_cmd_maybe_filter`` is initialized to empty and later
possibly set as:

   if(gp_grep_cmd)
     set(gp_cmd_maybe_filter
       COMMAND ${gp_grep_cmd} "^[[:blank:]]*DLL Name: "
       )
   endif()
I tried that first but couldn't get it to work, maybe because I didn't initialize it to empty, I will try again.

Thanks,
-Brad

Thanks for the review,
regards
Bill Somerville.
--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to