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.

Sign up for GitHub

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

Use nmake #816

Closed
wants to merge 4 commits into from
Closed

Use nmake #816

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
564bd4c
Add: option to use NMake instead of Make
Jul 24, 2016
11d8e26
Add: option to use nmake instead of make
Jul 24, 2016
0bd7cc5
Add: option to use nmake instead of make
Jul 24, 2016
b201c0e
Fix: typo
Jul 26, 2016
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
29 changes: 18 additions & 11 deletions bin/catkin_make
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,14 @@ def main():
args.pkg = [name for name in args.pkg if name in packages_by_name]

if not [arg for arg in cmake_args if arg.startswith('-G')]:
if not args.use_ninja:
cmake_args += ['-G', 'Unix Makefiles']
else:
if args.use_ninja:
cmake_args += ['-G', 'Ninja']
elif args.use_ninja:
return fmt("@{rf}Error: either specify a generator using '-G...' or '--use-ninja' but not both")
elif args.use_nmake:
cmake_args += ['-G', 'NMake Makefiles']
else:
cmake_args += ['-G', 'Unix Makefiles']
elif args.use_ninja or args.use_nmake:
return fmt("@{rf}Error: either specify a generator using '-G...' or '--use-[ninja|nmake]' but not both")

# check if cmake must be run (either for a changed list of package paths or changed cmake arguments)
force_cmake = cmake_input_changed(packages, build_path, cmake_args=cmake_args)
Expand All @@ -203,10 +205,12 @@ def main():
except subprocess.CalledProcessError:
return fmt('@{rf}Invoking @{boldon}"cmake"@{boldoff} failed')
else:
if not args.use_ninja:
cmd = ['make', 'cmake_check_build_system']
else:
if args.use_ninja:
cmd = ['ninja', 'build.ninja']
elif args.use_nmake:
cmd = ['nmake', 'cmake_check_build_system']
else:
cmd = ['make', 'cmake_check_build_system']
try:
print_command_banner(cmd, build_path, color=not args.no_color)
if args.no_color:
Expand All @@ -219,10 +223,12 @@ def main():
ensure_workspace_marker(base_path)

# invoke make
if not args.use_ninja:
cmd = ['make']
else:
if args.use_ninja:
cmd = ['ninja']
elif args.use_nmake:
cmd = ['nmake']
else:
cmd = ['make']
cmd.extend(handle_make_arguments(args.make_args))
try:
if not args.pkg:
Expand Down Expand Up @@ -259,6 +265,7 @@ def _parse_args(args=sys.argv[1:]):
add('--source', help="The path to the source space (default 'workspace_base/src')")
add('--build', help="The path to the build space (default 'workspace_base/build')")
add('--use-ninja', action='store_true', help="Use 'ninja' instead of 'make'")
add('--use-nmake', action='store_true', help="Use 'nmake' instead of 'make'")
add('--force-cmake', action='store_true', help="Invoke 'cmake' even if it has been executed before")
add('--no-color', action='store_true', help='Disables colored output (only for catkin_make and CMake)')
add('--pkg', nargs='+', help="Invoke 'make' on specific packages only")
Expand Down
2 changes: 2 additions & 0 deletions bin/catkin_make_isolated
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def parse_args(args=None):
add('--install-space', default=None,
help="Sets the target install space (default 'workspace_base/install_isolated')")
add('--use-ninja', action='store_true', help="Use 'ninja' instead of 'make'")
add('--use-nmake', action='store_true', help="Use 'nmake' instead of 'make'")
add('--install', action='store_true', default=False,
help='Causes each catkin package to be installed.')
add('--force-cmake', action='store_true', default=False,
Expand Down Expand Up @@ -151,6 +152,7 @@ def main():
only_pkg_with_deps=opts.only_pkg_with_deps,
destdir=destdir,
use_ninja=opts.use_ninja,
use_nmake=opts.use_nmake,
override_build_tool_check=opts.override_build_tool_check,
)

