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
