[v16,11/12] config: Make configuration file configurable

24021 diff mbox series

To support easy switching of configurations, let's introduce
LIBCAMERA_CONFIG_NAME environment variable, which:

- specifies the path of the configuration file to load if it is an
  absolute path; or
- prevents any configuration file from loading if it is defined and
  empty; or
- specifies the path of the configuration file to load, relative to
  the configuration directories.

If such a configuration file doesn't exist, no custom configuration is
loaded.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 Documentation/runtime_configuration.rst |  8 ++++++
 src/libcamera/global_configuration.cpp  | 36 +++++++++++++++++--------
 2 files changed, 33 insertions(+), 11 deletions(-)

Hi Milan,

Thanks for the patch.

Quoting Milan Zamazal (2025-07-29 16:31:59)
> To support easy switching of configurations, let's introduce
> LIBCAMERA_CONFIG_NAME environment variable, which:
> 
> - specifies the path of the configuration file to load if it is an
>   absolute path; or
> - prevents any configuration file from loading if it is defined and
>   empty; or
> - specifies the path of the configuration file to load, relative to
>   the configuration directories.
> 
> If such a configuration file doesn't exist, no custom configuration is
> loaded.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  Documentation/runtime_configuration.rst |  8 ++++++
>  src/libcamera/global_configuration.cpp  | 36 +++++++++++++++++--------
>  2 files changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
> index 0708302d4..71a1a175d 100644
> --- a/Documentation/runtime_configuration.rst
> +++ b/Documentation/runtime_configuration.rst
> @@ -19,6 +19,14 @@ order:
>  - LIBCAMERA_SYSCONF_DIR/configuration.yaml
>  - LIBCAMERA_DATA_DIR/libcamera/configuration.yaml
>  
> +The default name of the configuration file, configuration.yaml, can be
> +overridden in LIBCAMERA_CONFIG_NAME environment variable. The variable
> +can specify just an alternative configuration file name to be looked up
> +in the locations above, or it can contain a whole relative or absolute

afaiu (it was really hard to comprehend) you cannot use relative paths? At
least not in a real sense, as the relative path is confined to being relative
to the configuration directories.

I wonder if there's a way we can make this less confusing. Either you can specify the
full absolute path to the configuration file, or you can only specify the
filename to search for in the configuration directories.


Paul

