Updates per comments from @wjwwood

Renamed the CATKIN_MULTIARCH_*_DESTINATION variables to CATKIN_*_ENVIRONMENT_PATHS. Use isinstance to verify that variables are lists. Check Popen return code instead of std_err. Fix logic error in catkin_generate_environment.cmake
scpeters · May 5, 2014 · 655012a · 655012a
1 parent 35d9827
commit 655012a
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 14 deletions.
diff --git a/cmake/catkin_generate_environment.cmake b/cmake/catkin_generate_environment.cmake
@@ -11,8 +11,8 @@ function(catkin_generate_environment)
   endif()
 
   # get multiarch name
-  set(CATKIN_MULTIARCH_LIB_DESTINATION "'${CATKIN_GLOBAL_LIB_DESTINATION}'")
-  set(CATKIN_MULTIARCH_PKGCONFIG_DESTINATION "os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', 'pkgconfig')")
+  set(CATKIN_LIB_ENVIRONMENT_PATHS "'${CATKIN_GLOBAL_LIB_DESTINATION}'")
+  set(CATKIN_PKGCONFIG_ENVIRONMENT_PATHS "os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', 'pkgconfig')")
   if (UNIX AND NOT APPLE)
     # Two step looking for multiarch support: check for gcc -print-multiarch
     # and, if failed, try to run dpkg-architecture
@@ -21,16 +21,16 @@ function(catkin_generate_environment)
                     OUTPUT_STRIP_TRAILING_WHITESPACE
                     ERROR_QUIET
     )
-    if (NOT "${CATKIN_MULTIARCH}" STREQUAL "")
+    if ("${CATKIN_MULTIARCH}" STREQUAL "")
       execute_process(COMMAND dpkg-architecture -qDEB_HOST_MULTIARCH
                       OUTPUT_VARIABLE CATKIN_MULTIARCH
                       OUTPUT_STRIP_TRAILING_WHITESPACE
                       ERROR_QUIET
       )
     endif()
     if (NOT "${CATKIN_MULTIARCH}" STREQUAL "")
-      set(CATKIN_MULTIARCH_LIB_DESTINATION "['${CATKIN_GLOBAL_LIB_DESTINATION}', os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', '${CATKIN_MULTIARCH}')]")
-      set(CATKIN_MULTIARCH_PKGCONFIG_DESTINATION "[os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', 'pkgconfig'), os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', '${CATKIN_MULTIARCH}', 'pkgconfig')]")
+      set(CATKIN_LIB_ENVIRONMENT_PATHS "['${CATKIN_GLOBAL_LIB_DESTINATION}', os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', '${CATKIN_MULTIARCH}')]")
+      set(CATKIN_PKGCONFIG_ENVIRONMENT_PATHS "[os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', 'pkgconfig'), os.path.join('${CATKIN_GLOBAL_LIB_DESTINATION}', '${CATKIN_MULTIARCH}', 'pkgconfig')]")
     endif()
   endif()
 

diff --git a/cmake/templates/_setup_util.py.in b/cmake/templates/_setup_util.py.in
@@ -52,9 +52,9 @@ IS_WINDOWS = (system == 'Windows')
 ENV_VAR_SUBFOLDERS = {
     'CMAKE_PREFIX_PATH': '',
     'CPATH': 'include',
-    'LD_LIBRARY_PATH' if not IS_DARWIN else 'DYLD_LIBRARY_PATH': @CATKIN_MULTIARCH_LIB_DESTINATION@,
+    'LD_LIBRARY_PATH' if not IS_DARWIN else 'DYLD_LIBRARY_PATH': @CATKIN_LIB_ENVIRONMENT_PATHS@,
     'PATH': '@CATKIN_GLOBAL_BIN_DESTINATION@',
-    'PKG_CONFIG_PATH': @CATKIN_MULTIARCH_PKGCONFIG_DESTINATION@,
+    'PKG_CONFIG_PATH': @CATKIN_PKGCONFIG_ENVIRONMENT_PATHS@,
     'PYTHONPATH': '@PYTHON_INSTALL_DIR@',
 }
 
@@ -69,7 +69,7 @@ def rollback_env_variables(environ, env_var_subfolders):
     unmodified_environ = copy.copy(environ)
     for key in sorted(env_var_subfolders.keys()):
         subfolders = env_var_subfolders[key]
-        if type(subfolders) != type([]):
+        if not isinstance(subfolders, list):
             subfolders = [subfolders]
         for subfolder in subfolders:
             value = _rollback_env_variable(unmodified_environ, key, subfolder)
@@ -154,7 +154,7 @@ def _prefix_env_variable(environ, name, paths, subfolders):
     environ_paths = [path for path in value.split(os.pathsep) if path]
     checked_paths = []
     for path in paths:
-        if type(subfolders) != type([]):
+        if not isinstance(subfolders, list):
             subfolders = [subfolders]
         for subfolder in subfolders:
             path_tmp = path

diff --git a/python/catkin/builder.py b/python/catkin/builder.py
@@ -252,10 +252,11 @@ def get_multiarch():
     # this function returns the suffix for lib directories on supported systems or an empty string
     # it uses two step approach to look for multiarch: first run gcc -print-multiarch and if
     # failed try to run dpkg-architecture
-    out, err = subprocess.Popen(
+    p = subprocess.Popen(
         ['gcc -print-multiarch'],
-        stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True).communicate()
-    if err:
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+    out, err = p.communicate()
+    if p.returncode != 0:
       out, err = subprocess.Popen(
           ['dpkg-architecture -qDEB_HOST_MULTIARCH'],
           stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True).communicate()

diff --git a/test/unit_tests/test_setup_util.py b/test/unit_tests/test_setup_util.py
@@ -7,8 +7,8 @@
 
 data = configure_file(os.path.join(os.path.dirname(__file__), '..', '..', 'cmake', 'templates', '_setup_util.py.in'),
                       {
-                          'CATKIN_MULTIARCH_LIB_DESTINATION': '\'lib\'',
-                          'CATKIN_MULTIARCH_PKGCONFIG_DESTINATION': 'os.path.join(\'lib\', \'pkgconfig\')',
+                          'CATKIN_LIB_ENVIRONMENT_PATHS': '\'lib\'',
+                          'CATKIN_PKGCONFIG_ENVIRONMENT_PATHS': 'os.path.join(\'lib\', \'pkgconfig\')',
                           'CATKIN_GLOBAL_BIN_DESTINATION': 'bin',
                           'PYTHON_INSTALL_DIR': 'pythonX.Y/packages',
                           'CMAKE_PREFIX_PATH_AS_IS': '',