-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb][RPC] Upstream LLDB to RPC converstion Python script #138028
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
[lldb][RPC] Upstream LLDB to RPC converstion Python script #138028
Conversation
@llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) ChangesAs part of upstreaming LLDB RPC, this commit adds python scripts that are used by LLDB RPC to modify the public lldb header files for use with RPC. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804 Full diff: https://github.com/llvm/llvm-project/pull/138028.diff 3 Files Affected:
diff --git a/lldb/scripts/convert-lldb-header-to-rpc-header.py b/lldb/scripts/convert-lldb-header-to-rpc-header.py
new file mode 100755
index 0000000000000..4550837d8e08d
--- /dev/null
+++ b/lldb/scripts/convert-lldb-header-to-rpc-header.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+# Usage: convert-lldb-header-to-rpc-header.py <path/to/input-header.h> <path/to/output-header.h>
+# This scripts takes common LLDB headers (such as lldb-defines.h) and replaces references to LLDB
+# with those for RPC. This happens for:
+# - namespace definitions
+# - namespace usage
+# - version string macros
+# - ifdef/ifndef lines
+
+import argparse
+import os
+import re
+
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ with open(input_path, "r") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w") as output_file:
+ for line in lines:
+ # NOTE: We do not use lldb-forward.h or lldb-versioning.h in RPC, so remove
+ # all includes that are found for these files.
+ if re.match(
+ r'#include "lldb/lldb-forward|#include "lldb/lldb-versioning', line
+ ):
+ continue
+ # For lldb-rpc-defines.h, replace the ifndef LLDB_LLDB_ portion with LLDB_RPC_ as we're not
+ # using LLDB private definitions in RPC.
+ elif re.match(r".+LLDB_LLDB_", line):
+ output_file.write(re.sub(r"LLDB_LLDB_", r"LLDB_RPC_", line))
+ # Similarly to lldb-rpc-defines.h, replace the ifndef for LLDB_API in SBDefines.h to LLDB_RPC_API_ for the same reason.
+ elif re.match(r".+LLDB_API_", line):
+ output_file.write(re.sub(r"LLDB_API_", r"LLDB_RPC_API_", line))
+ # Replace the references for the macros that define the versioning strings in
+ # lldb-rpc-defines.h.
+ elif re.match(r".+LLDB_VERSION", line):
+ output_file.write(re.sub(r"LLDB_VERSION", r"LLDB_RPC_VERSION", line))
+ elif re.match(r".+LLDB_REVISION", line):
+ output_file.write(re.sub(r"LLDB_REVISION", r"LLDB_RPC_REVISION", line))
+ elif re.match(r".+LLDB_VERSION_STRING", line):
+ output_file.write(
+ re.sub(r"LLDB_VERSION_STRING", r"LLDB_RPC_VERSION_STRING", line)
+ )
+ # For local #includes
+ elif re.match(r'#include "lldb/lldb-', line):
+ output_file.write(re.sub(r"lldb/lldb-", r"lldb-rpc-", line))
+ # Rename the lldb namespace definition to lldb-rpc.
+ elif re.match(r"namespace lldb", line):
+ output_file.write(re.sub(r"lldb", r"lldb_rpc", line))
+ # Rename namespace references
+ elif re.match(r".+lldb::", line):
+ output_file.write(re.sub(r"lldb::", r"lldb_rpc::", line))
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+
+
+if __name__ == "__main__":
+ main()
diff --git a/lldb/scripts/framework-header-include-fix.py b/lldb/scripts/framework-header-include-fix.py
new file mode 100755
index 0000000000000..e214ce005923f
--- /dev/null
+++ b/lldb/scripts/framework-header-include-fix.py
@@ -0,0 +1,44 @@
+#!/usr/bin/env python3
+# Usage: framework-header-include-fix.py <path/to/input-header.h> <path/to/output-header.h>
+# This script modifies all #include lines in all lldb-rpc headers
+# from either filesystem or local includes to liblldbrpc includes.
+
+import argparse
+import os
+import re
+
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ with open(input_path, "r+") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w+") as output_file:
+ for line in lines:
+ # Replace includes from RPCCommon to liblldbrpc includes.
+ # e.g. #include <lldb-rpc/common/RPCArgument.h> -> #include <LLDBRPC/RPCArgument.h>
+ if re.match(r".+<lldb-rpc/common", line):
+ output_file.write(re.sub(r"<lldb-rpc/common", r"<LLDBRPC", line))
+ # Replace all local file includes to liblldbrpc includes.
+ # e.g. #include "SBFoo.h" -> #include <LLDBRPC/SBFoo.h>
+ elif re.match(r'#include "(.*)"', line):
+ include_filename = re.search(r'#include "(.*)"', line).groups()[0]
+ output_file.write(
+ re.sub(
+ r'#include "(.*)"',
+ r"#include <LLDBRPC/" + include_filename + ">",
+ line,
+ )
+ )
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+
+
+if __name__ == "__main__":
+ main()
diff --git a/lldb/scripts/framework-header-version-fix.py b/lldb/scripts/framework-header-version-fix.py
new file mode 100755
index 0000000000000..72185f8e820ce
--- /dev/null
+++ b/lldb/scripts/framework-header-version-fix.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+# Usage: framework-header-version-fix.py <path/to/input-header.h> <path/to/output-header.h> MAJOR MINOR PATCH
+# This script modifies lldb-rpc-defines.h to uncomment the macro defines used for the LLDB
+# major, minor and patch values as well as populating their definitions.
+
+import argparse
+import os
+import re
+
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input")
+ parser.add_argument("output")
+ parser.add_argument("lldb_version_major")
+ parser.add_argument("lldb_version_minor")
+ parser.add_argument("lldb_version_patch")
+ args = parser.parse_args()
+ input_path = str(args.input)
+ output_path = str(args.output)
+ lldb_version_major = args.lldb_version_major
+ lldb_version_minor = args.lldb_version_minor
+ lldb_version_patch = args.lldb_version_patch
+
+ with open(input_path, "r") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w") as output_file:
+ for line in lines:
+ # Uncomment the line that defines the LLDB major version and populate its value.
+ if re.match(r"//#define LLDB_RPC_VERSION$", line):
+ output_file.write(
+ re.sub(
+ r"//#define LLDB_RPC_VERSION",
+ r"#define LLDB_RPC_VERSION " + lldb_version_major,
+ line,
+ )
+ )
+ # Uncomment the line that defines the LLDB minor version and populate its value.
+ elif re.match(r"//#define LLDB_RPC_REVISION$", line):
+ output_file.write(
+ re.sub(
+ r"//#define LLDB_RPC_REVISION",
+ r"#define LLDB_RPC_REVISION " + lldb_version_minor,
+ line,
+ )
+ )
+ # Uncomment the line that defines the complete LLDB version string and populate its value.
+ elif re.match(r"//#define LLDB_RPC_VERSION_STRING$", line):
+ output_file.write(
+ re.sub(
+ r"//#define LLDB_RPC_VERSION_STRING",
+ r'#define LLDB_RPC_VERSION_STRING "{0}.{1}.{2}"'.format(
+ lldb_version_major, lldb_version_minor, lldb_version_patch
+ ),
+ line,
+ )
+ )
+ else:
+ # Write any line that doesn't need to be converted
+ output_file.write(line)
+
+
+if __name__ == "__main__":
+ main()
|
We should add shell tests for these scripts. |
I can add shell tests 👍🏾 |
c915f7c
to
c51a312
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests. Please let others take a look before merging.
lldb/test/Shell/RPC/Scripts/TestFrameworkIncludeFixScript/CheckSBClass.test
Outdated
Show resolved
Hide resolved
lldb/test/Shell/RPC/Scripts/TestVersionFixScript/CheckLLDBDefines.test
Outdated
Show resolved
Hide resolved
For what it's worth, while these scripts do work for intended purpose for now, an approach I'd love is to have all this header modification be a subtool of the |
c51a312
to
a00c76d
Compare
@DavidSpickett I pushed here to address most of the outstanding changes I needed to make from your comments, and this change should also fix the CI issue on BuildKite. Could you give this patch another pass over? |
// Copy lldb-defines.h from source. | ||
# RUN: mkdir -p %t/input | ||
# RUN: mkdir -p %t/output | ||
# RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're mixing two comment styles here, and lit doesn't actually treat these any differently (it just look for RUN lines). So let's settle on one and drop them everywhere else.
// Copy lldb-defines.h from source. | |
# RUN: mkdir -p %t/input | |
# RUN: mkdir -p %t/output | |
# RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input | |
// Copy lldb-defines.h from source. | |
RUN: mkdir -p %t/input | |
RUN: mkdir -p %t/output | |
RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input |
or
// Copy lldb-defines.h from source. | |
# RUN: mkdir -p %t/input | |
# RUN: mkdir -p %t/output | |
# RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input | |
# Copy lldb-defines.h from source. | |
RUN: mkdir -p %t/input | |
RUN: mkdir -p %t/output | |
RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input |
I think the latter is slightly more common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I didn't know that about llvm-lit. I'll change everything to use Python-style comments and remove the hashes from the lit commands
# Usage: convert-lldb-header-to-rpc-header.py <path/to/input-header.h> <path/to/output-header.h> | ||
# This scripts takes common LLDB headers (such as lldb-defines.h) and replaces references to LLDB | ||
# with those for RPC. This happens for: | ||
# - namespace definitions | ||
# - namespace usage | ||
# - version string macros | ||
# - ifdef/ifndef lines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python has a PEP that describes how to document files/modules. We should use that here too.
# Usage: convert-lldb-header-to-rpc-header.py <path/to/input-header.h> <path/to/output-header.h> | |
# This scripts takes common LLDB headers (such as lldb-defines.h) and replaces references to LLDB | |
# with those for RPC. This happens for: | |
# - namespace definitions | |
# - namespace usage | |
# - version string macros | |
# - ifdef/ifndef lines | |
""" | |
Usage: convert-lldb-header-to-rpc-header.py <path/to/input-header.h> <path/to/output-header.h> | |
This scripts takes common LLDB headers (such as lldb-defines.h) and replaces references to LLDB | |
with those for RPC. This happens for: | |
- namespace definitions | |
- namespace usage | |
- version string macros | |
- ifdef/ifndef lines | |
""" |
// Copy lldb-rpc-defines.h from source. | ||
# RUN: mkdir -p %t/input | ||
# RUN: mkdir -p %t/output | ||
# RUN: cp %p/../../../../../include/lldb/lldb-defines.h %t/input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the goal to run this on the sources to catch regressions, or are we just using the source file as a meaningful input? If it's the latter, I would recommend copying the file into the Inputs directory and reducing it to the bare minimum needed for the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the script failed to work at all, that would be caught by the build step, right?
In which case yes the input for this should be small. It'll save us cutting down the real file to find the place that's failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed all the tests so that the files are no longer copied from source. Each test should now be using truncated versions of whatever files will be operated on and these files are in the tests' Inputs directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Jonas' comments addressed I'm probably fine with this. Need to read the design doc again to be sure.
01e35b2
to
2b6f460
Compare
I think this could be ready to land now @DavidSpickett and @JDevlieghere, could you give this another once over? |
2b6f460
to
4520d08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the license comment addressed.
CI failure but looks unrelated. |
4520d08
to
208288f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Might be worth making this match the other scripts and doing this once instead of line-by-line.
208288f
to
4f818cc
Compare
✅ With the latest revision this PR passed the Python code formatter. |
As part of upstreaming LLDB RPC, this commit adds a python script that is used by LLDB RPC to modify the public lldb header files for use with RPC. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804
4f818cc
to
ba96f13
Compare
Updated the patch to do this 👍🏾 |
As part of upstreaming LLDB RPC, this commit adds a python script that is used by LLDB RPC to modify the public lldb header files for use with RPC. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804
As part of upstreaming LLDB RPC, this commit adds a python script that is used by LLDB RPC to modify the public lldb header files for use with RPC. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804 (cherry picked from commit a2d2941)
As part of upstreaming LLDB RPC, this commit adds a python script that is used by LLDB RPC to modify the public lldb header files for use with RPC. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804
As part of upstreaming LLDB RPC, this commit adds a python script that
is used by LLDB RPC to modify the public lldb header files for use with
RPC.
https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804