Skip to content

Commit

Permalink
Restrict clang-tidy
Browse files Browse the repository at this point in the history
  • Loading branch information
andreilitvin committed Oct 8, 2024
1 parent 7c68210 commit 73fd3cb
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 25 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,20 @@ jobs:
- name: Ensure codegen is done for sanitize
run: |
./scripts/run_in_build_env.sh "./scripts/run_codegen_targets.sh out/sanitizers"
- name: Find changed files
id: changed-files
uses: tj-actions/changed-files@v34
- name: Clang-tidy validation
# NOTE: clang-tidy crashes on CodegenDataModel_Write due to Nullable/std::optional check.
# See https://github.com/llvm/llvm-project/issues/97426
run: |
echo "${{ steps.changed-files.outputs.all}}" >out/changed_files.txt
./scripts/run_in_build_env.sh \
"./scripts/run-clang-tidy-on-compile-commands.py \
--compile-database out/sanitizers/compile_commands.json \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write|QuieterReporting' \
--file-list-file out/changed_files.txt
check \
"
- name: Clean output
Expand Down Expand Up @@ -425,14 +431,20 @@ jobs:
- name: Ensure codegen is done for sanitize
run: |
./scripts/run_in_build_env.sh "./scripts/run_codegen_targets.sh out/default"
- name: Find changed files
id: changed-files
uses: tj-actions/changed-files@v34
- name: Clang-tidy validation
# NOTE: clang-tidy crashes on CodegenDataModel_Write due to Nullable/std::optional check.
# See https://github.com/llvm/llvm-project/issues/97426
run: |
echo "${{ steps.changed-files.outputs.all}}" >out/changed_files.txt
./scripts/run_in_build_env.sh \
"./scripts/run-clang-tidy-on-compile-commands.py \
--compile-database out/default/compile_commands.json \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write|QuieterReporting' \
--file-list-file out/changed_files.txt
check \
"
- name: Uploading diagnostic logs
Expand Down
70 changes: 45 additions & 25 deletions scripts/run-clang-tidy-on-compile-commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,19 @@ def __init__(self, json_entry, gcc_sysroot=None):
#
# However that seems to potentially disable a lot, so for now just filter out the
# offending argument
command_items = [arg for arg in command_items if arg not in {'-c', '-S'}]
command_items = [arg for arg in command_items if arg not in {"-c", "-S"}]

# Allow gcc/g++ invocations to also be tidied - arguments should be
# compatible and on darwin gcc/g++ is actually a symlink to clang
if compiler in ['clang++', 'clang', 'gcc', 'g++']:
if compiler in ["clang++", "clang", "gcc", "g++"]:
self.valid = True
self.clang_arguments = command_items[1:]
else:
logging.warning(
"Cannot tidy %s - not a clang compile command", self.file)
logging.warning("Cannot tidy %s - not a clang compile command", self.file)
return

if compiler in ['gcc', 'g++'] and gcc_sysroot:
self.clang_arguments.insert(0, '--sysroot='+gcc_sysroot)
if compiler in ["gcc", "g++"] and gcc_sysroot:
self.clang_arguments.insert(0, "--sysroot=" + gcc_sysroot)

@property
def full_path(self):
Expand All @@ -122,8 +121,12 @@ def SetChecks(self, checks: str):
def Check(self):
logging.debug("Running tidy on %s from %s", self.file, self.directory)
try:
cmd = ["clang-tidy", self.file] + \
self.tidy_arguments + ["--"] + self.clang_arguments
cmd = (
["clang-tidy", self.file]
+ self.tidy_arguments
+ ["--"]
+ self.clang_arguments
)
logging.debug("Executing: %r" % cmd)

proc = subprocess.Popen(
Expand Down Expand Up @@ -156,7 +159,7 @@ def Check(self):
"Use -system-headers to display errors from system headers as well.",
]

for line in err.decode('utf-8').split('\n'):
for line in err.decode("utf-8").split("\n"):
line = line.strip()

if any(map(lambda s: s in line, skip_strings)):
Expand All @@ -165,7 +168,7 @@ def Check(self):
if not line:
continue # no empty lines

logging.warning('TIDY %s: %s', self.file, line)
logging.warning("TIDY %s: %s", self.file, line)

if proc.returncode != 0:
if proc.returncode < 0:
Expand Down Expand Up @@ -203,18 +206,22 @@ def Failure(self, path: str):


