scripts: build: ruff compliance
#99719
Conversation
Apply `ruff` formatting rules to all scripts in the build folder. Signed-off-by: Jordan Yates <jordan@embeint.com>
zephyrbot
added
area: Userspace
zephyrbot
requested review from
57300,
ceolin,
dcpleung,
jeremybettis,
kartben,
keith-zephyr,
nashif,
nordicjm,
pdgendt,
tejlmand and
ycsin
JordanYates
force-pushed
the
251120_ruff_build
branch
from
c735776 to
1ebfa53
Compare
|
|
||
| # This is needed to load edt.pickle files. | ||
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "dts", "python-devicetree", "src")) | ||
| from devicetree import edtlib # pylint: disable=unused-import |
There was a problem hiding this comment.
The warning was disabled for pylint, maybe this is still needed? CC @fabiobaltieri
There was a problem hiding this comment.
Missed this, I'm pretty sure it is needed, added it back
JordanYates
force-pushed
the
251120_ruff_build
branch
2 times, most recently
from
211db4a to
0a61c64
Compare
Update scripts in the build folder to pass all enabled `ruff` checks. Signed-off-by: Jordan Yates <jordan@embeint.com>
JordanYates
force-pushed
the
251120_ruff_build
branch
from
0a61c64 to
694e02e
Compare
|
There was a problem hiding this comment.
Pull Request Overview
This pull request updates Python scripts in the scripts/build folder to comply with ruff linting rules. The changes are done in two phases: first applying ruff format for automatic formatting, then manually addressing additional rule violations.
Key Changes
- Reformatted imports to follow standard ordering (stdlib, third-party, local)
- Converted old-style string formatting (
%operator) to f-strings - Updated
open()calls to remove redundant'r'mode parameter - Replaced
assert Falsestatements with properraisestatements - Applied consistent code formatting and spacing throughout
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/build/uf2conv.py | Sorted imports, converted to f-strings, replaced assert with raise, improved boolean logic |
| scripts/build/subfolder_list.py | Sorted imports, removed redundant open mode parameter |
| scripts/build/process_gperf.py | Sorted imports, converted to f-strings, simplified string concatenation |
| scripts/build/parse_syscalls.py | Sorted imports, converted to f-strings, improved exception handling |
| scripts/build/mergehex.py | Sorted imports, converted to f-strings, added exception chaining |
| scripts/build/llext_prepare_exptab.py | Sorted imports, converted to f-strings, improved context manager usage |
| scripts/build/llext_inject_slids.py | Sorted imports, converted to f-strings, used enumerate for loops |
| scripts/build/gen_syscalls.py | Sorted imports, converted to f-strings, reformatted multi-line arguments |
| scripts/build/gen_symtab.py | Sorted imports, converted to f-strings, removed unused variables |
| scripts/build/gen_strsignal_table.py | Simplified directory creation with exist_ok parameter |
| scripts/build/gen_strerror_table.py | Simplified directory creation with exist_ok parameter |
| scripts/build/gen_relocate_app.py | Added noqa comments for intentional mixed indentation in templates |
| scripts/build/gen_offset_header.py | Sorted imports, converted to f-strings |
| scripts/build/gen_kobject_placeholders.py | Sorted imports, improved context manager usage |
| scripts/build/gen_kobject_list.py | Sorted imports, converted to f-strings, improved formatting |
| scripts/build/gen_isr_tables_parser_local.py | Converted to f-strings, improved formatting |
| scripts/build/gen_isr_tables_parser_carrays.py | Converted to f-strings, improved formatting |
| scripts/build/gen_isr_tables.py | Sorted imports, converted to f-strings, improved formatting |
| scripts/build/gen_image_info.py | Sorted imports, added noqa comment for open usage |
| scripts/build/gen_device_deps.py | Sorted imports, converted to f-strings |
| scripts/build/gen_cfb_font_header.py | Sorted imports, converted to f-strings, reformatted arguments |
| scripts/build/gen_app_partitions.py | Sorted imports, converted to f-strings, improved formatting |
| scripts/build/file2hex.py | Reformatted multi-line arguments, added strict parameter to zip |
| scripts/build/elf_parser.py | Sorted imports, added noqa comment for open usage |
| scripts/build/check_init_priorities_test.py | Sorted imports, improved formatting |
| scripts/build/check_init_priorities.py | Sorted imports, converted to f-strings, improved formatting |
| .ruff-excludes.toml | Removed exclusions for build scripts now that they comply with ruff |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func_type, func_name = typename_split(func) | ||
| except SyscallParseException: | ||
| sys.stderr.write("In declaration of %s\n" % func) | ||
| sys.stderr.write("In declaration of {func}\n") |
There was a problem hiding this comment.
The f-string is missing the f prefix. The interpolation {func} won't work without it.
This should be:
sys.stderr.write(f"In declaration of {func}\n")| sys.stderr.write("In declaration of {func}\n") | |
| sys.stderr.write(f"In declaration of {func}\n") |
| f"multiple registrations at table_index {table_index} for irq {irq} " | ||
| + " (0x{irq:x})" | ||
| + f"\nExisting handler 0x{swt[table_index][0][1]:x}, " | ||
| + "new handler 0x{func:x}" | ||
| + "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | ||
| + "on the same irq multiple times?" |
There was a problem hiding this comment.
The f-string is missing the {irq:x} placeholder. Line 180 shows " (0x{irq:x})" but this needs to be part of an f-string expression.
The concatenation should be:
f"multiple registrations at table_index {table_index} for irq {irq} (0x{irq:x})"| f"multiple registrations at table_index {table_index} for irq {irq} " | |
| + " (0x{irq:x})" | |
| + f"\nExisting handler 0x{swt[table_index][0][1]:x}, " | |
| + "new handler 0x{func:x}" | |
| + "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | |
| + "on the same irq multiple times?" | |
| f"multiple registrations at table_index {table_index} for irq {irq} (0x{irq:x})" | |
| f"\nExisting handler 0x{swt[table_index][0][1]:x}, new handler 0x{func:x}" | |
| "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | |
| "on the same irq multiple times?" |
| f"multiple registrations at table_index {table_index} for irq {irq} " | ||
| + "(0x{irq:x})" | ||
| + f"\nExisting section {swt[table_index]}, new section {sname}" | ||
| + "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | ||
| + "on the same irq multiple times?" |
There was a problem hiding this comment.
The f-string is incomplete. Line 225 shows + "(0x{irq:x})" but this is concatenated as a regular string, not an f-string.
This should be:
f"multiple registrations at table_index {table_index} for irq {irq} (0x{irq:x})"| f"multiple registrations at table_index {table_index} for irq {irq} " | |
| + "(0x{irq:x})" | |
| + f"\nExisting section {swt[table_index]}, new section {sname}" | |
| + "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | |
| + "on the same irq multiple times?" | |
| f"multiple registrations at table_index {table_index} for irq {irq} (0x{irq:x})" | |
| f"\nExisting section {swt[table_index]}, new section {sname}" | |
| "\nHas IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked " | |
| "on the same irq multiple times?" |
| print('"', end='') | ||
| remainder = args.length | ||
| for chunk in iter(lambda: fp.read(min(1024, remainder)), b''): | ||
| for chunk in iter(lambda rem=remainder: fp.read(min(1024, rem)), b''): |
There was a problem hiding this comment.
The lambda function captures remainder by value at definition time, but the intent is to use the updated value of remainder in each iteration. This lambda should use the current value of remainder from the enclosing scope, not capture it.
This should be:
for chunk in iter(lambda: fp.read(min(1024, remainder)), b''):The variable rem=remainder in the lambda parameters creates a default argument that captures the initial value, which won't update as remainder changes in the loop.
| else: | ||
| remainder = args.length | ||
| for chunk in iter(lambda: fp.read(min(1024, remainder)), b''): | ||
| for chunk in iter(lambda rem=remainder: fp.read(min(1024, rem)), b''): |
There was a problem hiding this comment.
Same issue as line 121. The lambda function captures remainder by value at definition time, but the intent is to use the updated value of remainder in each iteration.
This should be:
for chunk in iter(lambda: fp.read(min(1024, remainder)), b''):| "dts", "python-devicetree", "src")) | ||
| from devicetree import edtlib # pylint: disable=unused-import | ||
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "dts", "python-devicetree", "src")) | ||
| from devicetree import edtlib # noqa: F401 |
There was a problem hiding this comment.
Import of 'edtlib' is not used.
|
|
||
| def __init__(self, kernel, edt, device_start_symbol): | ||
| self.elf = ELFFile(open(kernel, "rb")) | ||
| self.elf = ELFFile(open(kernel, "rb")) # noqa : SIM115 |
There was a problem hiding this comment.
File is opened but is not closed.
|
|
||
| def read_segments(filename): | ||
| elffile = ELFFile(open(filename, 'rb')) | ||
| elffile = ELFFile(open(filename, 'rb')) # noqa: SIM115 |
There was a problem hiding this comment.
File is opened but is not closed.
| if args.gperf_output: | ||
| assert args.kernel, "--kernel ELF required for --gperf-output" | ||
| elf = ELFFile(open(args.kernel, "rb")) | ||
| elf = ELFFile(open(args.kernel, "rb")) # noqa: SIM115 |
There was a problem hiding this comment.
File is opened but is not closed.
4 participants
Update scripts in the build folder to pass all enabled
ruffchecks (Except the line length checks ingen_app_partitions.py).Done in two phases, the first just applies
ruff formatto the folder, the second does the manual rule updates (that weren't done automatically byruff check --fix).