Re: [PATCH i-g-t v6 12/17] scripts/igt_doc: Add '--exclude-files' parameter

["Manszewski, Christoph" <christoph.manszewski@intel.com>]

Hi all,

On 9.09.2024 15:57, Zbigniew Kempczyński wrote:
> On Mon, Sep 09, 2024 at 01:31:25PM +0200, Kamil Konieczny wrote:
>> Hi Christoph,
>> On 2024-09-05 at 11:28:07 +0200, Christoph Manszewski wrote:
>>> Currently the doc scripts relies on a static list of files passed
>>> through the respective '<config>.json' file for generating
>>> documentation. This results in a compilation error when we exclude some
>>> tests from building since the doc script notices documentation generated
>>> for missing binaries.
>>>
>>> Make it possible to dynamically exclude files from doc generation using
>>> '--exclude-files' parameter. Merge excluded files passed with that parameter
>>> with files excluded from the test config '.json' files. Align the behavior
>>> of the '--files' parameter to also merge with files included from the config.
>>
>> Could we get rid of this option and instead check if there was
>> a binary file generated for a given C source file?
>> As I understand it, we could skip C source checks if there are
>> no binary generated.
> 
> I like this idea, I'll ask Christoph for drop exclude-files option and
> check corresponding .c files only if binary exists.

I already replied to Kamil's patch and will include it in this series 
instead of this one. Note however that this results in generating 
documentation for tests that weren't included in the build. I was 
specifically trying to avoid that but since Kamil suggests a change like 
this I guess it's fine.

Thanks,
Christoph
> 
> --
> Zbigniew
> 
>>
>> Or are there any other constrains like tests planned?
>>
>> Adding Jari and Katarczyna on Cc.
>>
>> Cc: Jari Tahvanainen <jari.tahvanainen@intel.com>
>> Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
>>
>> Regards,
>> Kamil
>>
>>>
>>> Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
>>> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>> ---
>>>   scripts/igt_doc.py   |  3 +++
>>>   scripts/test_list.py | 47 +++++++++++++++++++++++++-------------------
>>>   2 files changed, 30 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
>>> index fa2c2c7ca..9b7317feb 100755
>>> --- a/scripts/igt_doc.py
>>> +++ b/scripts/igt_doc.py
>>> @@ -313,6 +313,8 @@ def main():
>>>                           help="Generate testlists for Intel CI integration at the INTELCI_TESTLIST directory.")
>>>       parser.add_argument('--files', nargs='+',
>>>                           help="File name(s) to be processed")
>>> +    parser.add_argument('--exclude-files', nargs='+',
>>> +                        help="File name(s) to ignore")
>>>   
>>>       parse_args = parser.parse_args()
>>>   
>>> @@ -325,6 +327,7 @@ def main():
>>>           tests = IgtTestList(config_fname = config,
>>>                               include_plan = parse_args.include_plan,
>>>                               file_list = parse_args.files,
>>> +                            exclude_file_list = parse_args.exclude_files,
>>>                               igt_build_path = parse_args.igt_build_path)
>>>   
>>>           if parse_args.filter_field:
>>> diff --git a/scripts/test_list.py b/scripts/test_list.py
>>> index 69c830ca1..4fe209e5a 100644
>>> --- a/scripts/test_list.py
>>> +++ b/scripts/test_list.py
>>> @@ -250,7 +250,7 @@ class TestList:
>>>                    config_dict = None, sources_path = None,
>>>                    test_tag = "TEST", subtest_tag = "SUBTESTS?",
>>>                    main_name = "igt", planned_name = "planned",
>>> -                 subtest_separator = "@"):
>>> +                 subtest_separator = "@", exclude_file_list = None):
>>>           self.doc = {}
>>>           self.test_number = 0
>>>           self.config = None
>>> @@ -368,28 +368,35 @@ class TestList:
>>>           if "_properties_" in self.props:
>>>               del self.props["_properties_"]
>>>   
>>> -        has_implemented = False
>>> +        if not exclude_file_list:
>>> +            exclude_file_list = []
>>> +        else:
>>> +            exclude_file_list = [os.path.normpath(f) for f in exclude_file_list]
>>> +
>>> +        exclude_file_glob = self.config.get("exclude_files", [])
>>> +        for cfg_file in exclude_file_glob:
>>> +            cfg_file = cfg_path + cfg_file
>>> +            for fname in glob.glob(cfg_file):
>>> +                exclude_file_list.append(fname)
>>> +
>>>           if not self.filenames:
>>>               self.filenames = []
>>> -            exclude_files = []
>>> -            files = self.config["files"]
>>> -            exclude_file_glob = self.config.get("exclude_files", [])
>>> -            for cfg_file in exclude_file_glob:
>>> -                cfg_file = cfg_path + cfg_file
>>> -                for fname in glob.glob(cfg_file):
>>> -                    exclude_files.append(fname)
>>> -
>>> -            for cfg_file in files:
>>> -                cfg_file = cfg_path + cfg_file
>>> -                for fname in glob.glob(cfg_file):
>>> -                    if fname in exclude_files:
>>> -                        continue
>>> -                    self.filenames.append(fname)
>>> -                    has_implemented = True
>>>           else:
>>> -            for cfg_file in self.filenames:
>>> -                if cfg_file:
>>> -                    has_implemented = True
>>> +            self.filenames = [os.path.normpath(f) for f in self.filenames]
>>> +
>>> +        files = self.config["files"]
>>> +        for cfg_file in files:
>>> +            cfg_file = cfg_path + cfg_file
>>> +            for fname in glob.glob(cfg_file):
>>> +                if fname in exclude_file_list:
>>> +                    continue
>>> +                self.filenames.append(fname)
>>> +
>>> +        has_implemented = False
>>> +        for cfg_file in self.filenames:
>>> +            if cfg_file:
>>> +                has_implemented = True
>>> +                break
>>>   
>>>           has_planned = False
>>>           if include_plan and "planning_files" in self.config:
>>> -- 
>>> 2.34.1
>>>