-
Notifications
You must be signed in to change notification settings - Fork 469
feat: implement options to pass custom LLDB command and python script files… #338
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
Conversation
…. A little refactoring.
I'll review soon. |
OK, cool, just to note - there will be merge conflicts between this and #339, so after merging this I'll probably need to update the other. |
Pushing to 1.9.4 since I don't have time to review for 1.9.3 (need to get fix out for build issue) |
…-scripting # Conflicts: # src/ios-deploy/ios-deploy.m # scripts/lldb_cmds
I merged #339 which had some lldb script changes. You mentioned that there might be conflicts, can you take a look. |
Does anyone has any objections against this pull request? It's been hanging here for more than a year, I could merge it, but it would be for the best that someone at least checks if it does not break anythin. |
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'm weary of adding more odd shell scripts to our build system rather than using than using odd strings. This will likely break people who are trying to build with systems that aren't standard Xcode(e.g. bazel of ninja).
Could this be refactored to not add new shell scripts to the build process (or provide better justification for adding that complexity).
I don't see no new shell scripts - this just moves some LLDB commands out to external source (i.e. cleans up the main source file a bit) and adds a feature to add your own LLDB comands to run when app is installed on the device. But I agree that this should be tested with those exotic build systems. |
); | ||
runOnlyForDeploymentPostprocessing = 0; | ||
shellPath = /bin/sh; | ||
shellScript = "echo \"\\\"# AUTO-GENERATED - DO NOT MODIFY\\\\\\n\\\"\" > src/ios-deploy/lldb_cmds.h\nawk '{ print \"\\\"\"$0\"\\\\n\\\"\"}' src/scripts/lldb_cmds >> src/ios-deploy/lldb_cmds.h\n"; |
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 know that other people are building this not using xcodebuild and that this change will require updating those build systems to create a system to emulate what this awk script does (adding escape sequences to src/scripts/lldb_cmds and placing it in src/ios-deploy/lldb_cmds.h).
Specifically I sometimes use bazel to build this, which means I do the .py->.h transform when I pull the files down from github. Bazel is heavy on verifiable inputs and outputs during compilation(it doesn't like dynamically transposing a source file).
This PR is a bit stale... but is also a place where we could make ios-deploy better... The awk script python injection makes building ios-deploy janky... What about removing that? We can just include the source of the Python script and add the escaping when we inject it(i.e. remove the Python ShellScript in the Xcode project file). Removing the Python ShellScript would remove my objections here. |
Closing this for now since XCode12 has Python script changes. |
This is a re-submitted pull request from a separate feature branch.
I wrote a proposal in shazron/phonegap-questions#57
Here is the implementation of the first solution - two arguments that would add ability to pass custom LLDB commands and a custom Python script to handle communication with the process. This will require for the user to implement the whole "fruitstrap" logic on their own side, but it also gives way for custom communication with LLDB Python APIs which wasn't possible before.