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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.02.2023 10:30, Luca Fancellu wrote:
> 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'm having trouble making a connection between a file having an entry here
and the need/desire to include it in scanning. Can you clarify please why
you see an implication from one to the other here?

> 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.

Maybe it's easier, but it's then also less useful. For example I view
it as quite relevant what the origin of a file is. That may, for
example, have an implication on whether "backport" is sensible to set
to "true" (origin projects could, after all, be largely unmaintained,
and hence it may be difficult to get any changes into there).

> 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.

Couldn't documentation of the fields live at the top of the file itself?

> 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)
> 
> 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

Personally I'd drop "url of" - the specification of the origin needs to be
precise, but I'm not convinced it absolutely needs to come in the form of
a URL. What would imo be more important to mandate is that the reference
be to the "canonical" copy of the project, not e.g. some github clone.

> 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.

I guess this needs clarifying: What revision is meant here? The one originally
imported, the one last updated from, or yet something else? Keep in mind that
we may also be selectively importing changes, in which case any particular
"revision" can easily end up misleading. The format likely also wants
specifying, e.g. (like we do for Fixes: tags) the first 12 digits of a commit
hash. For said selective updating this might then allow for something like
<base-hash>+<cherry-pick>+...

Jan