Re: Proposal: List files in Xen originated from external sources

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 3 Feb 2023, Luca Fancellu wrote:
> Hi all,
> 
> I’m starting a proposal for the external files that needs to be removed from 
> the MISRA scan,
> the work was originally started by Michal here:
> https://patchwork.kernel.org/project/xen-devel/patch/20221116092032.4423-1-michal.orzel@xxxxxxx/
> so I started by that thread, the aim of this work is to have an initial 
> format to start as soon as possible to
> exclude the external files from the MISRA scan (at least initially from 
> cppcheck).
> 
> I think we can use the JSON format because it’s easy to integrate it with 
> python and it’s easy to add/remove
> fields from the structure without interfering with the other elements during 
> time, so we can define a structure
> now but if in the future we see the needs for additional field, we can just 
> add them without changes to the
> analysis script.
> 
> In my opinion many of these fields can be left empty in a first push, to let 
> the script exclude the files and during
> time with the contributions of the community we can add the missing 
> informations.
> I think it’s easier for the community to pick an entry, do some research to 
> fill the gaps and push, instead to wait
> until having all the informations before adding the entry.
> 
> 
> This is my first though for the fields, let me know yours:
> 
> docs/misra/external-files.json:
> {
>  "version": "1.0”,
>  "content": [
>    {
>      "path": "relative/path/from/xen/“,
>      "backport": True/False,
>      "origin_project": "URL to origin project",
>      "origin_path": "relative path in the original project",
>      "origin_revision": "revision in original project”
>    }
>  ]
> }
> 
> 
> Maybe, documentation for this file and the fields can reside in 
> docs/misra/external-files.rst.
> 
> Here follows the explanation for the fields:
> 
> path: is a relative path from the xen base folder, it can refer to a file or 
> it can be a path to an entire folder
>          (Taking as example libfdt)

I think "path" should be the only mandatory field. path alone should be
sufficient for a MISRA C scanner to know if a file or directory should
be excluded.


> backport: it’s a boolean flag that says if the file is subject to backport, 
> so every file where this field is true
>                 won’t be included in the analysis. A file is subject to 
> backport when it’s external and it doesn’t
>                 accept direct changes (changes needs to be made in the 
> original project and then backported
>                 to Xen)
> 
> origin_project: url of the repository where this file was originated
> 
> origin_path: relative path in the original project, mostly linked to the 
> original_revision field
> 
> origin_revision: revision of the changes in the repository when this file was 
> taken.
 
These other fields (backport, origin_project, origin_path,
origin_revision) are nice to have but not required. They don't provide
information necessary to exclude something from MISRA C scans, however
they provide information that are useful to the Xen maintainers.

This is to say that we should introduce these fields only if they are
useful to us human maintainers, not automated tools.

That said, I think they are all useful and I would keep them as you did.


> Now, I’m not entirely sure about the field “backport”, because if a file is 
> not subject to backport, then why
> we should not make direct changes? 

"backport" is meant to say that *only* backports are allowed, no direct
changes to the source file. E.g. only backports from Linux commits, so
first you need to get your patch in Linux, then we'll take it from
there.


> Shall we maybe change its codestyle and convert it to Xen codestyle?

Typically if you only accept backports to a file, it doesn't make any
sense to convert it to Xen codestyle. But it is not a hard rule and I
don't think we should say anything in that regard here.


> And then, if the file is not subject to backport and now we “own” the file, 
> there is no more the need to list it
> in the external file, a commit search can reveal when it was converted to Xen 
> codestyle and removed from
> the external file list so we don’t lose the history.

Yes, I agree. If a file is coverted to Xen coding style and follows
MISRA rules, then there is no need to list the file in
docs/misra/external-files.json.


> So we would end up excluding just all the file listed in external-file.json 
> by the analysis.

Right, I think so too