| Message ID | 20210827144753.419929-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Fri, Aug 27, 2021 at 03:47:53PM +0100, Kieran Bingham wrote: > The .clang-format file can use CaseSensitive on Regex rules for Include > Categories. With this we can distinguish between QT headers which s/QT/Qt/ > commence with a 'Q' verses system headers including <queue>. > > This requires clang-format-12 to support that rule, and clang-format is > not very amicable to .clang-format files with unsupported entries, and > will fail to run on previous versions if we add that option directly. > > This causes the checkstyle utility to become unusable for users of > clang-format versions prior to 12. > > Unfortunately, clang-format does not provide a way to specify a specific > file to use for the rules to apply when formatting, so we can not easily > call clang-format with a file specific to its versioned needs. > > Implement a means of identifying the version of clang-format, and use > that information to search for the most recent supportable > .clang-format-$(version) file, which can then be symlinked to the fixed > file location used by clang format (.clang-format). > > .clang-format-12: is added with CaseSensitive:true for QT headers Ditto. > .clang-format-7: is moved from the current .clang-format implementation > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > Note there are few things I dislike about this, hence RFC: > > - We leave our tree without a .clang-format until checkstyle runs.. A bit annoying, but I think we can live with that. Maybe it's a good way to encourage usage of checkstyle.py :-) > - We run the unlinking and symlinking on every invocation of checkstyle Shouldn't be too costly :-) It's slightly annoying that we can't run checkstyle.py on a read-only directory anymore though, but I don't think it's a common use case. > - The 'autogenerated' file in place of a git tracked one causes > issues on rebase. Could be an issue for bisects...? Possibly, but I don't think we'll often bisect and run checkstyle.py at the same time. > > .clang-format-12 | 169 +++++++++++++++++++++++++++++++ > .clang-format => .clang-format-7 | 0 > utils/checkstyle.py | 49 ++++++++- The symlink should be added to .gitignore. > 3 files changed, 217 insertions(+), 1 deletion(-) > create mode 100644 .clang-format-12 > rename .clang-format => .clang-format-7 (100%) > > diff --git a/.clang-format-12 b/.clang-format-12 > new file mode 100644 > index 000000000000..98e9fad472fc > --- /dev/null > +++ b/.clang-format-12 > @@ -0,0 +1,169 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# clang-format configuration file. Intended for clang-format >= 12. > +# > +# For more information, see: > +# > +# Documentation/process/clang-format.rst > +# https://clang.llvm.org/docs/ClangFormat.html > +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html > +# > +--- > +Language: Cpp > +AccessModifierOffset: -8 > +AlignAfterOpenBracket: Align > +AlignConsecutiveAssignments: false > +AlignConsecutiveDeclarations: false > +AlignEscapedNewlines: Right > +AlignOperands: true > +AlignTrailingComments: false > +AllowAllParametersOfDeclarationOnNextLine: false > +AllowShortBlocksOnASingleLine: false > +AllowShortCaseLabelsOnASingleLine: false > +AllowShortFunctionsOnASingleLine: InlineOnly > +AllowShortIfStatementsOnASingleLine: false > +AllowShortLoopsOnASingleLine: false > +AlwaysBreakAfterDefinitionReturnType: None > +AlwaysBreakAfterReturnType: None > +AlwaysBreakBeforeMultilineStrings: false > +AlwaysBreakTemplateDeclarations: MultiLine > +BinPackArguments: true > +BinPackParameters: true > +BraceWrapping: > + AfterClass: true > + AfterControlStatement: false > + AfterEnum: false > + AfterFunction: true > + AfterNamespace: false > + AfterObjCDeclaration: false > + AfterStruct: false > + AfterUnion: false > + AfterExternBlock: false > + BeforeCatch: false > + BeforeElse: false > + IndentBraces: false > + SplitEmptyFunction: true > + SplitEmptyRecord: true > + SplitEmptyNamespace: true > +BreakBeforeBinaryOperators: None > +BreakBeforeBraces: Custom > +BreakBeforeInheritanceComma: false > +BreakInheritanceList: BeforeColon > +BreakBeforeTernaryOperators: true > +BreakConstructorInitializers: BeforeColon > +BreakAfterJavaFieldAnnotations: false > +BreakStringLiterals: false > +ColumnLimit: 0 > +CommentPragmas: '^ IWYU pragma:' > +CompactNamespaces: false > +ConstructorInitializerAllOnOneLineOrOnePerLine: false > +ConstructorInitializerIndentWidth: 8 > +ContinuationIndentWidth: 8 > +Cpp11BracedListStyle: false > +DerivePointerAlignment: false > +DisableFormat: false > +ExperimentalAutoDetectBinPacking: false > +FixNamespaceComments: true > +ForEachMacros: > + - 'udev_list_entry_foreach' > +SortIncludes: true > +IncludeBlocks: Regroup > +IncludeCategories: > + # Headers matching the name of the component are matched automatically. > + # Priority 1 > + # Other library headers (explicit overrides to match before system headers) > + - Regex: '(<jpeglib.h>|<libudev.h>|<tiffio.h>|<xf86drm.h>|<xf86drmMode.h>|<yaml.h>)' > + Priority: 9 > + # Qt includes (match before C++ standard library) > + - Regex: '<Q([A-Za-z0-9\-_])+>' > + Priority: 9 > + CaseSensitive: true > + # Headers in <> with an extension. (+system libraries) > + - Regex: '<([A-Za-z0-9\-_])+\.h>' > + Priority: 2 > + # System headers > + - Regex: '<sys/.*>' > + Priority: 2 > + # C++ standard library includes (no extension) > + - Regex: '<([A-Za-z0-9\-_/])+>' > + Priority: 2 > + # Linux headers, as a second group/subset of system headers > + - Regex: '<linux/.*>' > + Priority: 3 > + # Headers for libcamera Base support > + - Regex: '<libcamera/base/private.h>' > + Priority: 4 > + - Regex: '<libcamera/base/.*\.h>' > + Priority: 5 > + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/) > + - Regex: '<libcamera/([A-Za-z0-9\-_])+.h>' > + Priority: 6 > + # IPA Interfaces > + - Regex: '<libcamera/ipa/.*\.h>' > + Priority: 7 > + # libcamera Internal headers in "" > + - Regex: '"libcamera/internal/.*\.h"' > + Priority: 8 > + # Other libraries headers with one group per library (.h or .hpp) > + - Regex: '<.*/.*\.hp*>' > + Priority: 9 > + # local modular includes "path/file.h" (.h or .hpp) > + - Regex: '"(.*/)+.*\.hp*"' > + Priority: 10 > + # Other local headers "file.h" with extension (.h or .hpp) > + - Regex: '".*.hp*"' > + Priority: 11 > + # Any unmatched line, separated from the last group > + - Regex: '"*"' > + Priority: 100 > + > +IncludeIsMainRegex: '(_test)?$' > +IndentCaseLabels: false > +IndentPPDirectives: None > +IndentWidth: 8 > +IndentWrappedFunctionNames: false > +JavaScriptQuotes: Leave > +JavaScriptWrapImports: true > +KeepEmptyLinesAtTheStartOfBlocks: false > +MacroBlockBegin: '' > +MacroBlockEnd: '' > +MaxEmptyLinesToKeep: 1 > +NamespaceIndentation: None > +ObjCBinPackProtocolList: Auto > +ObjCBlockIndentWidth: 8 > +ObjCSpaceAfterProperty: true > +ObjCSpaceBeforeProtocolList: true > + > +# Taken from git's rules > +PenaltyBreakAssignment: 10 > +PenaltyBreakBeforeFirstCallParameter: 30 > +PenaltyBreakComment: 10 > +PenaltyBreakFirstLessLess: 0 > +PenaltyBreakString: 10 > +PenaltyBreakTemplateDeclaration: 10 > +PenaltyExcessCharacter: 100 > +PenaltyReturnTypeOnItsOwnLine: 60 > + > +PointerAlignment: Right > +ReflowComments: false > +SortIncludes: true > +SortUsingDeclarations: true > +SpaceAfterCStyleCast: false > +SpaceAfterTemplateKeyword: false > +SpaceBeforeAssignmentOperators: true > +SpaceBeforeCpp11BracedList: false > +SpaceBeforeCtorInitializerColon: true > +SpaceBeforeInheritanceColon: true > +SpaceBeforeParens: ControlStatements > +SpaceBeforeRangeBasedForLoopColon: true > +SpaceInEmptyParentheses: false > +SpacesBeforeTrailingComments: 1 > +SpacesInAngles: false > +SpacesInContainerLiterals: false > +SpacesInCStyleCastParentheses: false > +SpacesInParentheses: false > +SpacesInSquareBrackets: false > +Standard: Cpp11 > +TabWidth: 8 > +UseTab: Always > +... I trust you on this. > diff --git a/.clang-format b/.clang-format-7 > similarity index 100% > rename from .clang-format > rename to .clang-format-7 > diff --git a/utils/checkstyle.py b/utils/checkstyle.py > index ececb46eaacc..279ace8346b4 100755 > --- a/utils/checkstyle.py > +++ b/utils/checkstyle.py > @@ -16,6 +16,7 @@ > import argparse > import difflib > import fnmatch > +import glob > import os.path > import re > import shutil > @@ -566,9 +567,55 @@ class Formatter(metaclass=ClassRegistry): > return patterns > > > -class CLangFormatter(Formatter): > +# Identify the version of clang-format running, and match it against locally > +# available versioned configuration files of the form: > +# .clang-format-$(version) > +# > +# The highest supported local configuration file is linked as .clang-format by > +# ClangFormatLinker.relink() for use by the ClangFormatter. > +# > +class ClangFormatLinker(): No need for parentheses if you don't inherit from another class, but you can inherit from object. Do we need ClangFormatLinker though ? Couldn't those functions be move to ClangFormatter ? Although relinking should probably be done from main(), see below. > + def version(): > + version_regex = re.compile('[^0-9]*([0-9]+).*') > + ret = subprocess.run(['clang-format', '--version'], stdout=subprocess.PIPE) What happens if clang-format isn't available ? We check for it in main(), but I think this code will run before that. > + version = ret.stdout.decode('utf-8') > + search = version_regex.search(version) > + if search: > + version = search.group(1) > + return int(version) > + > + # Lowest supported version > + return 7 Shouldn't this cause an error instead ? > + > + def supported(): > + version = ClangFormatLinker.version() That's a bit of an abuse of the Python class system. Instance methods are supposed to be called on an instance and take a self argument. These three functions should be declared as class method I think. This would become @classmethod def supported(cls): version = cls.version() > + > + # Match the highest supported version > + clang_files = glob.glob('.clang-format-[0-9]*') When checkstyle.py is run manually, it may be run from a subdirectory of the git tree. You should pass the git top-level directory to relink(). This would then call for calling relink() directly from main() instead of ClangFormatter. > + clang_files = sorted(clang_files, reverse=True, > + key=lambda s: [int(re.sub("[^0-9]", "", s))]) We use single quotes for strings when possible. > + for f in clang_files: > + file_version = int(re.sub("[^0-9]", "", f)) > + if file_version <= version: > + return f > + > + return None > + > + def relink(): > + supported = ClangFormatLinker.supported() > + if not supported: > + return > + > + if os.path.exists('.clang-format'): > + os.unlink('.clang-format') > + os.symlink(supported, '.clang-format') Could we avoid relinking if the symlink points to the right file already ? > + > + > +class ClangFormatter(Formatter): CLangFormatter became ClangFormatter. Is it intentional ? > patterns = ('*.c', '*.cpp', '*.h') > > + ClangFormatLinker.relink() > + > @classmethod > def format(cls, filename, data): > ret = subprocess.run(['clang-format', '-style=file',
On 27/08/2021 16:43, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Fri, Aug 27, 2021 at 03:47:53PM +0100, Kieran Bingham wrote: >> The .clang-format file can use CaseSensitive on Regex rules for Include >> Categories. With this we can distinguish between QT headers which > > s/QT/Qt/ > >> commence with a 'Q' verses system headers including <queue>. >> >> This requires clang-format-12 to support that rule, and clang-format is >> not very amicable to .clang-format files with unsupported entries, and >> will fail to run on previous versions if we add that option directly. >> >> This causes the checkstyle utility to become unusable for users of >> clang-format versions prior to 12. >> >> Unfortunately, clang-format does not provide a way to specify a specific >> file to use for the rules to apply when formatting, so we can not easily >> call clang-format with a file specific to its versioned needs. >> >> Implement a means of identifying the version of clang-format, and use >> that information to search for the most recent supportable >> .clang-format-$(version) file, which can then be symlinked to the fixed >> file location used by clang format (.clang-format). >> >> .clang-format-12: is added with CaseSensitive:true for QT headers > > Ditto. > >> .clang-format-7: is moved from the current .clang-format implementation >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> --- >> Note there are few things I dislike about this, hence RFC: >> >> - We leave our tree without a .clang-format until checkstyle runs.. > > A bit annoying, but I think we can live with that. Maybe it's a good way > to encourage usage of checkstyle.py :-) > >> - We run the unlinking and symlinking on every invocation of checkstyle > > Shouldn't be too costly :-) It's slightly annoying that we can't run > checkstyle.py on a read-only directory anymore though, but I don't think > it's a common use case. > >> - The 'autogenerated' file in place of a git tracked one causes >> issues on rebase. Could be an issue for bisects...? > > Possibly, but I don't think we'll often bisect and run checkstyle.py at > the same time. > >> >> .clang-format-12 | 169 +++++++++++++++++++++++++++++++ >> .clang-format => .clang-format-7 | 0 >> utils/checkstyle.py | 49 ++++++++- > > The symlink should be added to .gitignore. > >> 3 files changed, 217 insertions(+), 1 deletion(-) >> create mode 100644 .clang-format-12 >> rename .clang-format => .clang-format-7 (100%) >> >> diff --git a/.clang-format-12 b/.clang-format-12 >> new file mode 100644 >> index 000000000000..98e9fad472fc >> --- /dev/null >> +++ b/.clang-format-12 >> @@ -0,0 +1,169 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +# >> +# clang-format configuration file. Intended for clang-format >= 12. >> +# >> +# For more information, see: >> +# >> +# Documentation/process/clang-format.rst >> +# https://clang.llvm.org/docs/ClangFormat.html >> +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html >> +# >> +--- >> +Language: Cpp >> +AccessModifierOffset: -8 >> +AlignAfterOpenBracket: Align >> +AlignConsecutiveAssignments: false >> +AlignConsecutiveDeclarations: false >> +AlignEscapedNewlines: Right >> +AlignOperands: true >> +AlignTrailingComments: false >> +AllowAllParametersOfDeclarationOnNextLine: false >> +AllowShortBlocksOnASingleLine: false >> +AllowShortCaseLabelsOnASingleLine: false >> +AllowShortFunctionsOnASingleLine: InlineOnly >> +AllowShortIfStatementsOnASingleLine: false >> +AllowShortLoopsOnASingleLine: false >> +AlwaysBreakAfterDefinitionReturnType: None >> +AlwaysBreakAfterReturnType: None >> +AlwaysBreakBeforeMultilineStrings: false >> +AlwaysBreakTemplateDeclarations: MultiLine >> +BinPackArguments: true >> +BinPackParameters: true >> +BraceWrapping: >> + AfterClass: true >> + AfterControlStatement: false >> + AfterEnum: false >> + AfterFunction: true >> + AfterNamespace: false >> + AfterObjCDeclaration: false >> + AfterStruct: false >> + AfterUnion: false >> + AfterExternBlock: false >> + BeforeCatch: false >> + BeforeElse: false >> + IndentBraces: false >> + SplitEmptyFunction: true >> + SplitEmptyRecord: true >> + SplitEmptyNamespace: true >> +BreakBeforeBinaryOperators: None >> +BreakBeforeBraces: Custom >> +BreakBeforeInheritanceComma: false >> +BreakInheritanceList: BeforeColon >> +BreakBeforeTernaryOperators: true >> +BreakConstructorInitializers: BeforeColon >> +BreakAfterJavaFieldAnnotations: false >> +BreakStringLiterals: false >> +ColumnLimit: 0 >> +CommentPragmas: '^ IWYU pragma:' >> +CompactNamespaces: false >> +ConstructorInitializerAllOnOneLineOrOnePerLine: false >> +ConstructorInitializerIndentWidth: 8 >> +ContinuationIndentWidth: 8 >> +Cpp11BracedListStyle: false >> +DerivePointerAlignment: false >> +DisableFormat: false >> +ExperimentalAutoDetectBinPacking: false >> +FixNamespaceComments: true >> +ForEachMacros: >> + - 'udev_list_entry_foreach' >> +SortIncludes: true >> +IncludeBlocks: Regroup >> +IncludeCategories: >> + # Headers matching the name of the component are matched automatically. >> + # Priority 1 >> + # Other library headers (explicit overrides to match before system headers) >> + - Regex: '(<jpeglib.h>|<libudev.h>|<tiffio.h>|<xf86drm.h>|<xf86drmMode.h>|<yaml.h>)' >> + Priority: 9 >> + # Qt includes (match before C++ standard library) >> + - Regex: '<Q([A-Za-z0-9\-_])+>' >> + Priority: 9 >> + CaseSensitive: true >> + # Headers in <> with an extension. (+system libraries) >> + - Regex: '<([A-Za-z0-9\-_])+\.h>' >> + Priority: 2 >> + # System headers >> + - Regex: '<sys/.*>' >> + Priority: 2 >> + # C++ standard library includes (no extension) >> + - Regex: '<([A-Za-z0-9\-_/])+>' >> + Priority: 2 >> + # Linux headers, as a second group/subset of system headers >> + - Regex: '<linux/.*>' >> + Priority: 3 >> + # Headers for libcamera Base support >> + - Regex: '<libcamera/base/private.h>' >> + Priority: 4 >> + - Regex: '<libcamera/base/.*\.h>' >> + Priority: 5 >> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/) >> + - Regex: '<libcamera/([A-Za-z0-9\-_])+.h>' >> + Priority: 6 >> + # IPA Interfaces >> + - Regex: '<libcamera/ipa/.*\.h>' >> + Priority: 7 >> + # libcamera Internal headers in "" >> + - Regex: '"libcamera/internal/.*\.h"' >> + Priority: 8 >> + # Other libraries headers with one group per library (.h or .hpp) >> + - Regex: '<.*/.*\.hp*>' >> + Priority: 9 >> + # local modular includes "path/file.h" (.h or .hpp) >> + - Regex: '"(.*/)+.*\.hp*"' >> + Priority: 10 >> + # Other local headers "file.h" with extension (.h or .hpp) >> + - Regex: '".*.hp*"' >> + Priority: 11 >> + # Any unmatched line, separated from the last group >> + - Regex: '"*"' >> + Priority: 100 >> + >> +IncludeIsMainRegex: '(_test)?$' >> +IndentCaseLabels: false >> +IndentPPDirectives: None >> +IndentWidth: 8 >> +IndentWrappedFunctionNames: false >> +JavaScriptQuotes: Leave >> +JavaScriptWrapImports: true >> +KeepEmptyLinesAtTheStartOfBlocks: false >> +MacroBlockBegin: '' >> +MacroBlockEnd: '' >> +MaxEmptyLinesToKeep: 1 >> +NamespaceIndentation: None >> +ObjCBinPackProtocolList: Auto >> +ObjCBlockIndentWidth: 8 >> +ObjCSpaceAfterProperty: true >> +ObjCSpaceBeforeProtocolList: true >> + >> +# Taken from git's rules >> +PenaltyBreakAssignment: 10 >> +PenaltyBreakBeforeFirstCallParameter: 30 >> +PenaltyBreakComment: 10 >> +PenaltyBreakFirstLessLess: 0 >> +PenaltyBreakString: 10 >> +PenaltyBreakTemplateDeclaration: 10 >> +PenaltyExcessCharacter: 100 >> +PenaltyReturnTypeOnItsOwnLine: 60 >> + >> +PointerAlignment: Right >> +ReflowComments: false >> +SortIncludes: true >> +SortUsingDeclarations: true >> +SpaceAfterCStyleCast: false >> +SpaceAfterTemplateKeyword: false >> +SpaceBeforeAssignmentOperators: true >> +SpaceBeforeCpp11BracedList: false >> +SpaceBeforeCtorInitializerColon: true >> +SpaceBeforeInheritanceColon: true >> +SpaceBeforeParens: ControlStatements >> +SpaceBeforeRangeBasedForLoopColon: true >> +SpaceInEmptyParentheses: false >> +SpacesBeforeTrailingComments: 1 >> +SpacesInAngles: false >> +SpacesInContainerLiterals: false >> +SpacesInCStyleCastParentheses: false >> +SpacesInParentheses: false >> +SpacesInSquareBrackets: false >> +Standard: Cpp11 >> +TabWidth: 8 >> +UseTab: Always >> +... > > I trust you on this. > >> diff --git a/.clang-format b/.clang-format-7 >> similarity index 100% >> rename from .clang-format >> rename to .clang-format-7 >> diff --git a/utils/checkstyle.py b/utils/checkstyle.py >> index ececb46eaacc..279ace8346b4 100755 >> --- a/utils/checkstyle.py >> +++ b/utils/checkstyle.py >> @@ -16,6 +16,7 @@ >> import argparse >> import difflib >> import fnmatch >> +import glob >> import os.path >> import re >> import shutil >> @@ -566,9 +567,55 @@ class Formatter(metaclass=ClassRegistry): >> return patterns >> >> >> -class CLangFormatter(Formatter): >> +# Identify the version of clang-format running, and match it against locally >> +# available versioned configuration files of the form: >> +# .clang-format-$(version) >> +# >> +# The highest supported local configuration file is linked as .clang-format by >> +# ClangFormatLinker.relink() for use by the ClangFormatter. >> +# >> +class ClangFormatLinker(): > > No need for parentheses if you don't inherit from another class, but you > can inherit from object. > > Do we need ClangFormatLinker though ? Couldn't those functions be move > to ClangFormatter ? Although relinking should probably be done from > main(), see below. Probably not, I was just trying to group this code together really, and wanted to break out into functions a little to make it clearer. > >> + def version(): >> + version_regex = re.compile('[^0-9]*([0-9]+).*') >> + ret = subprocess.run(['clang-format', '--version'], stdout=subprocess.PIPE) > > What happens if clang-format isn't available ? We check for it in > main(), but I think this code will run before that. > >> + version = ret.stdout.decode('utf-8') >> + search = version_regex.search(version) >> + if search: >> + version = search.group(1) >> + return int(version) >> + >> + # Lowest supported version >> + return 7 > > Shouldn't this cause an error instead ? Probably - we expect there to be a clang-format, and if we can't interrogate it - then it's an issue. > >> + >> + def supported(): >> + version = ClangFormatLinker.version() > > That's a bit of an abuse of the Python class system. Instance methods > are supposed to be called on an instance and take a self argument. These > three functions should be declared as class method I think. This would > become > > @classmethod > def supported(cls): > version = cls.version() > >> + >> + # Match the highest supported version >> + clang_files = glob.glob('.clang-format-[0-9]*') > > When checkstyle.py is run manually, it may be run from a subdirectory of > the git tree. You should pass the git top-level directory to relink(). > This would then call for calling relink() directly from main() instead > of ClangFormatter. > >> + clang_files = sorted(clang_files, reverse=True, >> + key=lambda s: [int(re.sub("[^0-9]", "", s))]) > > We use single quotes for strings when possible. Ok > >> + for f in clang_files: >> + file_version = int(re.sub("[^0-9]", "", f)) >> + if file_version <= version: >> + return f >> + >> + return None >> + >> + def relink(): >> + supported = ClangFormatLinker.supported() >> + if not supported: >> + return >> + >> + if os.path.exists('.clang-format'): >> + os.unlink('.clang-format') >> + os.symlink(supported, '.clang-format') > > Could we avoid relinking if the symlink points to the right file already > ? Probably, - this was just the quick and dirty RFC (where I forgot to add --rfc to git-format-patch :D) Presumably os.readlink() will give us something we can work with ... >> + >> + >> +class ClangFormatter(Formatter): > > CLangFormatter became ClangFormatter. Is it intentional ? Yes, but it was supposed to be in a separate patch. I don't think I've ever seen anyone really call it "C Lang" rather than "Clang" > >> patterns = ('*.c', '*.cpp', '*.h') >> >> + ClangFormatLinker.relink() >> + >> @classmethod >> def format(cls, filename, data): >> ret = subprocess.run(['clang-format', '-style=file', >
diff --git a/.clang-format-12 b/.clang-format-12 new file mode 100644 index 000000000000..98e9fad472fc --- /dev/null +++ b/.clang-format-12 @@ -0,0 +1,169 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# clang-format configuration file. Intended for clang-format >= 12. +# +# For more information, see: +# +# Documentation/process/clang-format.rst +# https://clang.llvm.org/docs/ClangFormat.html +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html +# +--- +Language: Cpp +AccessModifierOffset: -8 +AlignAfterOpenBracket: Align +AlignConsecutiveAssignments: false +AlignConsecutiveDeclarations: false +AlignEscapedNewlines: Right +AlignOperands: true +AlignTrailingComments: false +AllowAllParametersOfDeclarationOnNextLine: false +AllowShortBlocksOnASingleLine: false +AllowShortCaseLabelsOnASingleLine: false +AllowShortFunctionsOnASingleLine: InlineOnly +AllowShortIfStatementsOnASingleLine: false +AllowShortLoopsOnASingleLine: false +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakAfterReturnType: None +AlwaysBreakBeforeMultilineStrings: false +AlwaysBreakTemplateDeclarations: MultiLine +BinPackArguments: true +BinPackParameters: true +BraceWrapping: + AfterClass: true + AfterControlStatement: false + AfterEnum: false + AfterFunction: true + AfterNamespace: false + AfterObjCDeclaration: false + AfterStruct: false + AfterUnion: false + AfterExternBlock: false + BeforeCatch: false + BeforeElse: false + IndentBraces: false + SplitEmptyFunction: true + SplitEmptyRecord: true + SplitEmptyNamespace: true +BreakBeforeBinaryOperators: None +BreakBeforeBraces: Custom +BreakBeforeInheritanceComma: false +BreakInheritanceList: BeforeColon +BreakBeforeTernaryOperators: true +BreakConstructorInitializers: BeforeColon +BreakAfterJavaFieldAnnotations: false +BreakStringLiterals: false +ColumnLimit: 0 +CommentPragmas: '^ IWYU pragma:' +CompactNamespaces: false +ConstructorInitializerAllOnOneLineOrOnePerLine: false +ConstructorInitializerIndentWidth: 8 +ContinuationIndentWidth: 8 +Cpp11BracedListStyle: false +DerivePointerAlignment: false +DisableFormat: false +ExperimentalAutoDetectBinPacking: false +FixNamespaceComments: true +ForEachMacros: + - 'udev_list_entry_foreach' +SortIncludes: true +IncludeBlocks: Regroup +IncludeCategories: + # Headers matching the name of the component are matched automatically. + # Priority 1 + # Other library headers (explicit overrides to match before system headers) + - Regex: '(<jpeglib.h>|<libudev.h>|<tiffio.h>|<xf86drm.h>|<xf86drmMode.h>|<yaml.h>)' + Priority: 9 + # Qt includes (match before C++ standard library) + - Regex: '<Q([A-Za-z0-9\-_])+>' + Priority: 9 + CaseSensitive: true + # Headers in <> with an extension. (+system libraries) + - Regex: '<([A-Za-z0-9\-_])+\.h>' + Priority: 2 + # System headers + - Regex: '<sys/.*>' + Priority: 2 + # C++ standard library includes (no extension) + - Regex: '<([A-Za-z0-9\-_/])+>' + Priority: 2 + # Linux headers, as a second group/subset of system headers + - Regex: '<linux/.*>' + Priority: 3 + # Headers for libcamera Base support + - Regex: '<libcamera/base/private.h>' + Priority: 4 + - Regex: '<libcamera/base/.*\.h>' + Priority: 5 + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/) + - Regex: '<libcamera/([A-Za-z0-9\-_])+.h>' + Priority: 6 + # IPA Interfaces + - Regex: '<libcamera/ipa/.*\.h>' + Priority: 7 + # libcamera Internal headers in "" + - Regex: '"libcamera/internal/.*\.h"' + Priority: 8 + # Other libraries headers with one group per library (.h or .hpp) + - Regex: '<.*/.*\.hp*>' + Priority: 9 + # local modular includes "path/file.h" (.h or .hpp) + - Regex: '"(.*/)+.*\.hp*"' + Priority: 10 + # Other local headers "file.h" with extension (.h or .hpp) + - Regex: '".*.hp*"' + Priority: 11 + # Any unmatched line, separated from the last group + - Regex: '"*"' + Priority: 100 + +IncludeIsMainRegex: '(_test)?$' +IndentCaseLabels: false +IndentPPDirectives: None +IndentWidth: 8 +IndentWrappedFunctionNames: false +JavaScriptQuotes: Leave +JavaScriptWrapImports: true +KeepEmptyLinesAtTheStartOfBlocks: false +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 1 +NamespaceIndentation: None +ObjCBinPackProtocolList: Auto +ObjCBlockIndentWidth: 8 +ObjCSpaceAfterProperty: true +ObjCSpaceBeforeProtocolList: true + +# Taken from git's rules +PenaltyBreakAssignment: 10 +PenaltyBreakBeforeFirstCallParameter: 30 +PenaltyBreakComment: 10 +PenaltyBreakFirstLessLess: 0 +PenaltyBreakString: 10 +PenaltyBreakTemplateDeclaration: 10 +PenaltyExcessCharacter: 100 +PenaltyReturnTypeOnItsOwnLine: 60 + +PointerAlignment: Right +ReflowComments: false +SortIncludes: true +SortUsingDeclarations: true +SpaceAfterCStyleCast: false +SpaceAfterTemplateKeyword: false +SpaceBeforeAssignmentOperators: true +SpaceBeforeCpp11BracedList: false +SpaceBeforeCtorInitializerColon: true +SpaceBeforeInheritanceColon: true +SpaceBeforeParens: ControlStatements +SpaceBeforeRangeBasedForLoopColon: true +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 1 +SpacesInAngles: false +SpacesInContainerLiterals: false +SpacesInCStyleCastParentheses: false +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: Cpp11 +TabWidth: 8 +UseTab: Always +... diff --git a/.clang-format b/.clang-format-7 similarity index 100% rename from .clang-format rename to .clang-format-7 diff --git a/utils/checkstyle.py b/utils/checkstyle.py index ececb46eaacc..279ace8346b4 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -16,6 +16,7 @@ import argparse import difflib import fnmatch +import glob import os.path import re import shutil @@ -566,9 +567,55 @@ class Formatter(metaclass=ClassRegistry): return patterns -class CLangFormatter(Formatter): +# Identify the version of clang-format running, and match it against locally +# available versioned configuration files of the form: +# .clang-format-$(version) +# +# The highest supported local configuration file is linked as .clang-format by +# ClangFormatLinker.relink() for use by the ClangFormatter. +# +class ClangFormatLinker(): + def version(): + version_regex = re.compile('[^0-9]*([0-9]+).*') + ret = subprocess.run(['clang-format', '--version'], stdout=subprocess.PIPE) + version = ret.stdout.decode('utf-8') + search = version_regex.search(version) + if search: + version = search.group(1) + return int(version) + + # Lowest supported version + return 7 + + def supported(): + version = ClangFormatLinker.version() + + # Match the highest supported version + clang_files = glob.glob('.clang-format-[0-9]*') + clang_files = sorted(clang_files, reverse=True, + key=lambda s: [int(re.sub("[^0-9]", "", s))]) + for f in clang_files: + file_version = int(re.sub("[^0-9]", "", f)) + if file_version <= version: + return f + + return None + + def relink(): + supported = ClangFormatLinker.supported() + if not supported: + return + + if os.path.exists('.clang-format'): + os.unlink('.clang-format') + os.symlink(supported, '.clang-format') + + +class ClangFormatter(Formatter): patterns = ('*.c', '*.cpp', '*.h') + ClangFormatLinker.relink() + @classmethod def format(cls, filename, data): ret = subprocess.run(['clang-format', '-style=file',
The .clang-format file can use CaseSensitive on Regex rules for Include Categories. With this we can distinguish between QT headers which commence with a 'Q' verses system headers including <queue>. This requires clang-format-12 to support that rule, and clang-format is not very amicable to .clang-format files with unsupported entries, and will fail to run on previous versions if we add that option directly. This causes the checkstyle utility to become unusable for users of clang-format versions prior to 12. Unfortunately, clang-format does not provide a way to specify a specific file to use for the rules to apply when formatting, so we can not easily call clang-format with a file specific to its versioned needs. Implement a means of identifying the version of clang-format, and use that information to search for the most recent supportable .clang-format-$(version) file, which can then be symlinked to the fixed file location used by clang format (.clang-format). .clang-format-12: is added with CaseSensitive:true for QT headers .clang-format-7: is moved from the current .clang-format implementation Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Note there are few things I dislike about this, hence RFC: - We leave our tree without a .clang-format until checkstyle runs.. - We run the unlinking and symlinking on every invocation of checkstyle - The 'autogenerated' file in place of a git tracked one causes issues on rebase. Could be an issue for bisects...? .clang-format-12 | 169 +++++++++++++++++++++++++++++++ .clang-format => .clang-format-7 | 0 utils/checkstyle.py | 49 ++++++++- 3 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 .clang-format-12 rename .clang-format => .clang-format-7 (100%)