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

allow dash in catkin package names #199

Merged
merged 1 commit into from Jan 17, 2018
Merged

Conversation

dirk-thomas
Copy link
Member

Replaces #182.

@gavanderhoorn
Copy link

@dirk-thomas wrote (in #182):

I just hope we will not run into the problem of duplicate package names afterwards or uncertainty about how the CMake variables are being exported (since that was a main reason why this rule enforced uniqueness and consistency).

can you say anything about the exported CMake variables? Would the dashes be retained there?

@dirk-thomas
Copy link
Member Author

can you say anything about the exported CMake variables? Would the dashes be retained there?

CMake variables can't contain dashes.

@gavanderhoorn
Copy link

CMake variables can't contain dashes.

exactly, so will the variables for those pkgs have underscores instead then? Is that the potential confusion you are referring to?

@dirk-thomas
Copy link
Member Author

so will the variables for those pkgs have underscores instead then? Is that the potential confusion you are referring to?

Yes, I would assume using underscores would be a reasonable mapping.

A package name with dashes is more common for e.g. Python packages though. I hope this won't be used for CMake packages often.

@dirk-thomas
Copy link
Member Author

@ros-infrastructure/ros_team Please review.

@wjwwood
Copy link
Contributor

wjwwood commented Jan 17, 2018

CMake variables can't contain dashes.

Actually that's not true (/shudder) 🤢:

william@johnnycake /tmp
% cat test.cmake
set(a-dash "Hello World!")
message(STATUS ${a-dash})

william@johnnycake /tmp
% cmake -P test.cmake
-- Hello World!

@dirk-thomas
Copy link
Member Author

CMake variables can't contain dashes.

Actually that's not true (/shudder) 🤢:

Interesting, that is the first time I have seen that but the docs "agree" with it.

@dirk-thomas dirk-thomas merged commit 46a8aa7 into master Jan 17, 2018
@dirk-thomas dirk-thomas deleted the allow_dash_in_pkg_names branch January 17, 2018 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants