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

Jump to bottom

Relationship of defaults.yaml to command line and to mixins is unclear #44

Open
StephenApptronik opened this issue Feb 27, 2025 · 7 comments · May be fixed by colcon/colcon.readthedocs.org#106
Open

Relationship of defaults.yaml to command line and to mixins is unclear #44

StephenApptronik opened this issue Feb 27, 2025 · 7 comments · May be fixed by colcon/colcon.readthedocs.org#106

Comments

@StephenApptronik
Copy link

The documentation of defaults.yaml is sparse:
https://colcon.readthedocs.io/en/released/user/configuration.html#defaults-yaml
I would like to know how the system is supposed to work and whether the following behavior is a bug/has any workarounds.

What we have observed: Suppose we have a defaults.yaml file of this:

{
    "build": {
        "cmake-args": [
            "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON",
            "-DCMAKE_CXX_FLAGS=' -Werror=suggest-override -Wunused-variable -Wuninitialized -Wshadow -Wformat-security -Wswitch-default -Wparentheses -Wsign-conversion -Werror -I/usr/include/c++/11 -I/usr/include/x86_64-linux-gnu/c++/11 -I/usr/lib/gcc/x86_64-linux-gnu/11/include '"
        ]
    }
}

If we build with colcon build then everything works as expected. But if we do this:
colcon build --cmake-args "-DCMAKE_BUILD_TYPE=Debug"
then we lose all the warning flags. Similarly, if we do this:
colcon build --mixin customthing
We also lose the defaults. Now, the first case kind of makes sense -- the command line overrode all the cmake instructions and replaced them. What's unclear is why they second one also tossed out the defaults.yaml values.

Is there any way to have parameters at the command line that add to the defaults.yaml? I do not want to have to copy out the entire defaults set everytime I want to change one thing in the set.
@StephenApptronik
Copy link
Author

I later found this:
colcon/colcon-defaults#32
which explains that merging the settings is impossible. OK, fine... so is there a recommended way to make a mixin that reads the defaults.yaml file and adds the settings?

@christophebedard
Copy link

We simply put all options in each mixin.

@StephenApptronik
Copy link
Author

We simply put all options in each mixin.

And when defaults.yaml changes, you edit every mixin? I mean, that works, but seems error prone.

@christophebedard
Copy link

christophebedard commented Feb 28, 2025 •
edited
Loading

I think we simply try to avoid clashes between the options that mixins set and the options that a defaults.yaml file sets. In my case, my defaults.yaml file simply sets things like colcon test's --executor and --event-handlers options, while mixins set colcon build's --cmake-args, etc. I agree it's not ideal.

@StephenApptronik
Copy link
Author

Thank you. This at least helps me stop searching for the info.

To the developers: I think updating the documentation with info about how defaults.yaml gets supplanted would be good for future users.
https://colcon.readthedocs.io/en/released/user/configuration.html#defaults-yaml

@christophebedard christophebedard linked a pull request Feb 28, 2025 that will close this issue
Open
@christophebedard
Copy link

In a way, I think it is kind of in the name: it's a default value. It's only used if a value isn't provided for the given option, which could be through a mixin. I opened a PR to hopefully make the docs a bit clearer, both on the default file side and the mixin side: colcon/colcon.readthedocs.org#106

@StephenApptronik
Copy link
Author

In a way, I think it is kind of in the name: it's a default value.

Yeah, but the other name is "mixin" not "replacer". :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Warn about mixin overriding default values christophebedard/colcon.readthedocs.org
2 participants
@christophebedard @StephenApptronik