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

add --use-ninja option to catkin_make(_isolated) to use ninja instead of make #693

Merged
merged 1 commit into from
Oct 23, 2014

Conversation

dirk-thomas
Copy link
Member

@tfoote @wjwwood Please review.

@tfoote
Copy link
Member

tfoote commented Oct 22, 2014

+1 there are two or three repeated if clauses for the makefile name and command line args, but at two instances the function method will not save lines, but if there's ever to be more, it might make sense to make them a function.

@@ -133,11 +133,20 @@ def main():
print(fmt('@{yf}Packages @{boldon}"%s"@{boldoff} not found in the workspace - ignoring them' % ', '.join(sorted(unknown_packages))), file=sys.stderr)
args.pkg = [name for name in args.pkg if name in packages_by_name]

if '-G' not in cmake_args:
Copy link
Member

Choose a reason for hiding this comment

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

Is it valid for an input to be ['-GFoo'], which would cause this to fail? It seems to be possible:

% cmake -GXcode ..
-- The C compiler identification is AppleClang 6.0.0.6000054
-- The CXX compiler identification is AppleClang 6.0.0.6000054
-- Check for working C compiler using: Xcode
-- Check for working C compiler using: Xcode -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler using: Xcode
^C

Would it not be an error if you specify both a -G argument in the cmake_args and --use-ninja?

Copy link
Member Author

Choose a reason for hiding this comment

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

Both good points. Addressed in second commit.

@wjwwood
Copy link
Member

wjwwood commented Oct 22, 2014

+1

@dirk-thomas
Copy link
Member Author

Squashed.

dirk-thomas added a commit that referenced this pull request Oct 23, 2014
add --use-ninja option to catkin_make(_isolated) to use ninja instead of make
@dirk-thomas dirk-thomas merged commit a0e9f3e into indigo-devel Oct 23, 2014
@dirk-thomas dirk-thomas deleted the ninja branch October 23, 2014 17:39
cwecht pushed a commit to cwecht/catkin that referenced this pull request Mar 20, 2018
Allow same topics to be used in different bags (a previously supported use case).
Remove unused variable `current_bag_sensor_topics`.
Touch up flag descriptions.

Fixes ros#693.

pair=@gaschler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants