fix: script for removing unwanted flags from compile_commands.json #1235
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a helper script to strip unsupported GCC flags from compile_commands.json, integrates it into the CMake build, and documents the workflow for ESP-IDF users.
- Introduce
fix_compile_commands.pyto remove-m*and-f*flags. - Add a CMake custom target (
fix_clangd) to run the script post-build. - Extend the RST documentation with setup steps and example links.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| resources/fix_compile_commands/fix_compile_commands.py | New Python script to filter out -m* and -f* flags |
| resources/fix_compile_commands/CMakeLists.txt | Custom CMake target to invoke the cleanup script after build |
| docs/en/additionalfeatures/clangd_cdt_support.rst | Added instructions and example links for running the cleanup tool |
Comments suppressed due to low confidence (3)
resources/fix_compile_commands/fix_compile_commands.py:20
- The remove_mf_flags function currently lacks unit tests. Add tests covering both the 'arguments' array and the 'command' string cases, including edge cases with quoted flags.
def remove_mf_flags(compile_commands):
docs/en/additionalfeatures/clangd_cdt_support.rst:62
- The URL includes a duplicated 'resources/resources' segment. Update the link to point to
resources/fix_compile_commands/fix_compile_commands.py.
Download the script from `here <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/resources/fix_compile_commands/fix_compile_commands.py>`_.
docs/en/additionalfeatures/clangd_cdt_support.rst:64
- The example link path also contains 'resources/resources'. Correct it to
resources/fix_compile_commands/CMakeLists.txt.
Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/resources/fix_compile_commands/CMakeLists.txt>`_.
|
|
||
| def remove_mf_flags(compile_commands): | ||
| for entry in compile_commands: | ||
| args = entry.get("arguments") or entry.get("command", "").split() |
There was a problem hiding this comment.
Use shlex.split on the command string instead of str.split() to correctly handle quoted arguments and paths with spaces.
| if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py") | ||
| add_custom_target( | ||
| fix_clangd ALL | ||
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | ||
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py |
There was a problem hiding this comment.
CMakeLists resides in a subdirectory, so ${CMAKE_SOURCE_DIR} may not resolve to this folder. Use ${CMAKE_CURRENT_SOURCE_DIR} to reference the script path correctly.
| if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py") | |
| add_custom_target( | |
| fix_clangd ALL | |
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | |
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py | |
| if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py") | |
| add_custom_target( | |
| fix_clangd ALL | |
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | |
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
WalkthroughA new documentation section was added to address "Unknown argument" errors in clangd when working with ESP-IDF projects. A Python script and CMake integration were introduced to automatically clean problematic compiler flags from compile_commands.json, ensuring smoother navigation in LSP-based C/C++ editors. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CMake
participant fix_compile_commands.py
participant compile_commands.json
Developer->>CMake: Build project
CMake->>fix_compile_commands.py: Run as custom target after build
fix_compile_commands.py->>compile_commands.json: Locate and read file
fix_compile_commands.py->>compile_commands.json: Remove -m* and -f* flags
fix_compile_commands.py->>compile_commands.json: Overwrite with cleaned content
fix_compile_commands.py-->>Developer: Print confirmation message
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
resources/fix_compile_commands/fix_compile_commands.py (2)
22-22: 🛠️ Refactor suggestionUse shlex.split() for proper command parsing.
Using
str.split()on command strings can incorrectly split quoted arguments and paths containing spaces, leading to malformed compilation commands.Apply this diff to use proper shell-aware parsing:
+import shlex- args = entry.get("arguments") or entry.get("command", "").split() + args = entry.get("arguments") or shlex.split(entry.get("command", ""))
39-46: 🛠️ Refactor suggestionAdd error handling for JSON operations and file I/O.
The script lacks error handling for JSON parsing and file operations, which could cause cryptic failures if the file is corrupted or has permission issues.
Apply this diff to add proper error handling:
- with open(cc_path, "r") as f: - compile_commands = json.load(f) - - cleaned_commands = remove_mf_flags(compile_commands) - - with open(cc_path, "w") as f: - json.dump(cleaned_commands, f, indent=2) + try: + with open(cc_path, "r") as f: + compile_commands = json.load(f) + + if not isinstance(compile_commands, list): + print(f"Error: Expected list in {cc_path}, got {type(compile_commands)}") + return + + cleaned_commands = remove_mf_flags(compile_commands) + + with open(cc_path, "w") as f: + json.dump(cleaned_commands, f, indent=2) + except (json.JSONDecodeError, IOError) as e: + print(f"Error processing {cc_path}: {e}") + returnresources/fix_compile_commands/CMakeLists.txt (3)
6-6: 🛠️ Refactor suggestionUse CMAKE_CURRENT_SOURCE_DIR for correct script path resolution.
CMakeLists.txt resides in a subdirectory, so
CMAKE_SOURCE_DIRmay not resolve to this folder correctly.Apply this diff to reference the script path correctly:
-if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py") +if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py")
7-13: 🛠️ Refactor suggestionAdd dependency to ensure compile_commands.json exists before cleaning.
The custom target runs with
ALLbut doesn't depend on the main build target. This could cause the script to run beforecompile_commands.jsonis generated.Apply this diff to add proper dependency:
add_custom_target( fix_clangd ALL COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py COMMENT "Cleaning compile_commands.json for clangd" VERBATIM + DEPENDS ${CMAKE_PROJECT_NAME}.elf )
10-10: 🛠️ Refactor suggestionUse CMAKE_CURRENT_SOURCE_DIR for script execution path.
Consistent with the existence check, the script execution should also use the correct path variable.
Apply this diff:
- COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py + COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py
🧹 Nitpick comments (3)
resources/fix_compile_commands/fix_compile_commands.py (2)
15-15: Fix unused variable in directory traversal.The
dirsvariable fromos.walk()is not used within the loop body.Apply this diff to rename the unused variable:
- for root, dirs, files in os.walk(start_dir): + for root, _dirs, files in os.walk(start_dir):🧰 Tools
🪛 Ruff (0.11.9)
15-15: Loop control variable
dirsnot used within loop bodyRename unused
dirsto_dirs(B007)
32-32: Improve build directory location logic.The script assumes the build directory is relative to the script location, which may not always be correct in different project structures.
Consider making the build directory configurable:
+import sys + def main(): - build_dir = os.path.join(os.path.dirname(__file__), "build") + build_dir = sys.argv[1] if len(sys.argv) > 1 else os.path.join(os.path.dirname(__file__), "build")docs/en/additionalfeatures/clangd_cdt_support.rst (1)
48-81: Enhance the troubleshooting section with additional context.The section effectively addresses the core issue but could benefit from explaining why these flags cause problems and when users might encounter this issue.
Consider adding this explanatory text after line 60:
+The root cause of these errors is that clangd (LLVM-based language server) doesn't recognize certain GCC-specific compiler flags like ``-fno-tree-switch-conversion``, ``-fno-shrink-wrap``, and ``-fstrict-volatile-bitfields``. These flags are automatically added by ESP-IDF's build system but are not understood by clangd's parser. + Please follow the steps below to fix it:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/en/additionalfeatures/clangd_cdt_support.rst(2 hunks)resources/fix_compile_commands/CMakeLists.txt(1 hunks)resources/fix_compile_commands/fix_compile_commands.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
resources/fix_compile_commands/fix_compile_commands.py
15-15: Loop control variable dirs not used within loop body
Rename unused dirs to _dirs
(B007)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_macos
|
|
||
| 1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/resources/fix_compile_commands/fix_compile_commands.py>`_. | ||
|
|
||
| 2. Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/resources/fix_compile_commands/CMakeLists.txt>`_: |
There was a problem hiding this comment.
Fix broken link path to the CMakeLists.txt file.
Similar to the Python script link, this also contains a duplicate /resources/ segment.
Apply this diff to fix the link:
-2. Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/resources/fix_compile_commands/CMakeLists.txt>`_:
+2. Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/fix_compile_commands/CMakeLists.txt>`_:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 2. Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/resources/fix_compile_commands/CMakeLists.txt>`_: | |
| 2. Invoke the script from the project post build step. Here is example for `CMakeLists.txt <https://github.com/espressif/idf-eclipse-plugin/blob/master/resources/fix_compile_commands/CMakeLists.txt>`_: |
🤖 Prompt for AI Agents
In docs/en/additionalfeatures/clangd_cdt_support.rst at line 64, the link to the
CMakeLists.txt file contains a duplicated /resources/ segment in its path. Fix
this by correcting the URL to remove the extra /resources/ so it matches the
correct path structure, similar to the Python script link.
| if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py") | ||
| add_custom_target( | ||
| fix_clangd ALL | ||
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | ||
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py | ||
| COMMENT "Cleaning compile_commands.json for clangd" | ||
| VERBATIM | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update CMake example to match the corrected implementation.
The documentation should reflect the fixes needed in the actual CMakeLists.txt file to ensure users get working code.
Apply this diff to show the corrected CMake code:
- if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py")
+ if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py")
add_custom_target(
fix_clangd ALL
COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..."
- COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py
+ COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py
COMMENT "Cleaning compile_commands.json for clangd"
VERBATIM
+ DEPENDS ${CMAKE_PROJECT_NAME}.elf
)
endif()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(EXISTS "${CMAKE_SOURCE_DIR}/fix_compile_commands.py") | |
| add_custom_target( | |
| fix_clangd ALL | |
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | |
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_SOURCE_DIR}/fix_compile_commands.py | |
| COMMENT "Cleaning compile_commands.json for clangd" | |
| VERBATIM | |
| ) | |
| endif() | |
| if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py") | |
| add_custom_target( | |
| fix_clangd ALL | |
| COMMAND ${CMAKE_COMMAND} -E echo "Running fix_compile_commands.py..." | |
| COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/fix_compile_commands.py | |
| COMMENT "Cleaning compile_commands.json for clangd" | |
| VERBATIM | |
| DEPENDS ${CMAKE_PROJECT_NAME}.elf | |
| ) | |
| endif() |
🤖 Prompt for AI Agents
In docs/en/additionalfeatures/clangd_cdt_support.rst around lines 68 to 76,
update the CMake example to match the corrected implementation by replacing the
existing snippet with the provided diff. This involves ensuring the
add_custom_target named fix_clangd runs the fix_compile_commands.py script
correctly with proper commands and comments, reflecting the actual fixed
CMakeLists.txt usage for users.
| Please follow the steps below to fix it: | ||
|
|
||
| 1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/resources/fix_compile_commands/fix_compile_commands.py>`_. |
There was a problem hiding this comment.
Fix broken link path to the Python script.
The link contains a duplicate /resources/ segment in the path, making it incorrect.
Apply this diff to fix the link:
-1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/resources/fix_compile_commands/fix_compile_commands.py>`_.
+1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/fix_compile_commands/fix_compile_commands.py>`_.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/resources/fix_compile_commands/fix_compile_commands.py>`_. | |
| 1. Download the script for `fix_compile_commands.py <https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/fix_compile_commands/fix_compile_commands.py>`_. |
🤖 Prompt for AI Agents
In docs/en/additionalfeatures/clangd_cdt_support.rst at line 62, the hyperlink
to the Python script fix_compile_commands.py contains a duplicated /resources/
segment in the URL path. Correct the link by removing the extra /resources/ so
that the URL points to
https://github.com/espressif/idf-eclipse-plugin/tree/master/resources/fix_compile_commands/fix_compile_commands.py,
ensuring the link is valid and directs to the intended script.
|
@kolipakakondal @AndriiFilippov found some issues related to clangd syntax highlighting from this PR's builds after merge to the master. Can it be something related to the added CMake file or python script. @AndriiFilippov please share your original bug and findings here. Although this is not directly impacting but it might help you in fixing the issue |
|
Hi @alirana01 The script was only for documentation purposes and wasn’t called anywhere from the code. We can suggest this approach to users who has errors when they navigate the esp-idf source files — it won’t have any impact on the build as such. |
|
As mentioned in the PR description, here is the plan
|
Yes sorry I just saw it now |
Description
Write a script to remove -m and -f arguments from compile_commands.json so that we can avoid errors when navigating the ESP-IDF internal component source code
Why? Those arguments are applicable to the GCC compiler, but when Clangd uses the compile_commands.json file, it throws errors saying the arguments are invalid.
This PR includes the script and the steps required to use it, so we can recommend it to users. Going forward, we can consider implementing it by default as part of project creation, along with updating the CMakeLists.txt.
Fixes # (IEP-1550)
Triggered by https://esp32.com/viewtopic.php?t=45824
Type of change
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
New Features
Documentation