[PATCH i-g-t v2 6/7] runner/settings: Serialize command line

[Lucas De Marchi]

On Wed, Jan 29, 2025 at 08:09:14PM +0200, Petri Latvala wrote:
>On Wed, Jan 29, 2025 at 06:23:48PM +0100, Kamil Konieczny wrote:
>> Hi Lucas,
>> On 2025-01-28 at 13:34:24 -0600, Lucas De Marchi wrote:
>> > 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
>>
>> Well, what I try to comment on a commit description:
>>
>> > Serialize the command line to metadata.txt. [...]
>>
>> imho this should be written in setup.txt and setup.txt should be
>> included in results.json There is no need for igt_runner options
>> in metadata.txt, as it is used by igt_resume in shard re-runs
>> after a reboot happens in a middle of testlist.
>
>The command line in metadata.txt does no harm as it's just
>informational text to be recorded in the final result. It's also
>supposed to indeed be a direct dump of the settings object, and thus
>should have the command line if that is to be put into the settings
>object. But I suppose the pertinent question is: why does it need to
>be put into the settings object? There's already things like uname.txt
>and starttime.txt and whatnot, written outside of
>metadata.txt. Consolidate miscellaneous info dumps to a single file?

yes, ideally we'd have it consolidated in a single file if we can, which
motivated me to improve the load/store of metadata.txt. IMO it's easier
to have a single format and use the same functions for dump/restore. It
also allows one to always fallback to 1 file when the visualization is
not updated.

Example:

For https://intel-gfx-ci.01.org/tree/intel-xe/xe-2570-bdfc862a5c719d934efbdedb050ef891b9feef15/bat-lnl-1/igt@fbdev@eof.html
if something is missing I can simply look at 
https://intel-gfx-ci.01.org/tree/intel-xe/xe-2570-bdfc862a5c719d934efbdedb050ef891b9feef15/bat-lnl-1/metadata0.txt

and get the missing information rather than figuring out the right
intermediate state file used by igt_runner.

https://intel-gfx-ci.01.org/tree/intel-xe/xe-2570-bdfc862a5c719d934efbdedb050ef891b9feef15/bat-lnl-1/metadata0.txt

thanks
Lucas De Marchi

>Do that when serializing command line _somewhere_, or perhaps later?
>
>I have no opinions to offer in any direction, just drive-by commenting
>here...
>
>
>-- 
>Petri Latvala