search

colcon / colcon-mixin

Extension for colcon to read CLI mixins from files
http://colcon.readthedocs.io
Apache License 2.0
2 stars 7 forks source link

Additional CFLAGS/CXXFLAGS are suppressed when using colcon mixins #20

Open robbel opened 4 years ago

robbel commented 4 years ago

I am not sure if this is intended behavior but I can observe that (e.g.) colcon build --cmake-args -DCMAKE_C_FLAGS=-fno-omit-frame-pointer -DCMAKE_CXX_FLAGS=-fno-omit-frame-pointer --mixin asan-gcc results in the mixin build switches (here: -fsanitize=address) to be suppressed. Is this the intended behavior that any switches defined in a mixin are not to be replicated (or, added to) via the command line?

dirk-thomas commented 4 years ago

Atm there is no special knowledge about the CMake arguments.

Individual CMake arguments are being merged (e.g. a mixin containing -DFOO and the CLI passing -DBAR).

For a mixin value -DCMAKE_C_FLAGS=foo and a CLI value -DCMAKE_C_FLAGS=bar to be merged into -DCMAKE_C_FLAGS=foo bar that would require custom knowledge about the semantic of -DCMAKE_C_FLAGS (that multiple values should be concatenated).

This would probably require the definition of a new extension point so other Python packages can contribute to that special knowledge.

robbel commented 4 years ago

Thanks for that explanation @dirk-thomas. From your description it is clear why this is the behavior. On the front-end / user side it can still catch one by surprise, I think (silent suppression without a warning or something to that effect). I wonder if if there is a good place to document that behavior / warning that no merging is done in case options appear multiple times. I imagine this to the be the same when multiple mixins are specified (e.g., --mixin asan-gcc --mixin custom-no-omit-frame-pointer) where it doesn't seem obvious what compiler flags will ultimately be chosen. I'm happy to submit a documentation patch if there's a good place to put it.

dirk-thomas commented 4 years ago

All docs are in https://github.com/colcon/colcon.readthedocs.org/ Please consider to contribute a pull request for it. Not sure what the best sections would be to add this note.

robbel commented 4 years ago

I am not sure if I can assign reviewers but I proposed a docufix here: https://github.com/colcon/colcon.readthedocs.org/pull/66 cc @dirk-thomas