Skip to content
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.

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

[Runtimes] Merge 'compile_commands.json' files from runtimes build #116303

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Nov 15, 2024

Summary:
When building a project in a runtime mode, the compilation database is a
separate CMake invocation. So its compile_commands.json file will be
placed elsewhere in the runtimes/runtime-bins directory. This is
somewhat annoying for ongoing development when a runtimes build is
necessary. This patch adds some CMake magic to merge the two files.

@@ -0,0 +1,42 @@
#!/usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need LLVM copyright here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think so, none of the other utility scripts I saw had one.

data = json.load(f)
merged_data.extend(data)
except (IOError, json.JSONDecodeError):
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

probably drop a warning or so here

Copy link

github-actions bot commented Nov 15, 2024

✅ With the latest revision this PR passed the Python code formatter.

Summary:
When building a project in a runtime mode, the compilation database is a
separate CMake invocation. So its `compile_commands.json` file will be
placed elsewhere in the `runtimes/runtime-bins` directory. This is
somewhat annoying for ongoing development when a runtimes build is
necessary. This patch adds some CMake magic to merge the two files.
${LLVM_BINARY_DIR}/compile_commands.json
${CMAKE_BINARY_DIR}/compile_commands.json
-o ${LLVM_BINARY_DIR}/compile_commands.json
DEPENDS ${CMAKE_BINARY_DIR}/compile_commands.json)
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC this custom command will just depend on the main compile_commands.json, which is generated after CMake configuration. However, the runtime build is the last stage of build, so I'm not sure if this dependency can push it back to the end.

Copy link
Contributor Author

@jhuber6 jhuber6 Nov 15, 2024

Choose a reason for hiding this comment

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

Yeah I'm not 100% positive on how this will work out. When I test it under normal circumstances (fresh build and modification of files) it works fine. however if I manually delete the main compile_commands.json it will get into a dumb state where it keeps thinking it's not there.

@jhuber6 jhuber6 removed the request for review from a team November 15, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants