Merge pull request #608 from mikepurvis/fix-build-type

Check build_type with function from catkin_pkg.

catkin_tools/common.py

@@ -610,22 +610,6 @@ def wide_log(msg, **kwargs):
         pass
 
 
-def get_build_type(package):
-    """Returns the build type for a given package.
-
-    :param package: package object
-    :type package: :py:class:`catkin_pkg.package.Package`
-    :returns: build type of the package, e.g. 'catkin' or 'cmake'
-    :rtype: str
-    """
-    export_tags = [e.tagname for e in package.exports]
-    if 'build_type' in export_tags:
-        build_type_tag = [e.content for e in package.exports if e.tagname == 'build_type'][0]
-    else:
-        build_type_tag = 'catkin'
-    return build_type_tag
-
-
 def find_enclosing_package(search_start_path=None, ws_path=None, warnings=None, symlinks=True):
     """Get the package containing a specific directory.
 

catkin_tools/execution/controllers.py

@@ -695,9 +695,8 @@ def run(self):
                 if self.show_buffered_stdout:
                     if len(event.data['interleaved']) > 0:
                         lines = [
-                            l
-                            for l in event.data['interleaved'].splitlines(True)
-                            if (self.show_compact_io is False or len(l.strip()) > 0)
+                            line for line in event.data['interleaved'].splitlines(True)
+                            if (self.show_compact_io is False or len(line.strip()) > 0)
                         ]
                     else:
                         header_border = None
@@ -706,9 +705,8 @@ def run(self):
                 elif self.show_buffered_stderr:
                     if len(event.data['stderr']) > 0:
                         lines = [
-                            l
-                            for l in event.data['stderr'].splitlines(True)
-                            if (self.show_compact_io is False or len(l.strip()) > 0)
+                            line for line in event.data['stderr'].splitlines(True)
+                            if (self.show_compact_io is False or len(line.strip()) > 0)
                         ]
                         lines_target = sys.stderr
                     else:
@@ -768,4 +766,4 @@ def format_interleaved_lines(self, data):
         template = '\r{}\r{}'.format(' ' * terminal_width(), prefix)
         suffix = clr('@|')
 
-        return ''.join(template + l + suffix for l in data['data'].splitlines(True))
+        return ''.join(template + line + suffix for line in data['data'].splitlines(True))

catkin_tools/jobs/commands/cmake.py

@@ -112,7 +112,7 @@ def color_lines(self, data):
         # of direclty splitting lines, we should buffer the data lines until
         # the last character is a line break
         lines = decoded_data.splitlines(True)  # Keep line breaks
-        colored_lines = [self.colorize_cmake(l) for l in lines]
+        colored_lines = [self.colorize_cmake(line) for line in lines]
         colored_data = ''.join(colored_lines)
         encoded_data = self._encode(colored_data)
         return encoded_data

catkin_tools/verbs/catkin_build/build.py

@@ -43,7 +43,6 @@
 
 from catkin_tools.common import FakeLock, expand_glob_package
 from catkin_tools.common import format_time_delta
-from catkin_tools.common import get_build_type
 from catkin_tools.common import get_cached_recursive_build_depends_in_workspace
 from catkin_tools.common import get_recursive_run_depends_in_workspace
 from catkin_tools.common import log
@@ -280,9 +279,10 @@ def build_isolated_workspace(
     if os.path.exists(context.build_space_abs):
         if os.path.isfile(context.build_space_abs):
             sys.exit(clr(
-                "[build] @{rf}Error:@| Build space '{0}' exists but is a file and not a folder."
+                "[build] @{rf}Error:@| " +
+                "Build space '{0}' exists but is a file and not a folder."
                 .format(context.build_space_abs)))
-    # If it dosen't exist, create it
+    # If it doesn't exist, create it
     else:
         log("[build] Creating build space: '{0}'".format(context.build_space_abs))
         os.makedirs(context.build_space_abs)
@@ -500,7 +500,7 @@ def build_isolated_workspace(
             pre_clean=pre_clean)
 
         # Create the job based on the build type
-        build_type = get_build_type(pkg)
+        build_type = pkg.get_build_type()
 
         if build_type in build_job_creators:
             jobs.append(build_job_creators[build_type](**build_job_kwargs))

catkin_tools/verbs/catkin_build/cli.py

@@ -37,7 +37,6 @@
 from catkin_tools.argument_parsing import add_cmake_and_make_and_catkin_make_args
 from catkin_tools.argument_parsing import configure_make_args
 
-from catkin_tools.common import get_build_type
 from catkin_tools.common import getcwd
 from catkin_tools.common import is_tty
 from catkin_tools.common import log
@@ -215,7 +214,7 @@ def dry_run(context, packages, no_deps, start_with):
     max_name_len = str(max([len(pkg.name) for pth, pkg in packages_to_be_built]))
     prefix = clr('@{pf}' + ('------ ' if start_with else '- ') + '@|')
     for pkg_path, pkg in packages_to_be_built:
-        build_type = get_build_type(pkg)
+        build_type = pkg.get_build_type()
         if build_type == 'catkin' and 'metapackage' in [e.tagname for e in pkg.exports]:
             build_type = 'metapackage'
         if start_with and pkg.name == start_with:

catkin_tools/verbs/catkin_clean/clean.py

@@ -40,7 +40,7 @@
 from catkin_tools.execution.executor import execute_jobs
 from catkin_tools.execution.executor import run_until_complete
 
-from catkin_tools.common import get_build_type, expand_glob_package
+from catkin_tools.common import expand_glob_package
 from catkin_tools.common import get_recursive_build_dependents_in_workspace
 from catkin_tools.common import wide_log
 
@@ -145,7 +145,7 @@ def clean_packages(
             clean_install=True)
 
         # Create the job based on the build type
-        build_type = get_build_type(pkg)
+        build_type = pkg.get_build_type()
 
         if build_type in clean_job_creators:
             jobs.append(clean_job_creators[build_type](**clean_job_kwargs))

tests/system/resources/catkin_pkgs/build_type_condition/CMakeLists.txt

@@ -0,0 +1,2 @@
+cmake_minimum_required (VERSION 2.6)
+project(build_type_condition)

tests/system/resources/catkin_pkgs/build_type_condition/package.xml

@@ -0,0 +1,14 @@
+<package>
+  <name>build_type_condition</name>
+  <description>Package with conditional build_type element.</description>
+  <version>0.1.0</version>
+  <license>BSD</license>
+  <maintainer email="todo@todo.todo">todo</maintainer>
+
+  <buildtool_depend>catkin</buildtool_depend>
+  <buildtool_depend>ament</buildtool_depend>
+
+  <export>
+    <build_type condition="$ROS_VERSION == 2">ament_cmake</build_type>
+  </export>
+</package>

tests/system/verbs/catkin_build/test_build.py

@@ -266,7 +266,7 @@ def test_install_cmake():
     """Test building and installing cmake packages without DESTDIR."""
     with redirected_stdio() as (out, err):
         with workspace_factory() as wf:
-            print(os.getcwd)
+            print(os.getcwd())
             wf.build()
             shutil.copytree(
                 os.path.join(RESOURCES_DIR, 'cmake_pkgs'),
@@ -281,7 +281,7 @@ def test_install_cmake_destdir():
     """Test building and installing cmake packages with DESTDIR."""
     with redirected_stdio() as (out, err):
         with workspace_factory() as wf:
-            print(os.getcwd)
+            print(os.getcwd())
             wf.build()
             shutil.copytree(
                 os.path.join(RESOURCES_DIR, 'cmake_pkgs'),
@@ -300,7 +300,7 @@ def test_install_catkin_destdir():
     """Test building and installing catkin packages with DESTDIR."""
     with redirected_stdio() as (out, err):
         with workspace_factory() as wf:
-            print(os.getcwd)
+            print(os.getcwd())
             wf.build()
             shutil.copytree(
                 os.path.join(RESOURCES_DIR, 'catkin_pkgs', 'products_0'),
@@ -332,11 +332,11 @@ def test_pkg_with_unicode_names():
     """Test building a package with unicode file names."""
     with redirected_stdio() as (out, err):
         with workspace_factory() as wf:
-            print(os.getcwd)
+            print(os.getcwd())
             wf.build()
             shutil.copytree(
                 os.path.join(RESOURCES_DIR, 'catkin_pkgs', 'products_unicode'),
-                os.path.join('src/cmake_pkgs'))
+                os.path.join('src/products_unicode'))
 
             assert catkin_success(['config', '--link-devel'])
             assert catkin_success(BUILD)
@@ -361,3 +361,22 @@ def test_glob_pattern_build():
                 assert not os.path.exists(os.path.join('build', 'pkg_7'))
                 assert not os.path.exists(os.path.join('build', 'pkg_8'))
                 assert not os.path.exists(os.path.join('build', 'pkg_9'))
+
+
+def test_pkg_with_conditional_build_type():
+    """Test building a dual catkin/ament package."""
+    with redirected_stdio() as (out, err):
+        with workspace_factory() as wf:
+            print(os.getcwd())
+            wf.build()
+            shutil.copytree(
+                os.path.join(RESOURCES_DIR, 'catkin_pkgs', 'build_type_condition'),
+                os.path.join('src/build_type_condition'))
+
+            assert catkin_success(['config', '--merge-devel'])
+            assert catkin_success(BUILD)
+
+            # Currently the build verb skips over packages it doesn't know how to build.
+            # So we have to infer this skipping by checking the build directory.
+            msg = "Package with ROS 2 conditional build_type was skipped."
+            assert os.path.exists(os.path.join('build', 'build_type_condition')), msg