PR feedback

Author: cottsay

.travis.yml

@@ -57,17 +57,14 @@ matrix:
         # install colcon for test results
         - pip install colcon-core colcon-test-result
         - python setup.py install
-        - mkdir -p job_under job_over/underlay
-        - ln -s .. job_under/ros_buildfarm
-        - ln -s .. job_over/ros_buildfarm
+        - mkdir job job/underlay && cd job
+        - ln -s .. ros_buildfarm
       script:
-        - cd job_under
-        - generate_ci_script.py $CONFIG2_URL $ROS2_DISTRO_NAME default $OS_NAME $OS_CODE_NAME2 $ARCH --packages-select $UNDERLAY_PACKAGES --depth-before 100 > job_under.sh
-        - (. job_under.sh -y; exit $test_result_RC)
-        - cd ../job_over
-        - tar -xjf ../job_under/ros2-$ROS2_DISTRO_NAME-linux-$OS_CODE_NAME2-$ARCH-ci.tar.bz2 -C underlay
-        - generate_ci_script.py $CONFIG2_URL $ROS2_DISTRO_NAME default $OS_NAME $OS_CODE_NAME2 $ARCH --packages-select $OVERLAY_PACKAGES --depth-before 100 --build-ignore $UNDERLAY_PACKAGES --underlay-source-path underlay/ros2-linux > job_over.sh
-        - (. job_over.sh -y; exit $test_result_RC)
+        - generate_ci_script.py $CONFIG2_URL $ROS2_DISTRO_NAME default $OS_NAME $OS_CODE_NAME2 $ARCH --packages-select $UNDERLAY_PACKAGES --depth-before 100 > job.sh
+        - (. job.sh -y; exit $test_result_RC)
+        - tar -xjf ros2-$ROS2_DISTRO_NAME-linux-$OS_CODE_NAME2-$ARCH-ci.tar.bz2 -C underlay
+        - generate_ci_script.py $CONFIG2_URL $ROS2_DISTRO_NAME default $OS_NAME $OS_CODE_NAME2 $ARCH --packages-select $OVERLAY_PACKAGES --depth-before 100 --build-ignore $UNDERLAY_PACKAGES --underlay-source-path underlay/ros2-linux > job.sh
+        - (. job.sh -y; exit $test_result_RC)
     - language: python
       python: "3.6"
       sudo: required

doc/configuration_options.rst

@@ -393,7 +393,7 @@ The following options are valid as keys in the ``_config`` dict under
   20-default.list file from your forked rosdistro repository.
 
 Specific options in CI build files
----------------------------------------
+----------------------------------
 
 This yaml file defines the configuration for *CI* jobs.
 
@@ -404,16 +404,18 @@ The file format is specified by the following information:
 
 The following options are valid in version ``1`` (beside the generic options):
 
-* ``build_tool``: the build tool to use. The following are valid values:
+* ``build_tool``: the build tool to use.
+  The following are valid values:
 
   * ``catkin_make_isolated`` (default)
   * ``colcon``
 
 * ``foundation_packages``: a list of packages which should be installed by
   default before any of the dependencies necessary to build the packages in the
-  workspace. Since not all packages in the workspace are necessarily ROS
-  packages, rosdep may be unable to detect and install the prerequisites for
-  those packages, so those prerequisite packages may need to be listed here.
+  workspace.
+  Since not all packages in the workspace are necessarily ROS packages, rosdep
+  may be unable to detect and install the prerequisites for those packages, so
+  those prerequisite packages may need to be listed here.
 
 * ``jenkins_job_priority``: the job priority of *CI* jobs.
 

doc/jobs/ci_jobs.rst

@@ -44,8 +44,6 @@ It generates three Dockerfiles: one to perform the *create-workspace* task to
 populate the workspace and enumerate prerequisites, one to perform the
 *build-and-install* task, and one to perform the *build-and-test* task.
 
-A typical job runs 4 Dockerfiles in all.
-
 Create workspace
 ^^^^^^^^^^^^^^^^
 
@@ -56,7 +54,7 @@ The task performs the following steps:
 
 * Prepare the package sources
 
-  * Fetches the `.repos` file(s)
+  * Fetches the ``.repos`` file(s)
   * Fetches the source repositories containing the package(s) to be built
   * If necessary, switches the branch in those repositories
 
@@ -86,11 +84,11 @@ Known limitations
 ^^^^^^^^^^^^^^^^^
 
 System dependency enumeration happens for all ROS packages that are part of the
-non-underlay workspace. This means that any non-ROS packages present in that
-workspace may need their dependencies explicitly called out for inclusion in
-the foundation package list, and also means that a missing dependency may be
-occluded by another package in the workspace correctly declaring the same
-dependency.
+non-underlay workspace.
+This means that any non-ROS packages present in that workspace may need their
+dependencies explicitly called out for inclusion in the foundation package list,
+and also means that a missing dependency may be occluded by another package in
+the workspace correctly declaring the same dependency.
 
 Run the *CI* job locally
 ------------------------
@@ -111,10 +109,10 @@ from ROS *Crystal* for Ubuntu *Bionic* *amd64*:
 Return code
 -----------
 
-The return code of the generated script will be zero if it successfully performed
-the build and ran the test even if some tests failed. By setting the environment
-variable `ABORT_ON_TEST_FAILURE=1` the return code will also be non-zero in case
-of failed tests.
+The return code of the generated script will be zero if it successfully
+performed the build and ran the test even if some tests failed.
+By setting the environment variable ``ABORT_ON_TEST_FAILURE=1`` the return code
+will also be non-zero in case of failed tests.
 
 Instead of invoking the generated script it can also be *sourced*:
 
