Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better message when missing a required command line tool. #455

Merged
merged 1 commit into from
May 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions catkin_tools/jobs/catkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from .utils import copyfiles
from .utils import loadenv
from .utils import makedirs
from .utils import require_command
from .utils import rmfiles


Expand Down Expand Up @@ -432,6 +433,8 @@ def create_catkin_build_job(context, package, package_path, dependencies, force_
prefix=context.package_dest_path(package)
))

require_command('cmake', CMAKE_EXEC)

# CMake command
stages.append(CommandStage(
'cmake',
Expand Down Expand Up @@ -473,6 +476,8 @@ def create_catkin_build_job(context, package, package_path, dependencies, force_
cwd=build_space,
))

require_command('make', MAKE_EXEC)

# Make command
stages.append(CommandStage(
'make',
Expand Down
5 changes: 5 additions & 0 deletions catkin_tools/jobs/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from .utils import copyfiles
from .utils import loadenv
from .utils import makedirs
from .utils import require_command
from .utils import rmfiles

from catkin_tools.execution.jobs import Job
Expand Down Expand Up @@ -260,6 +261,8 @@ def create_cmake_build_job(context, package, package_path, dependencies, force_c
dest_path=os.path.join(metadata_path, 'package.xml')
))

require_command('cmake', CMAKE_EXEC)

# CMake command
makefile_path = os.path.join(build_space, 'Makefile')
if not os.path.isfile(makefile_path) or force_cmake:
Expand Down Expand Up @@ -291,6 +294,8 @@ def create_cmake_build_job(context, package, package_path, dependencies, force_c
cwd=build_space,
))

require_command('make', MAKE_EXEC)

# Make command
stages.append(CommandStage(
'make',
Expand Down
13 changes: 13 additions & 0 deletions catkin_tools/jobs/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@
from catkin_tools.execution.events import ExecutionEvent


class CommandMissing(Exception):
'''A required command is missing.'''

def __init__(self, name):
super(CommandMissing, self).__init__(
'Cannot find required tool `%s` on the PATH, is it installed?' % name)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to move this to its own exceptions.py file, didn't want to bother with that unless there was a clear need.

Copy link
Member Author

@mikepurvis mikepurvis May 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(A good motivation to do so might be eliminating the use of sys.exit within build_isolated_workspace, in favour of throwing other exceptions that resolve to the string error messages. Doing this would make the function more cooperative as far as being wrapped or called from other tools.)



def require_command(name, which):
if not which:
raise CommandMissing(name)


def get_env_loaders(package, context):
"""Get a list of env loaders required to build this package."""

Expand Down
42 changes: 23 additions & 19 deletions catkin_tools/verbs/catkin_build/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@

import catkin_tools.execution.job_server as job_server

from catkin_tools.jobs.utils import CommandMissing
from catkin_tools.jobs.utils import loadenv

from catkin_tools.metadata import find_enclosing_workspace
Expand Down Expand Up @@ -400,22 +401,25 @@ def main(opts):
if opts.verbose:
os.environ['VERBOSE'] = '1'

return build_isolated_workspace(
ctx,
packages=opts.packages,
start_with=opts.start_with,
no_deps=opts.no_deps,
unbuilt=opts.unbuilt,
n_jobs=parallel_jobs,
force_cmake=opts.force_cmake,
pre_clean=opts.pre_clean,
force_color=opts.force_color,
quiet=not opts.verbose,
interleave_output=opts.interleave_output,
no_status=opts.no_status,
limit_status_rate=opts.limit_status_rate,
lock_install=not opts.no_install_lock,
no_notify=opts.no_notify,
continue_on_failure=opts.continue_on_failure,
summarize_build=opts.summarize # Can be True, False, or None
)
try:
return build_isolated_workspace(
ctx,
packages=opts.packages,
start_with=opts.start_with,
no_deps=opts.no_deps,
unbuilt=opts.unbuilt,
n_jobs=parallel_jobs,
force_cmake=opts.force_cmake,
pre_clean=opts.pre_clean,
force_color=opts.force_color,
quiet=not opts.verbose,
interleave_output=opts.interleave_output,
no_status=opts.no_status,
limit_status_rate=opts.limit_status_rate,
lock_install=not opts.no_install_lock,
no_notify=opts.no_notify,
continue_on_failure=opts.continue_on_failure,
summarize_build=opts.summarize # Can be True, False, or None
)
except CommandMissing as e:
sys.exit(clr("[build] @!@{rf}Error:@| {0}").format(e))