[Runtimes] Merge 'compile_commands.json' files from runtimes build #116303
Conversation
jhuber6
requested review from
compnerd,
jdoerfert,
llvm-beanz,
MaskRay,
petrhosek and
shiltian
|
✅ With the latest revision this PR passed the Python code formatter. |
jhuber6
force-pushed
the
JSONCommands
branch
from
11aab09 to
458aee6
Compare
|
This slightly changes the semantics of |
Well you'll still get the normal |
jhuber6
force-pushed
the
JSONCommands
branch
from
d36eabc to
4af3e4b
Compare
|
I fixed the issue where it would error if you manually deleted the dependency. Right now this properly re-merges it whenever the runtime compile commands changes. I believe this is very useful for making runtimes builds less painful, so hopefully there aren't any additional issues. |
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. warning
jhuber6
force-pushed
the
JSONCommands
branch
from
4af3e4b to
382a334
Compare
|
Ping |
No problem, I think this is fairly innocuous as-is, unless people are really concerned about a few extra MB of JSON. I've personally gotten really tired of doing this manually every time CMake reconfigured. |
|
Just want to confirm that,, when the top level configure runs again, the top level |
Yes, if only the projects configure stage runes. If the runtimes configure stage runs afterwards it will trigger that to be updated which will then make the file stale and trigger the merge again. |
|
That is fine. We don't want to change the semantics of top level configure, and also don't want to have any stale commands in it if only the top level configure runs. |
|
Have you measured at the performance on larger builds? For example, in the Fuchsia toolchain build that I normally use, I have 38 |
My build has three, but I'm on a beefier server. I'd assume someone doing a standard release build might turn off the compile commands if they don't want the overhead? Would it be possible for you to try it locally? It'd be hard for me to test Fuchsia's build locally. I didn't notice any considerable slowdown, and it only happens once per reconfigure so rebuilds are fast for the most part. |
|
So, on my machine it takes about 0.3 seconds to merge an 8 MB compile commands into a 23 MB one. That'll likely be done in parallel, that time is somewhat similar to some of the worse feature tests in CMake so it might not be too much of an issue. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/11493 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1707 Here is the relevant piece of the build log for the reference |
|
@jhuber6 This seems to break the runtimes build. With |
Ah, so this seems to be from invoking the runtimes build directly? In that case we probably don't want this to trigger. I wonder if there's a way to detect that. Feel free to revert for now. |
…build (#116303) 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. Fixed issue w/ standalone runtimes build by checking if the LLVM src and CMake src are the same.
| data = json.load(f) | ||
| merged_data.extend(data) | ||
| except (IOError, json.JSONDecodeError) as e: | ||
| print("Failed to parse {json_file}: {e}", file=sys.stderr) |
There was a problem hiding this comment.
| print("Failed to parse {json_file}: {e}", file=sys.stderr) | |
| print(f"Failed to parse {json_file}: {e}", file=sys.stderr) |
There was a problem hiding this comment.
Honestly I think this print shouldn't exist so people don't get confused.
Summary:
When building a project in a runtime mode, the compilation database is a
separate CMake invocation. So its
compile_commands.jsonfile will beplaced elsewhere in the
runtimes/runtime-binsdirectory. This issomewhat annoying for ongoing development when a runtimes build is
necessary. This patch adds some CMake magic to merge the two files.