@@ -127,12 +125,13 @@ The return code of the invocation of ``catkin_tests_results`` /
 ``test_result_RC``.
 
 Run the *CI* job on Travis
------------------------------
+--------------------------
 
 Since it is easy to run a *CI* job locally it can also be run on Travis to
-either test every commit or pull request. The setup and invocation is the same
-as locally. The following .travis.yml template is a good starting point and is
-ready to be use:
+either test every commit or pull request.
+The setup and invocation is the same as locally.
+The following .travis.yml template is a good starting point and is ready to be
+used:
 
 .. code:: yaml
 
@@ -179,7 +178,8 @@ ready to be use:
   notifications:
     email: false
 
-An example can be found in the `.travis.yml <https://github.com/ros-infrastructure/ros_buildfarm/blob/master/.travis.yml>`_ file of the *ros_buildfarm* repository.
+An example can be found in the `.travis.yml <https://github.com/ros-infrastructure/ros_buildfarm/blob/master/.travis.yml>`_
+file of the *ros_buildfarm* repository.
 
 Run for "custom" repositories
 -----------------------------

ros_buildfarm/argument.py

@@ -304,11 +304,11 @@ class CheckLength(argparse.Action):
         def __call__(self, parser, args, values, option_string=None):
             if len(values) < minargs:
                 raise argparse.ArgumentError(
-                        argument=self,
-                        message="expected at least %s arguments" % (minargs))
+                    argument=self,
+                    message='expected at least %s arguments' % (minargs))
             elif len(values) > maxargs:
                 raise argparse.ArgumentError(
-                        argument=self,
-                        message="expected at most %s arguments" % (minargs))
+                    argument=self,
+                    message='expected at most %s arguments' % (minargs))
             setattr(args, self.dest, values)
     return CheckLength

ros_buildfarm/ci_job.py

@@ -205,7 +205,7 @@ def configure_ci_job(
 
     job_config = _get_ci_job_config(
         index, rosdistro_name, build_file, os_name,
-        os_code_name, arch, job_name,
+        os_code_name, arch,
         build_file.repos_files,
         underlay_source_job,
         underlay_source_paths,
@@ -228,7 +228,7 @@ def configure_ci_view(jenkins, view_name, dry_run=False):
 
 def _get_ci_job_config(
         index, rosdistro_name, build_file, os_name,
-        os_code_name, arch, job_name,
+        os_code_name, arch,
         repos_files, underlay_source_job,
         underlay_source_paths, trigger_timer,
         is_disabled=False):
@@ -260,9 +260,6 @@ def _get_ci_job_config(
 
         'disabled': is_disabled,
 
-        # this should not be necessary
-        'job_name': job_name,
-
         'ros_buildfarm_repository': get_repository(),
 
         'script_generating_key_files': script_generating_key_files,

ros_buildfarm/templates/ci/ci_job.xml.em

@@ -164,7 +164,7 @@ parameters = [
         ' ' + ' '.join(repository_args) +
         ' --build-tool ' + build_tool +
         ' --ros-version ' + str(ros_version) +
-        ' --env-vars ' + ' '.join(build_environment_variables) +
+        ' --env-vars ' + ' '.join([v.replace('$', '\\$',) for v in build_environment_variables]) +
         ' --dockerfile-dir $WORKSPACE/docker_generating_dockers' +
         ' --repos-file-urls $repos_files' +
         ' --test-branch "$test_branch"' +

scripts/ci/build_task_generator.py

@@ -79,9 +79,10 @@ def main(argv=sys.argv[1:]):
 
     apt_cache = Cache()
 
-    debian_pkg_names = set(args.foundation_packages)
+    debian_pkg_names = set(['build-essential'])
+    debian_pkg_names.update(args.foundation_packages)
     if args.build_tool == 'colcon':
-        debian_pkg_names |= set([
+        debian_pkg_names.update([
             'python3-catkin-pkg-modules',
             'python3-colcon-ros',
             'python3-colcon-test-result',

scripts/ci/create_workspace_task_generator.py

@@ -140,10 +140,6 @@ def main(argv=sys.argv[1:]):
     create_dockerfile(
         'ci/create_workspace.Dockerfile.em', data, args.dockerfile_dir)
 
-    # output hints about necessary volumes to mount
-    ros_buildfarm_basepath = os.path.normpath(
-        os.path.join(os.path.dirname(__file__), '..', '..'))
-
 
 if __name__ == '__main__':
     main()

scripts/devel/build_and_test.py

@@ -67,14 +67,13 @@ def main(argv=sys.argv[1:]):
         with Scope('SUBSECTION', 'build workspace in isolation'):
             test_results_dir = os.path.join(
                 args.workspace_root, 'test_results')
-            cmake_args = ['-DBUILD_TESTING=1']
+            cmake_args = [
+                '-DBUILD_TESTING=1'
+                '-DCATKIN_ENABLE_TESTING=1', '-DCATKIN_SKIP_TESTING=0',
+                '-DCATKIN_TEST_RESULTS_DIR=%s' % test_results_dir]
             additional_args = None
             if args.build_tool == 'colcon':
                 additional_args = ['--test-result-base', test_results_dir]
-            elif args.build_tool == 'catkin_make_isolated':
-                cmake_args += [
-                    '-DCATKIN_ENABLE_TESTING=1', '-DCATKIN_SKIP_TESTING=0',
-                    '-DCATKIN_TEST_RESULTS_DIR=%s' % test_results_dir]
             env = dict(os.environ)
             env['MAKEFLAGS'] = '-j1'
             rc = call_build_tool(