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

23641 diff mbox series

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

- The path of the configuration file to load if it is an absolute path; or
- 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  | 33 ++++++++++++++++---------
 2 files changed, 30 insertions(+), 11 deletions(-)

Hi

2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:
> To support easy switching of configurations, let's introduce
> LIBCAMERA_CONFIG_NAME environment variable, which denotes:
> 
> - The path of the configuration file to load if it is an absolute path; or
> - 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  | 33 ++++++++++++++++---------
>   2 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
> index de74fa732..5648db7d7 100644
> --- a/Documentation/runtime_configuration.rst
> +++ b/Documentation/runtime_configuration.rst
> @@ -19,6 +19,14 @@ order:
>   - LIBCAMERA_SYSCONF_DIR/configuration.yaml
>   - /etc/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
> +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.
> +
>   The first configuration file found wins, contingent configuration files
>   in other locations are ignored.
>   
> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> index f7c69890c..2c3b6cf97 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("/etc/libcamera/configuration.yaml"),
> -};
> -}
> -
>   LOG_DEFINE_CATEGORY(Configuration)
>   
>   /**
> @@ -74,6 +67,25 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>   
>   bool GlobalConfiguration::load()
>   {
> +	const std::vector<std::filesystem::path> globalConfigurationDirectories = {
> +		std::filesystem::path(LIBCAMERA_SYSCONF_DIR),

I would probably add `LIBCAMERA_DATA_DIR` after the above, so e.g. `/usr/share/libcamera/configuration.yaml`
could be the configuration that libcamera ships by default. Although I suppose
it might not be too useful until configuration file merging is implemented.


> +		std::filesystem::path("/etc/libcamera"),

I am not sure if we want any hard-coded paths. I believe removing this won't affect
the vast majority, but e.g. if one wants a local installation with a custom prefix,
it might be surprising that libcamera loads configuration from outside.


Regards,
Barnabás Pőcze


> +	};
> +
> +	const char *libcameraConfigName =
> +		utils::secure_getenv("LIBCAMERA_CONFIG_NAME");
> +	if (!libcameraConfigName)
> +		libcameraConfigName = "";
> +	std::filesystem::path configName(libcameraConfigName);
> +
> +	if (configName.is_absolute()) {
> +		loadFile(configName);
> +		return !!yamlConfiguration_;
> +	}
> +
> +	if (configName.empty())
> +		configName = std::filesystem::path("configuration.yaml");
> +
>   	std::filesystem::path userConfigurationDirectory;
>   	const char *xdgConfigHome = utils::secure_getenv("XDG_CONFIG_HOME");
>   	if (xdgConfigHome) {
> @@ -87,15 +99,14 @@ bool GlobalConfiguration::load()
>   
>   	if (!userConfigurationDirectory.empty()) {
>   		std::filesystem::path user_configuration_file =
> -			userConfigurationDirectory / "libcamera" / "configuration.yaml";
> +			userConfigurationDirectory / "libcamera" / configName;
>   		if (loadFile(user_configuration_file))
>   			return !!yamlConfiguration_;
>   	}
>   
> -	for (const auto &path : globalConfigurationFiles) {
> -		if (loadFile(path))
> +	for (const auto &path : globalConfigurationDirectories)
> +		if (loadFile(path / configName))
>   			return !!yamlConfiguration_;
> -	}
>   
>   	yamlConfiguration_ = std::make_unique<YamlObject>();
>   	return true;

Hi Barnabás,

Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:

> Hi
>
> 2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:
>> To support easy switching of configurations, let's introduce
>> LIBCAMERA_CONFIG_NAME environment variable, which denotes:
>> - The path of the configuration file to load if it is an absolute path; or
>> - 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  | 33 ++++++++++++++++---------
>>   2 files changed, 30 insertions(+), 11 deletions(-)
>> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
>> index de74fa732..5648db7d7 100644
>> --- a/Documentation/runtime_configuration.rst
>> +++ b/Documentation/runtime_configuration.rst
>> @@ -19,6 +19,14 @@ order:
>>   - LIBCAMERA_SYSCONF_DIR/configuration.yaml
>>   - /etc/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
>> +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.
>> +
>>   The first configuration file found wins, contingent configuration files
>>   in other locations are ignored.
>>   diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>> index f7c69890c..2c3b6cf97 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("/etc/libcamera/configuration.yaml"),
>> -};
>> -}
>> -
>>   LOG_DEFINE_CATEGORY(Configuration)
>>     /**
>> @@ -74,6 +67,25 @@ bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>>     bool GlobalConfiguration::load()
>>   {
>> +	const std::vector<std::filesystem::path> globalConfigurationDirectories = {
>> +		std::filesystem::path(LIBCAMERA_SYSCONF_DIR),

Let's avoid cycling discussions:

> I would probably add `LIBCAMERA_DATA_DIR` after the above, so
> e.g. `/usr/share/libcamera/configuration.yaml`
> could be the configuration that libcamera ships by default. Although I suppose
> it might not be too useful until configuration file merging is implemented.

https://lists.libcamera.org/pipermail/libcamera-devel/2025-June/050991.html:

  At the moment, we don't need a default configuration in
  /usr/share/libcamera, maybe distributions could utilise it but they can
  simply patch libcamera if they need to change e.g. some path.
  /etc/libcamera is needed, for system-specific defaults.

Do you agree or disagree?

>> +		std::filesystem::path("/etc/libcamera"),
>
> I am not sure if we want any hard-coded paths. I believe removing this won't affect
> the vast majority, but e.g. if one wants a local installation with a custom prefix,
> it might be surprising that libcamera loads configuration from outside.

https://lists.libcamera.org/pipermail/libcamera-devel/2025-June/050992.html:

  With the default /usr/local prefix, LIBCAMERA_SYSCONF_DIR is
  /usr/local/etc, which makes sense.  Imagine somebody installs libcamera
  from the sources to /usr/local, the idea is that /etc/libcamera should
  still be looked up and have a preference.  Good or bad?

OK, makes sense, if libcamera is installed both from a package manager
and locally from the sources, they should have separate configurations.

> Regards,
> Barnabás Pőcze
>
>
>> +	};
>> +
>> +	const char *libcameraConfigName =
>> +		utils::secure_getenv("LIBCAMERA_CONFIG_NAME");
>> +	if (!libcameraConfigName)
>> +		libcameraConfigName = "";
>> +	std::filesystem::path configName(libcameraConfigName);
>> +
>> +	if (configName.is_absolute()) {
>> +		loadFile(configName);
>> +		return !!yamlConfiguration_;
>> +	}
>> +
>> +	if (configName.empty())
>> +		configName = std::filesystem::path("configuration.yaml");
>> +
>>   	std::filesystem::path userConfigurationDirectory;
>>   	const char *xdgConfigHome = utils::secure_getenv("XDG_CONFIG_HOME");
>>   	if (xdgConfigHome) {
>> @@ -87,15 +99,14 @@ bool GlobalConfiguration::load()
>>     	if (!userConfigurationDirectory.empty()) {
>>   		std::filesystem::path user_configuration_file =
>> -			userConfigurationDirectory / "libcamera" / "configuration.yaml";
>> +			userConfigurationDirectory / "libcamera" / configName;
>>   		if (loadFile(user_configuration_file))
>>   			return !!yamlConfiguration_;
>>   	}
>>   -	for (const auto &path : globalConfigurationFiles) {
>> -		if (loadFile(path))
>> +	for (const auto &path : globalConfigurationDirectories)
>> +		if (loadFile(path / configName))
>>   			return !!yamlConfiguration_;
>> -	}
>>     	yamlConfiguration_ = std::make_unique<YamlObject>();
>>   	return true;