def find_darwin_gcc_sysroot():
for line in subprocess.check_output('xcodebuild -sdk -version'.split()).decode('utf8').split('\n'):
if not line.startswith('Path: '):
for line in (
subprocess.check_output("xcodebuild -sdk -version".split())
.decode("utf8")
.split("\n")
):
if not line.startswith("Path: "):
continue
path = line[line.find(': ')+2:]
if '/MacOSX.platform/' not in path:
path = line[line.find(": ") + 2 :]
if "/MacOSX.platform/" not in path:
continue
logging.info("Found %s" % path)
return path

# A hard-coded value that works on default installations
logging.warning("Using default platform sdk path. This may be incorrect.")
return '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk'
return "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk"


class ClangTidyRunner:
Expand All @@ -228,13 +235,12 @@ def __init__(self):
self.gcc_sysroot = None
self.file_names_to_check = set()

if sys.platform == 'darwin':
if sys.platform == "darwin":
# Darwin gcc invocation will auto select a system root, however clang requires an explicit path since
# we are using the built-in pigweed clang-tidy.
logging.info(
'Searching for a MacOS system root for gcc invocations...')
logging.info("Searching for a MacOS system root for gcc invocations...")
self.gcc_sysroot = find_darwin_gcc_sysroot()
logging.info(' Chose: %s' % self.gcc_sysroot)
logging.info(" Chose: %s" % self.gcc_sysroot)

def AddDatabase(self, compile_commands_json):
database = json.load(open(compile_commands_json))
Expand All @@ -245,7 +251,7 @@ def AddDatabase(self, compile_commands_json):
continue

if item.file in self.file_names_to_check:
logging.info('Ignoring additional request for checking %s', item.file)
logging.info("Ignoring additional request for checking %s", item.file)
continue

self.file_names_to_check.add(item.file)
Expand All @@ -270,12 +276,12 @@ def Cleanup(self):
# Allow all diagnostics for distinct paths to be applied
# at once but never again for future paths
for d in diagnostics:
if d['DiagnosticMessage']['FilePath'] not in already_seen:
if d["DiagnosticMessage"]["FilePath"] not in already_seen:
all_diagnostics.append(d)

# in the future assume these files were already processed
for d in diagnostics:
already_seen.add(d['DiagnosticMessage']['FilePath'])
already_seen.add(d["DiagnosticMessage"]["FilePath"])

if all_diagnostics:
with open(self.fixes_file, "w") as out:
Expand Down Expand Up @@ -304,8 +310,7 @@ def ExportFixesTo(self, f):
for idx, e in enumerate(self.entries):
e.ExportFixesTo(
os.path.join(
self.fixes_temporary_file_dir.name, "fixes%d.yaml" % (
idx + 1,)
self.fixes_temporary_file_dir.name, "fixes%d.yaml" % (idx + 1,)
)
)

Expand Down Expand Up @@ -407,6 +412,12 @@ def Check(self):
type=str,
help="Checks to run (passed in to clang-tidy). If not set the .clang-tidy file is used.",
)
@click.option(
"--file-list-file",
default=None,
type=click.Path(exists=True),
help="When provided, only tidy files that match files mentioned in this file.",
)
@click.pass_context
def main(
context,
Expand All @@ -417,6 +428,7 @@ def main(
no_log_timestamps,
export_fixes,
checks,
file_list_file,
):
log_fmt = "%(asctime)s %(levelname)-7s %(message)s"
if no_log_timestamps:
Expand Down Expand Up @@ -452,6 +464,14 @@ def cleanup():
r = re.compile(file_exclude_regex)
runner.FilterEntries(lambda e: not r.search(e.file))

if file_list_file:
acceptable = set()
with open(file_list_file, "rt") as f:
for file_name in f.readlines():
logging.warning("DEBUG: file %s in file", file_name.strip())
acceptable.add(file_name.strip())
runner.FilterEntries(lambda e: e.file in acceptable)

if export_fixes:
runner.ExportFixesTo(export_fixes)

Expand Down Expand Up @@ -492,4 +512,4 @@ def cmd_fix(context):


if __name__ == "__main__":
main(auto_envvar_prefix='CHIP')
main(auto_envvar_prefix="CHIP")

0 comments on commit 73fd3cb

Please sign in to comment.