> +path. If an absolute path is specified then it is the only location that
> +is used; if the given file doesn't exist then no configuration file is
> +read. If the variable is defined but empty, no configuration is loaded.
> +
>  The first configuration file found wins, configuration files in other
>  locations are ignored.
>  
> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> index b9531fdfb..68a4731a1 100644
> --- a/src/libcamera/global_configuration.cpp
> +++ b/src/libcamera/global_configuration.cpp
> @@ -23,13 +23,6 @@
>  
>  namespace libcamera {
>  
> -namespace {
> -const std::vector<std::filesystem::path> globalConfigurationFiles = {
> -       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
> -       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
> -};
> -}
> -
>  LOG_DEFINE_CATEGORY(Configuration)
>  
>  /**
> @@ -75,6 +68,28 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>  
>  void GlobalConfiguration::load()
>  {
> +       const std::vector<std::filesystem::path> globalConfigurationDirectories = {
> +               std::filesystem::path(LIBCAMERA_SYSCONF_DIR),
> +               std::filesystem::path(LIBCAMERA_DATA_DIR),
> +       };
> +
> +       const char *libcameraConfigName =
> +               utils::secure_getenv("LIBCAMERA_CONFIG_NAME");
> +       if (libcameraConfigName && libcameraConfigName[0] == '\0')
> +               return;
> +       if (!libcameraConfigName)
> +               libcameraConfigName = "";
> +
> +       std::filesystem::path configName(libcameraConfigName);
> +
> +       if (configName.is_absolute()) {
> +               loadFile(configName);
> +               return;
> +       }
> +
> +       if (configName.empty())
> +               configName = std::filesystem::path("configuration.yaml");
> +
>         std::filesystem::path userConfigurationDirectory;
>         const char *xdgConfigHome = utils::secure_getenv("XDG_CONFIG_HOME");
>         if (xdgConfigHome) {
> @@ -88,15 +103,14 @@ void GlobalConfiguration::load()
>  
>         if (!userConfigurationDirectory.empty()) {
>                 std::filesystem::path user_configuration_file =
> -                       userConfigurationDirectory / "libcamera" / "configuration.yaml";
> +                       userConfigurationDirectory / "libcamera" / configName;
>                 if (loadFile(user_configuration_file))
>                         return;
>         }
>  
> -       for (const auto &path : globalConfigurationFiles) {
> -               if (loadFile(path))
> +       for (const auto &path : globalConfigurationDirectories)
> +               if (loadFile(path / configName))
>                         return;
> -       }
>  }
>  
>  /**
> -- 
> 2.50.1
>

Hi Paul,

thank you for review.

Paul Elder <paul.elder@ideasonboard.com> writes:

> Hi Milan,
>
> Thanks for the patch.
>
> Quoting Milan Zamazal (2025-07-29 16:31:59)
>> To support easy switching of configurations, let's introduce
>> LIBCAMERA_CONFIG_NAME environment variable, which:
>> 
>> - specifies the path of the configuration file to load if it is an
>>   absolute path; or
>> - prevents any configuration file from loading if it is defined and
>>   empty; or
>> - specifies the path of the configuration file to load, relative to
>>   the configuration directories.
>> 
>> If such a configuration file doesn't exist, no custom configuration is
>> loaded.
>> 
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>  Documentation/runtime_configuration.rst |  8 ++++++
>>  src/libcamera/global_configuration.cpp  | 36 +++++++++++++++++--------
>>  2 files changed, 33 insertions(+), 11 deletions(-)
>> 
>> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
>> index 0708302d4..71a1a175d 100644
>> --- a/Documentation/runtime_configuration.rst
>> +++ b/Documentation/runtime_configuration.rst
>> @@ -19,6 +19,14 @@ order:
>>  - LIBCAMERA_SYSCONF_DIR/configuration.yaml
>>  - LIBCAMERA_DATA_DIR/libcamera/configuration.yaml
>>  
>> +The default name of the configuration file, configuration.yaml, can be
>> +overridden in LIBCAMERA_CONFIG_NAME environment variable. The variable
>> +can specify just an alternative configuration file name to be looked up
>> +in the locations above, or it can contain a whole relative or absolute
>
> afaiu (it was really hard to comprehend) you cannot use relative paths? At
> least not in a real sense, as the relative path is confined to being relative
> to the configuration directories.

The relative path must be relative to something; in this case to the
configuration directories.  I think allowing it being relative e.g. to
the current working directory is not a very good idea for a library, as
the current directory can become something else than the user expects.
And what else should it be relative to?  The user can always use an
absolute path if needed, or LIBCAMERA_CONFIG_DIR variable introduced in
the following patch.

> I wonder if there's a way we can make this less confusing. Either you can specify the
> full absolute path to the configuration file, or you can only specify the
> filename to search for in the configuration directories.

OK, I don't mind removing the relative path case.  And it doesn't seem
to be needed to cover the use cases given by Barnabás in the response to
the following patch.

> Paul
>
>> +path. If an absolute path is specified then it is the only location that
>> +is used; if the given file doesn't exist then no configuration file is
>> +read. If the variable is defined but empty, no configuration is loaded.
>> +
>>  The first configuration file found wins, configuration files in other
>>  locations are ignored.
>>  
>> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>> index b9531fdfb..68a4731a1 100644
>> --- a/src/libcamera/global_configuration.cpp
>> +++ b/src/libcamera/global_configuration.cpp
>> @@ -23,13 +23,6 @@
>>  
>>  namespace libcamera {
>>  
>> -namespace {
>> -const std::vector<std::filesystem::path> globalConfigurationFiles = {
>> -       std::filesystem::path(LIBCAMERA_SYSCONF_DIR) / "configuration.yaml",
>> -       std::filesystem::path(LIBCAMERA_DATA_DIR) / "configuration.yaml",
>> -};
>> -}
>> -
>>  LOG_DEFINE_CATEGORY(Configuration)
>>  
>>  /**
>> @@ -75,6 +68,28 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>>  
>>  void GlobalConfiguration::load()
>>  {
>> +       const std::vector<std::filesystem::path> globalConfigurationDirectories = {
>> +               std::filesystem::path(LIBCAMERA_SYSCONF_DIR),
>> +               std::filesystem::path(LIBCAMERA_DATA_DIR),
>> +       };
>> +
>> +       const char *libcameraConfigName =
>> +               utils::secure_getenv("LIBCAMERA_CONFIG_NAME");
>> +       if (libcameraConfigName && libcameraConfigName[0] == '\0')
>> +               return;
>> +       if (!libcameraConfigName)
>> +               libcameraConfigName = "";
>> +
>> +       std::filesystem::path configName(libcameraConfigName);
>> +
>> +       if (configName.is_absolute()) {
>> +               loadFile(configName);
>> +               return;
>> +       }
>> +
>> +       if (configName.empty())
>> +               configName = std::filesystem::path("configuration.yaml");
>> +
>>         std::filesystem::path userConfigurationDirectory;
>>         const char *xdgConfigHome = utils::secure_getenv("XDG_CONFIG_HOME");
>>         if (xdgConfigHome) {
>> @@ -88,15 +103,14 @@ void GlobalConfiguration::load()
>>  
>>         if (!userConfigurationDirectory.empty()) {
>>                 std::filesystem::path user_configuration_file =
>> -                       userConfigurationDirectory / "libcamera" / "configuration.yaml";
>> +                       userConfigurationDirectory / "libcamera" / configName;
>>                 if (loadFile(user_configuration_file))
>>                         return;
>>         }
>>  
>> -       for (const auto &path : globalConfigurationFiles) {
>> -               if (loadFile(path))
>> +       for (const auto &path : globalConfigurationDirectories)
>> +               if (loadFile(path / configName))
>>                         return;
>> -       }
>>  }
>>  
>>  /**
>> -- 
>> 2.50.1
>>