search

nasa / fprime-tools

F´ Python tooling and helpers.
https://github.com/nasa/fprime
Apache License 2.0
21 stars 39 forks source link

Fix arguments order for generate stage #165

Closed SMorettini closed 1 year ago

SMorettini commented 1 year ago
Originating Project/Creator
Affected Component fprime-util generate

Change Description

Cmake arguments come from three different places:

  1. default
  2. settings.ini
  3. user-defined arguments when running fprime-util generate

Before this change, the order was the default, user-defined and settings.ini. Now I changed the order as reported above.

Rationale

I was trying to set the -DCMAKE_BUILD_TYPE=Release during the generation of the unit tests but I was not able to send anything else than Debug. This was happening because when the arguments were read from settings.ini, the build type was put by default to Debug. This overrides the build type set from the command line.

With my change, the user-defined variable when running fprime-util generate ... overrides any other variable.

thomas-bc commented 1 year ago

@SMorettini turns out the order of dictionary unpacking was a conscious decision, as users should not override the FPRIME_* variables from get_cmake_args() through the command line, but instead use the settings.ini file. Otherwise issues can arise. This means CMAKE_BUILD_TYPE should not be set as part of the get_cmake_args() call, but rather at the generate() level.

I went ahead and implemented this in your PR branch, I hope you don't mind. Would you want to give this a quick review?

I tested locally and it should behave as you requested. Would very much appreciate some additional eyes on this though! Thanks!

thomas-bc commented 1 year ago

Merging, let me know if you have any comments!