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

Fix space-suffix option #560

Merged
merged 1 commit into from
Nov 25, 2019
Merged

Conversation

matthew-reynolds
Copy link
Contributor

Addresses #545

From the docs, the catkin config --space-suffix argument should set the "Suffix for build, devel, and install space if they are not otherwise explicitly set." However, currently, the default values for each of these spaces are considered "explicitly set". This means that, except for an extremely narrow window when first initializing a workspace, the --space-suffix argument is always useless.

This PR allows --space-suffix to affect spaces whose values are either not set or are set to the default values. This brings the tool back in line with how it used to behave.


Aside: The documentation says that this affects the build, devel, and install spaces, but the code is implemented such that it affects the build, devel, install, and log spaces. I assume the code is correct and the documentation is out of date, but I'd like to get confirmation on which is prefered so that I can update the code/documentation accordingly.

@matthew-reynolds
Copy link
Contributor Author

Some more details:

Whenever a context is saved, it writes all of its space values to the profile's config.yaml file. This includes all space values, even if the space values are default. Therefore, the only time when the --space-suffix tool can have any effect is immediately after initializing a workspace before the context is saved.

Importantly, contexts are saved in a lot of different cases (whenever catkin profile add, catkin profile rename, catkin build --save-config, or any catkin config commands are run), meaning the --space-suffix tool will almost immediately be useless once you start using the workspace. Even just running catkin config to read back the config summary or adding a new profile will save the context and write the default values to the config file, rendering the --space-suffix argument useless.

@mikepurvis
Copy link
Member

Thanks for figuring this out and addressing it.

@mikepurvis mikepurvis merged commit dd65fbd into catkin:master Nov 25, 2019
@matthew-reynolds matthew-reynolds deleted the fix-space-suffix branch November 25, 2019 21:16
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.

2 participants