[PATCH i-g-t v2 6/7] runner/settings: Serialize command line
Lucas De Marchi
lucas.demarchi at intel.com
Tue Jan 28 19:34:24 UTC 2025
On Tue, Jan 28, 2025 at 07:37:44PM +0100, Kamil Konieczny wrote:
>Hi Lucas,
>On 2025-01-21 at 14:57:32 -0800, Lucas De Marchi wrote:
>> Serialize the command line to metadata.txt. The expected format in the
>> metadata.txt is like below:
>>
>> cmdline.argc : 6
>> cmdline.argv[0] : ./build/runner/igt_runner
>> cmdline.argv[1] : -o
>
>Sorry for late reponse but imho it should be saved for debugging
>purpose but not for reading this in metadata.txt, especially that
>this option '-o' is 'override existing results folder', so it
>should be used only once by igt_runner in setup phase but not for
>igt_resume.
>
>From igt_runner --help:
>-o, --overwrite If the results-path already exists, delete it
if you are using -o it's just because you don't care about the previous
results and want to completely overwrite it.
igt_resume doesn't have any option - it doesn't have a -o and it
basically does:
"Load the settings from the state dir back into the settings struct"...
igt_resume will read it back and create the settings object.
The results.json as architected in igt_runner discards everything and
is basically a conversion format from the state dir to a .json file at
the end of the execution.
>
>imho we should save it into some different file and include it into
>results.json
not sure I follow... metadata.txt has the details of how we are running
igt_runner and it's basically a dump of the settings object. I don't
follow what's the relation with the -o option you mentioned above.
End goal is to have the value in the results.json, but I see no reason
why the intermediary state needs to be in a different file.
Lucas De Marchi
>
>Also +Cc Petri
>Cc: Petri Latvala <adrinael at adrinael.net>
>
>Regards,
>Kamil
>
>> cmdline.argv[2] : --test-list
>> cmdline.argv[3] : /tmp/testlist.txt
>> cmdline.argv[4] : build/tests/
>> cmdline.argv[5] : /tmp/results
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> runner/settings.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>> runner/settings.h | 5 +++++
>> 2 files changed, 47 insertions(+)
>>
>> diff --git a/runner/settings.c b/runner/settings.c
>> index 2787869ee..ed1afc205 100644
>> --- a/runner/settings.c
>> +++ b/runner/settings.c
>> @@ -529,6 +529,18 @@ static void free_hook_strs(struct igt_vec *hook_strs)
>> igt_vec_fini(hook_strs);
>> }
>>
>> +static void free_cmdline(struct settings *settings)
>> +{
>> + if (!settings->cmdline.allocated)
>> + return;
>> +
>> + for (size_t i = 0; i < settings->cmdline.argc; i++)
>> + free(settings->cmdline.argv[i]);
>> +
>> + free(settings->cmdline.argv);
>> +}
>> +
>> +
>> static bool file_exists_at(int dirfd, const char *filename)
>> {
>> return faccessat(dirfd, filename, F_OK, 0) == 0;
>> @@ -646,6 +658,7 @@ void clear_settings(struct settings *settings)
>> free_regexes(&settings->exclude_regexes);
>> free_env_vars(&settings->env_vars);
>> free_hook_strs(&settings->hook_strs);
>> + free_cmdline(settings);
>>
>> init_settings(settings);
>> }
>> @@ -875,6 +888,8 @@ bool parse_options(int argc, char **argv,
>> goto error;
>> }
>>
>> + settings->cmdline.argc = argc;
>> + settings->cmdline.argv = argv;
>>
>> return true;
>>
>> @@ -1055,6 +1070,7 @@ static bool serialize_hook_strs(struct settings *settings, int dirfd)
>> bool serialize_settings(struct settings *settings)
>> {
>> #define SERIALIZE_LINE(f, s, name, format) fprintf(f, "%s : " format "\n", #name, s->name)
>> +#define SERIALIZE_ARRAY_ITEM(f, s, name, _i, format) fprintf(f, "%s[%d] : " format "\n", #name, _i, s->name[_i])
>>
>> FILE *f;
>> int dirfd, covfd;
>> @@ -1123,6 +1139,10 @@ bool serialize_settings(struct settings *settings)
>> SERIALIZE_LINE(f, settings, cov_results_per_test, "%d");
>> SERIALIZE_LINE(f, settings, code_coverage_script, "%s");
>>
>> + SERIALIZE_LINE(f, settings, cmdline.argc, "%d");
>> + for (int i = 0; i < settings->cmdline.argc; i++)
>> + SERIALIZE_ARRAY_ITEM(f, settings, cmdline.argv, i, "%s");
>> +
>> if (settings->sync) {
>> fflush(f);
>> fsync(fileno(f));
>> @@ -1177,9 +1197,21 @@ static char *parse_str(char **pval)
>> s->field = _f(&val); \
>> goto cleanup; \
>> }
>> +#define PARSE_LINE_ARRAY(s, name, val, field, _f, _max) \
>> + do { \
>> + int idx; \
>> + if (sscanf(name, #field "[%u]", &idx) == 1 && \
>> + idx < s->_max) { \
>> + s->field[idx] = _f(&val); \
>> + goto cleanup; \
>> + } \
>> + } while (0)
>> +
>> #define PARSE_INT(s, name, val, field) PARSE_LINE(s, name, val, field, parse_int)
>> #define PARSE_UL(s, name, val, field) PARSE_LINE(s, name, val, field, parse_ul)
>> #define PARSE_STR(s, name, val, field) PARSE_LINE(s, name, val, field, parse_str)
>> +#define PARSE_ARRAY_STR(s, name, val, field, _max) \
>> + PARSE_LINE_ARRAY(s, name, val, field, parse_str, _max)
>>
>> bool read_settings_from_file(struct settings *settings, FILE *f)
>> {
>> @@ -1211,6 +1243,15 @@ bool read_settings_from_file(struct settings *settings, FILE *f)
>> PARSE_INT(settings, name, val, enable_code_coverage);
>> PARSE_INT(settings, name, val, cov_results_per_test);
>> PARSE_STR(settings, name, val, code_coverage_script);
>> + PARSE_INT(settings, name, val, cmdline.argc);
>> +
>> + if (settings->cmdline.argc && !settings->cmdline.argv) {
>> + settings->cmdline.allocated = true;
>> + settings->cmdline.argv = calloc(settings->cmdline.argc,
>> + sizeof(*settings->cmdline.argv));
>> + }
>> +
>> + PARSE_ARRAY_STR(settings, name, val, cmdline.argv, cmdline.argc);
>>
>> printf("Warning: Unknown field in settings file: %s = %s\n",
>> name, val);
>> @@ -1234,6 +1275,7 @@ cleanup:
>> return true;
>> }
>> #undef PARSE_LINE
>> +#undef PARSE_LINE_ARRAY
>>
>> /**
>> * read_env_vars_from_file() - load env vars from a file
>> diff --git a/runner/settings.h b/runner/settings.h
>> index f69f09778..d563a0d16 100644
>> --- a/runner/settings.h
>> +++ b/runner/settings.h
>> @@ -75,6 +75,11 @@ struct settings {
>> char *code_coverage_script;
>> bool enable_code_coverage;
>> bool cov_results_per_test;
>> + struct {
>> + bool allocated;
>> + int argc;
>> + char **argv;
>> + } cmdline;
>> };
>>
>> /**
>> --
>> 2.48.0
>>
More information about the igt-dev
mailing list