Expand Down
54 changes: 35 additions & 19 deletions python/catkin/builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ def build_catkin_package(
path, package,
workspace, buildspace, develspace, installspace,
install, force_cmake, quiet, last_env, cmake_args, make_args,
destdir=None, use_ninja=False
destdir=None, use_ninja=False, use_nmake=False
):
cprint(
"Processing @{cf}catkin@| package: '@!@{bf}" +
Expand Down Expand Up @@ -403,10 +403,13 @@ def build_catkin_package(
else:
print('%s exists, skipping explicit cmake invocation...' % makefile_name)
# Check to see if cmake needs to be run via make
if not use_ninja:
make_check_cmake_cmd = ['make', 'cmake_check_build_system']
else:
if use_ninja:
make_check_cmake_cmd = ['ninja', 'build.ninja']
elif use_namke:
Copy link
Member

Choose a reason for hiding this comment

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

Spelling error fails CI.

Copy link
Author

Choose a reason for hiding this comment

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

Fixing

make_check_cmake_cmd = ['nmake', 'cmake_check_build_system']
else:
make_check_cmake_cmd = ['make', 'cmake_check_build_system']

add_env = get_additional_environment(install, destdir, installspace)
isolation_print_command(' '.join(make_check_cmake_cmd), build_dir, add_env=add_env)
if last_env is not None:
Expand All @@ -416,10 +419,13 @@ def build_catkin_package(
)

# Run make
if not use_ninja:
make_executable = 'make'
else:
if use_ninja:
make_executable = 'ninja'
elif use_nmake:
make_executable = 'nmake'
else:
make_executable = 'make'

make_cmd = [make_executable]
make_cmd.extend(handle_make_arguments(make_args))
isolation_print_command(' '.join(make_cmd), build_dir)
Expand All @@ -428,7 +434,8 @@ def build_catkin_package(
run_command(make_cmd, build_dir, quiet)

# Make install
if install:
# NMake doesn't have an option to list target so try it anyway
if install or use_nmake:
if has_make_target(build_dir, 'install', use_ninja=use_ninja):
make_install_cmd = [make_executable, 'install']
isolation_print_command(' '.join(make_install_cmd), build_dir)
Expand Down Expand Up @@ -465,7 +472,7 @@ def build_cmake_package(
path, package,
workspace, buildspace, develspace, installspace,
install, force_cmake, quiet, last_env, cmake_args, make_args,
destdir=None, use_ninja=False
destdir=None, use_ninja=False, use_nmake=False
):
# Notify the user that we are processing a plain cmake package
cprint(
Expand Down Expand Up @@ -507,10 +514,13 @@ def build_cmake_package(
else:
print('%s exists, skipping explicit cmake invocation...' % makefile_name)
# Check to see if cmake needs to be run via make
if not use_ninja:
make_check_cmake_cmd = ['make', 'cmake_check_build_system']
else:
if use_ninja:
make_check_cmake_cmd = ['ninja', 'build.ninja']
elif use_nmake:
make_check_cmake_cmd = ['nmake', 'cmake_check_build_system']
else:
make_check_cmake_cmd = ['make', 'cmake_check_build_system']

isolation_print_command(' '.join(make_check_cmake_cmd), build_dir)
if last_env is not None:
make_check_cmake_cmd = [last_env] + make_check_cmake_cmd
Expand All @@ -520,9 +530,11 @@ def build_cmake_package(

# Run make
if not use_ninja:
make_executable = 'make'
else:
make_executable = 'ninja'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this condition inverted? It looks like it now says:

if not use_ninja:
    make_executable = 'ninja'

That would seem to be the inverse of what is actually the intent.

Copy link
Member

Choose a reason for hiding this comment

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

For reference: #826

elif use_nmake:
make_executable = 'nmake'
else:
make_executable = 'make'
make_cmd = [make_executable]
make_cmd.extend(handle_make_arguments(make_args))
isolation_print_command(' '.join(make_cmd), build_dir)
Expand Down Expand Up @@ -739,6 +751,7 @@ def build_workspace_isolated(
only_pkg_with_deps=None,
destdir=None,
use_ninja=False,
use_nmake=False,
override_build_tool_check=False
):
'''
Expand Down Expand Up @@ -775,6 +788,7 @@ def build_workspace_isolated(
``[str]``
:param destdir: define DESTDIR for cmake/invocation, ``string``
:param use_ninja: if True, use ninja instead of make, ``bool``
:param use_nmake: if True, use nmake instead of make, ``bool``
:param override_build_tool_check: if True, build even if a space was built
by another tool previously.
'''
Expand Down Expand Up @@ -847,12 +861,14 @@ def build_workspace_isolated(
cmake_args = []

if not [arg for arg in cmake_args if arg.startswith('-G')]:
if not use_ninja:
cmake_args += ['-G', 'Unix Makefiles']
else:
if use_ninja:
cmake_args += ['-G', 'Ninja']
elif use_ninja:
print(colorize_line("Error: either specify a generator using '-G...' or '--use-ninja' but not both"))
elif use_nmake:
cmake_args += ['-G', 'NMake Makefiles']
else:
cmake_args += ['-G', 'Unix Makefiles']
elif use_ninja or use_nmake:
print(colorize_line("Error: either specify a generator using '-G...' or '--use-[ninja|nmake]' but not both"))
sys.exit(1)

if make_args:
Expand Down