Skip to content
New issue

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

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

Already on GitHub? # to your account

Treat spaces as handled config context options #477

Closed
wants to merge 1 commit into from
Closed

Treat spaces as handled config context options #477

wants to merge 1 commit into from

Conversation

mbinna
Copy link
Contributor

@mbinna mbinna commented Jul 25, 2017

Adding keyed parameters for those options inhibits the following warning when using profiles.

Unhandled config context options: {}

@mbinna
Copy link
Contributor Author

mbinna commented Jul 27, 2017

With this change, the spaces are not recognized anymore. For example, when switching profiles with different space suffixes, the directories to the spaces are not updated correctly. So this change does not work as expected. However, I am still wondering how to fix the warning. Therefore, I keep this pull request open.

@wjwwood
Copy link
Member

wjwwood commented Jul 27, 2017

Can you please give a more detailed set of steps to reproduce the warning?

@mbinna
Copy link
Contributor Author

mbinna commented Jul 28, 2017

@wjwwood Sure. Here we go:

  1. Start with clean workspace with only src/ directory (no .catkin_tools/ directory)
  2. catkin config -> does not show a warning
  3. catkin profile add release
  4. catkin profile set release
  5. catkin config -> shows the warning

Tested with HEAD of master (da79d37).

@wjwwood
Copy link
Member

wjwwood commented Aug 2, 2017

@mbinna I was able to reproduce the issue, but I had to do catkin config --init in the first step.

I'll look into the issue asap.

@racko
Copy link
Contributor

racko commented Oct 13, 2017

This change doesn't quite work. Try

$ catkin config -d path/to/catkin_ws/devel_linked

and check the Devel Space: output. It will still say path/to/catkin_ws/devel.

Explanation: If you list the spaces as parameters then they won't appear in kwargs. As a result they won't be used in the following code:

# Handle *space assignment and defaults
for space, space_dict in Context.SPACES.items():
key_name = space + '_space'
default = space_dict['default']
if space_suffix and space != 'source':
default += space_suffix
setattr(self, key_name, kwargs.get(key_name, default))

Instead the defaults will be read from space_dict. That's why the output above stills says path/to/catkin_ws/devel: It's the default.

That's why I created #489. Here I pop the kwargs during the loop and check the remaining number of kwargs afterwards: f67767c

@mikepurvis
Copy link
Member

Replaced by #489.

@mikepurvis mikepurvis closed this Mar 26, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants