From 142690876fd046750ee4c4e0a715ef554ffaec76 Mon Sep 17 00:00:00 2001 From: Jonathan Bohren Date: Sun, 24 Jan 2016 19:43:25 -0500 Subject: [PATCH] build: Cleaning up isolated devel space generation, fixing out of bounds leaves bug. Fixes #45. Fixes #65. --- catkin_tools/verbs/catkin_build/build.py | 133 +++++++++++++---------- 1 file changed, 77 insertions(+), 56 deletions(-) diff --git a/catkin_tools/verbs/catkin_build/build.py b/catkin_tools/verbs/catkin_build/build.py index 7bf8bc07..a7c00dbc 100644 --- a/catkin_tools/verbs/catkin_build/build.py +++ b/catkin_tools/verbs/catkin_build/build.py @@ -522,7 +522,7 @@ def build_isolated_workspace( # Create isolated devel setup if necessary if context.isolate_devel: if not context.install: - _create_unmerged_devel_setup(context) + _create_unmerged_devel_setup(context, now_unbuilt_pkgs) else: _create_unmerged_devel_setup_for_install(context) return 0 @@ -534,41 +534,81 @@ def build_isolated_workspace( event_queue.put(None) -def _create_unmerged_devel_setup(context): +def _create_unmerged_devel_setup(context, unbuilt): # Find all of the leaf packages in the workspace # where leaf means that nothing in the workspace depends on it - # Find all packages in the source space - # Suppress warnings since this is an internal function whose goal is not to - # give feedback on the user's packages - workspace_packages = find_packages(context.source_space_abs, exclude_subspaces=True, warnings=[]) + ordered_packages = context.packages + workspace_packages = dict([(p.name, p) for pth, p in ordered_packages]) + + # Get all packages which are dependencies of packages in the workspace which have been built + dependencies = set(sum([ + [d.name for d in p.buildtool_depends + p.build_depends + p.run_depends] + for _, p in workspace_packages.items() + if p.name not in unbuilt + ], [])) + + # Compute the packages on which no other packages depend + leaf_packages = [ + pkg.name + for name, pkg in workspace_packages.items() + if pkg.name not in dependencies + ] + leaf_paths = [ + os.path.join(context.devel_space_abs, p, 'setup.sh') + for p in leaf_packages + ] + leaf_sources = [ + '. {}'.format(source_path) + for source_path in leaf_paths + if os.path.isfile(source_path) + ] - ordered_packages = topological_order_packages(workspace_packages) - workspace_packages = dict([(p.name, p) for pth, p in workspace_packages.items()]) - dependencies = set([]) - for name, pkg in workspace_packages.items(): - dependencies.update([d.name for d in pkg.buildtool_depends + pkg.build_depends + pkg.run_depends]) - leaf_packages = [] - for name, pkg in workspace_packages.items(): - if pkg.name not in dependencies: - leaf_packages.append(pkg.name) - assert leaf_packages, leaf_packages # Defensive, there should always be at least one leaf - leaf_sources = [] - for pkg_name in leaf_packages: - source_path = os.path.join(context.devel_space_abs, pkg_name, 'setup.sh') - if os.path.isfile(source_path): - leaf_sources.append('. {0}'.format(source_path)) # In addition to the leaf packages, we need to source the recursive run depends of the leaf packages - run_depends = get_recursive_run_depends_in_workspace( + run_depends_packages = get_recursive_run_depends_in_workspace( [workspace_packages[p] for p in leaf_packages], ordered_packages) - run_depends_sources = [] - for run_dep_name in [p.name for pth, p in run_depends]: - source_path = os.path.join(context.devel_space_abs, run_dep_name, 'setup.sh') - if os.path.isfile(source_path): - run_depends_sources.append('. {0}'.format(source_path)) + run_depends_paths = [ + os.path.join(context.devel_space_abs, p, 'setup.sh') + for p in run_depends_packages + ] + run_depends_sources = [ + '. {}'.format(source_path) + for source_path in run_depends_paths + if os.path.isfile(source_path) + ] + # Create the setup.sh file setup_sh_path = os.path.join(context.devel_space_abs, 'setup.sh') - env_file = """\ + env_file = SETUP_SH_TEMPLATE.format( + first_source=leaf_sources[0], + leaf_sources='\n'.join(leaf_sources[1:]), + run_depends_sources='\n'.join(run_depends_sources) + ) + with open(setup_sh_path, 'w') as f: + f.write(env_file) + os.chmod(setup_sh_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) + + # Create setup.bash file + setup_bash_path = os.path.join(context.devel_space_abs, 'setup.bash') + with open(setup_bash_path, 'w') as f: + f.write(SETUP_BASH_TEMPLATE) + os.chmod(setup_bash_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) + + # Create setup.zsh file + setup_zsh_path = os.path.join(context.devel_space_abs, 'setup.zsh') + with open(setup_zsh_path, 'w') as f: + f.write(SETUP_ZSH_TEMPLATE) + os.chmod(setup_zsh_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) + + +def _create_unmerged_devel_setup_for_install(context): + """Create non-functioning placeholder scripts in develspace.""" + for path in [os.path.join(context.devel_space_abs, f) for f in ['setup.sh', 'setup.bash', 'setup.zsh']]: + with open(path, 'w') as f: + f.write(SETUP_PLACEHOLDER_TEMPLATE) + + +SETUP_SH_TEMPLATE = """\ #!/usr/bin/env sh # generated from within catkin_tools/verbs/catkin_build/build.py @@ -589,19 +629,9 @@ def _create_unmerged_devel_setup(context): # And now the setup.sh for each of their recursive run dependencies {run_depends_sources} -""".format( - first_source=leaf_sources[0], - leaf_sources='\n'.join(leaf_sources[1:]), - run_depends_sources='\n'.join(run_depends_sources) - ) - with open(setup_sh_path, 'w') as f: - f.write(env_file) - # Make this file executable - os.chmod(setup_sh_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) - # Create the setup.bash file - setup_bash_path = os.path.join(context.devel_space_abs, 'setup.bash') - with open(setup_bash_path, 'w') as f: - f.write("""\ +""" + +SETUP_BASH_TEMPLATE = """\ #!/usr/bin/env bash # generated from within catkin_tools/verbs/catkin_build/build.py @@ -610,12 +640,9 @@ def _create_unmerged_devel_setup(context): # source setup.sh from same directory as this file _BUILD_SETUP_DIR=$(builtin cd "`dirname "${BASH_SOURCE[0]}"`" && pwd) . "$_BUILD_SETUP_DIR/setup.sh" -""") - # Make this file executable - os.chmod(setup_bash_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) - setup_zsh_path = os.path.join(context.devel_space_abs, 'setup.zsh') - with open(setup_zsh_path, 'w') as f: - f.write("""\ +""" + +SETUP_ZSH_TEMPLATE = """\ #!/usr/bin/env zsh # generated from within catkin_tools/verbs/catkin_build/build.py @@ -626,19 +653,13 @@ def _create_unmerged_devel_setup(context): emulate sh # emulate POSIX . "$_BUILD_SETUP_DIR/setup.sh" emulate zsh # back to zsh mode -""") - # Make this file executable - os.chmod(setup_zsh_path, stat.S_IXUSR | stat.S_IWUSR | stat.S_IRUSR) - +""" -def _create_unmerged_devel_setup_for_install(context): - for path in [os.path.join(context.devel_space_abs, f) for f in ['setup.sh', 'setup.bash', 'setup.zsh']]: - with open(path, 'w') as f: - f.write("""\ +SETUP_PLACEHOLDER_TEMPLATE = """\ #!/usr/bin/env sh # generated from within catkin_tools/verbs/catkin_build/build.py echo "Error: This workspace was built with the '--install' option." echo " You should source the setup files in the install space instead." echo " Your environment has not been changed." -""") +"""