Hi Oleksandr, This morning, we had a discussion among maintainers, and the suggested approach moving forward is as follows:
- First, it would be helpful to see a sample of the proposed changes applied to a single source file as an example. If you could provide such a patch, it would help advance the discussion. - If the changes are acceptable, we need to properly document the new coding style in xen.git. If not, we will need to iterate again. We may also need to add a "xen" template to clang-format. - Once finalized, we will proceed by making changes to the Xen source code piece by piece, as you suggested, rather than applying a single large patch. Let me know your thoughts. Cheers, Stefano On Mon, 10 Feb 2025, Oleksandr Andrushchenko wrote: > Hello, everybody! > > What is the rationale behind introducing a tool to help with coding style > verification? I will partially quote Viktor Mitin here [2]: > > "The Xen Project has a coding standard in place, but like many projects, the > standard is only enforced through peer review. Such mistakes slip through and > code is imported from other projects which may not follow the same standard. > The > goal would be to come up with a tool that can audit the code base as part of a > CI loop for code style inconsistencies and potentially provide corrections. > This > tool is to embed as a part of the continuous integration loop." > > I would add that it would better reflect reality to say that Xen's coding > style > is quite incomplete to become a standard and needs some improvement to achieve > that. > > Here, I would like to provide a bit of history and acknowledge those brave > individuals who dared and tried to introduce to Xen coding style checking and > formatting support with clang-format. > > Year 2017, Ishani Chugh. > --------------------------------------------------------------------- > This journey began with a request from an Outreachy program member [1]. > Then came the first patches by Ishani Chugh [2] > Status: *busted*. > > Year 2019, Viktor Mitin > --------------------------------------------------------------------- > Then picked up by Viktor Mitin, EPAM's first attempt [3]. > Status: *busted*. > > Year 2020, Anastasiia Lukianenko > --------------------------------------------------------------------- > Continued by Anastasiia Lukianenko, EPAM's second attempt [4], [5]. > Some contributions were made to LLVM to make clang-format a better fit for > Xen [6]. > Xen-summit and presentation [7] and the summary document [8]. > Status: *busted*. > > Year 2023, Luca Fancellu > --------------------------------------------------------------------- > Luca restarted it, first ARM attempt [9], [10], [11]. > Status: *busted*. > > That's all for now, but it is still impressive as of 2025. > > So, in my opinion, what were the main issues with all these attempts? There > are > many different views, but I would emphasize the following: > > 1) clang-format doesn't perfectly fit Xen's code style as some rules it > applies > are not liked by the community or it applies rules that Xen's coding style > doesn't define (those Luca described in his .clang-format for every clang > option). > > 2) clang-format doesn't work in a "one-option-at-a-time" mode [12]: clang > maintainers strongly oppose requests to allow turning off all options except > some. Believe it or not, other maintainers also have strong opinions on what > is > right and what is not for their projects, and this is one area where they will > not compromise. > > 3) The size of the patch after applying clang-format is huge. Really. > Something > like 9 MB. Even if everyone agrees that the approach is good and we can > proceed > with it, it is highly unlikely anyone will be able to review it. Considering > that new patches are being added to the upstream during such a review, it may > also lead to new code style violations or require a new review of that huge > patch. > > 4) Which clang-format version should we set as the one used by Xen, so it is > easy for everyone to use it on their hosts? > > 5) You name it. I think many people in the community can name their points for > and against clang-format. > > So, in this attempt, I would suggest the following approach: > I think that I could start sending patches after the latest .clang-format 21 > for > some part of Xen, ARM code base for example, using work already done by Luca. > This way: > > 1) Patches are formatted with clang-format, which is a strong plus. > 2) The diff is well maintained and I can still alter it by hand if there are > comments or dislikes. > 3) Finally, when the patch is accepted, we can be more confident that > clang-format will find far fewer inconsistencies than if it were just applied > to > the "raw" code. Thus, the next time clang-format runs, it will produce a much > smaller patch than before. > 4) Finally, introduce clang-format and run it on the leftovers: at this stage, > it would be much easier to discuss every option clang has and the resulting > output it produces. > 5) Update existing/add new rules to the Xen coding style based on community > agreements and the results of this activity. > > We may define the subsystems to start this activity on and also define an > acceptable size of the patch for review, say 100K. Considering that the longer > the review, the more outdated the patch becomes and will require a new round > as > new code comes in. > > I would love to hear from the community on this approach and finally get it > done. Not busted. > > Stay safe, > Oleksandr Andrushchenko > > [1] > https://lore.kernel.org/xen-devel/1130763396.5603480.1492372859631.javamail.zim...@research.iiit.ac.in/T/#m1db2521362edd286875acf10296873993226dcf2 > [2] https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg01739.html > [3] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01862.html > [4] https://lists.xenproject.org/archives/html/xen-devel/2020-09/msg02157.html > [5] https://lists.xenproject.org/archives/html/xen-devel/2020-10/msg00022.html > [6] https://reviews.llvm.org/D91950 > [7] > https://xenproject.org/blog/clang-format-for-xen-coding-style-checking-scheduled/ > [8] > https://docs.google.com/document/d/1MDzYkPgfVpI_UuO_3NRXsRLAXqIZ6pj2btF7vsMYj8o > [9] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02294.html > [10] > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00498.html > [11] > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html > [12] https://github.com/llvm/llvm-project/issues/54137#issuecomment-1058564570 >
