Skip to content

Move clang formating away from zap regen all #41543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+17 −18
Open

Move clang formating away from zap regen all #41543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f05500d
modify clang format logic
dmaranges-gd Oct 9, 2025
c50af9a
Modify logic to fix error
dmaranges-gd Oct 14, 2025
8ce2f32
Update scripts/codegen.py gemini recomendation
dmaranges-gd Oct 20, 2025
8f31d75
Merge branch 'master' into 38774_move_formating_away_from_zap_regen_all
andy31415 Oct 21, 2025
49813b3
Merge branch 'master' into 38774_move_formating_away_from_zap_regen_all
dmaranges-gd Oct 22, 2025
adfe527
Revert "Update scripts/codegen.py gemini recomendation"
dmaranges-gd Oct 22, 2025
2cdd0a2
Restyle code
dmaranges-gd Oct 22, 2025
b4be92f
Restyle code
dmaranges-gd Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions scripts/codegen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@
from matter.idl.matter_idl_parser import CreateParser

import logging
import os.path
import subprocess
import sys
import traceback

import click
from tools.zap.clang_format import getClangFormatBinary

try:
import coloredlogs
Expand Down Expand Up @@ -170,6 +174,19 @@ def main(log_level, generator, option, output_dir, dry_run, name_only, expected_

sys.exit(1)

cpp_files = []
for file in storage.generated_paths:
_, ext = os.path.splitext(file)
if ext in ['.h', '.cpp', '.c', '.hpp']:
cpp_files.append(file)
if cpp_files:
try:
logging.debug("Formatting files: %s", cpp_files)

subprocess.check_call([getClangFormatBinary(), "-i"] + cpp_files)
except Exception:
traceback.print_exc()
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this! This will really help me out when done.

When I do codegen on top of this PR locally, I get:

Traceback (most recent call last):
  File "/Users/bzbarsky/connectedhomeip/./scripts/codegen.py", line 187, in main
    subprocess.check_call([getClangFormatBinary(), "-i"] + cpp_files)
  File "/Users/bzbarsky/connectedhomeip/.environment/cipd/packages/pigweed/lib/python3.11/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/Users/bzbarsky/connectedhomeip/.environment/cipd/packages/pigweed/bin/clang-format', '-i', ... long list of paths ... ]' returned non-zero exit status 1.

but then codegen claims to succeed even though it failed, because this code is swallowing the exception (which admittedly the old code did too).

The error messages before that look like a long list of:

RelativeHumidityMeasurement/MetadataProvider.h: No such file or directory

and such, so it looks like this is passing in relative paths that are not relative to the cwd?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review!! I missed the join method to select the right path, leading to an error related to the paths

I'm working on fixing it, and when I finish it I will request a new review


logging.info("Done")


Expand Down
18 changes: 0 additions & 18 deletions scripts/tools/zap_regen_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
from pathlib import Path
from typing import List

from zap.clang_format import getClangFormatBinary

CHIP_ROOT_DIR = os.path.realpath(
os.path.join(os.path.dirname(__file__), '../..'))

Expand Down Expand Up @@ -303,16 +301,6 @@ def formatKotlinFiles(self, paths):
traceback.print_exc()
raise

def formatWithClangFormat(self, paths):
try:
logging.info("Formatting %d cpp files:", len(paths))
for name in paths:
logging.info(" %s" % name)

subprocess.check_call([getClangFormatBinary(), "-i"] + paths)
except Exception:
traceback.print_exc()

def codeFormat(self):
outputs = subprocess.check_output(["./scripts/codegen.py", "--name-only", "--generator",
self.generator, "--log-level", "fatal", self.idl_path]).decode("utf8").split("\n")
Expand All @@ -327,12 +315,6 @@ def codeFormat(self):
if '.kt' in name_dict:
self.formatKotlinFiles(name_dict['.kt'])

cpp_files = []
for ext in ['.h', '.cpp', '.c', '.hpp']:
cpp_files.extend(name_dict.get(ext, []))
if cpp_files:
self.formatWithClangFormat(cpp_files)

def generate(self) -> TargetRunStats:
generate_start = time.time()

Expand Down
Loading