[PATCH i-g-t v4 3/5] runner/settings: Add function to set IGT_DATA_PATH environment variable

[Kamil Konieczny]

Hi Naladala,
On 2025-03-26 at 02:31:44 +0530, Naladala Ramanaidu wrote:
> Add environment variable to obtain absolute path for PNG files
> at runtime.
> 
> v2: Fix review comments. (Kamil)
> v3: Fix review comments. (kamil)
> V4: Fix review comments. (kamil)
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
>  runner/settings.c | 22 ++++++++++++++++++++++
>  runner/settings.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/runner/settings.c b/runner/settings.c
> index 1ccf345d9..94c95f26e 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -612,6 +612,21 @@ char *absolute_path(const char *path)
>  	return result;
>  }
>  
> +int set_tests_datadir(void)
> +{
> +	const char *datapath = "../share/igt-gpu-tools";

Hmm, maybe try also a build folder for local runs?

../build/data
../../build/data

> +	char *abpath;
> +
> +	abpath = absolute_path(datapath);

Make sure abpath is not NULL before using it.

> +
> +	if (getenv("IGT_DATA_PATH") == NULL)

Please make it simpler:
	if (getenv("IGT_DATA_PATH"))
		return 0;

And here add checks and settings.

> +		return setenv("IGT_DATA_PATH", abpath, 1);

Btw no need for "1" - you already checked it is unset.

> +
> +	return 0;
> +}
> +
> +
> +
>  static char *bin_path(char *fname)
>  {
>  	char *path, *p;
> @@ -654,6 +669,10 @@ void init_settings(struct settings *settings)
>  {
>  	memset(settings, 0, sizeof(*settings));
>  	IGT_INIT_LIST_HEAD(&settings->env_vars);
> +
> +	if (set_tests_datadir())
> +		fprintf(stderr, "Data dir path not set\n");

Could you also add printing of data path here?

	else
		printf("IGT data/ path: %s/\n", setting->data_path);

> +
>  	igt_vec_init(&settings->hook_strs, sizeof(char *));
>  }
>  
> @@ -671,6 +690,9 @@ void clear_settings(struct settings *settings)
>  	free_hook_strs(&settings->hook_strs);
>  	free_array_deep((void **)settings->cmdline.argv, settings->cmdline.argc);
>  
> +	if (getenv("IGT_DATA_PATH") != NULL)
> +		unsetenv("IGT_DATA_PATH");
> +

Please remember that is was set by runner so only unset this
if runner was setting this env var.

Overall looks good,

Regards,
Kamil

>  	init_settings(settings);
>  }
>  
> diff --git a/runner/settings.h b/runner/settings.h
> index 30743fc40..cbcf624b0 100644
> --- a/runner/settings.h
> +++ b/runner/settings.h
> @@ -156,5 +156,6 @@ bool serialize_settings(struct settings *settings);
>  
>  bool read_settings_from_file(struct settings *settings, FILE* f);
>  bool read_settings_from_dir(struct settings *settings, int dirfd);
> +int set_tests_datadir(void);
>  
>  #endif
> -- 
> 2.43.0
>