-
Notifications
You must be signed in to change notification settings - Fork 11
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
Minimal example to get debugging to work with iOS 17 #25
base: main
Are you sure you want to change the base?
Conversation
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 a lot for experimenting and creating this PR!
I'll try this out soon enough. In the meantime, I've added a few comments for the things that came to mind on first look.
@@ -162,7 +163,7 @@ export class DebugConfigurationProvider implements vscode.DebugConfigurationProv | |||
}); | |||
} | |||
} | |||
else | |||
else | |||
{ | |||
platformPath = await targetCommand.deviceAppPath({ |
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.
Does this still work? Doesn't this also use ios-deploy
?
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.
At the moment, getting the platform path still works with ios-deploy
.
One can also do it with devicectl
of course, for running processes as the runningProcesses.executable
of xcrun devicectl device info processes --json-output -
and for all apps installed via apps.url
of xcrun devicectl device info apps --json-output -
.
{ | ||
let platformPath: string|void; | ||
if (dbgConfig.iosRequest === "launch") | ||
{ | ||
if (dbgConfig.iosInstallApp) { | ||
platformPath = await targetCommand.deviceInstall({ |
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.
We'd also need to replace deviceInstall
, right? Just noting, we can do this incrementally in a different PR as well.
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.
Installing the app still works with ios-deploy
. Only getting the pid of a running process and spawning the debug server seems to require DeveloperDiskImage.dmg
and hence is retired since iOS 17.
Of course long-term it would be sort of cleaner to use xcrun devicectl
instead as well, but this will only work with XCode >= 15 installed, so at the time being ios-deploy
actually provides the highest compatibility.
// using detour with `script` command, as direct evocation causes CodeLLDB to crash at the moment | ||
// probably due to a bug in CodeLLDB / unusal exit results from the `device` command | ||
let processName = path.basename(platformPath).split('.')[0]; | ||
dbgConfig.processCreateCommands.push(`script lldb.debugger.HandleCommand("device process attach --name '${processName}'")`); |
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.
How reliable attaching by name? Can the process have different name than binary? Should we try attaching to pid instead of name, if we can get pid from the process launch command above?
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.
Not sure, pid might be cleaner. It gets returned as process.processIdentifier
from xcrun devicectl device process launch --json-output -
.
Cleanest option imo would be to use lldb
also to start the process, but I couldn't figure out a way to do this with Apples lldb device
.
// attach to the app via LLDB | ||
dbgConfig.processCreateCommands = (dbgConfig.postRunCommands instanceof Array) ? dbgConfig.processCreateCommands : []; | ||
// LLDB `device` command needs Apple's beta LLDB set as backend for CodeLLDB | ||
// e.g. set vscode://settings/lldb.library to /Applications/Xcode-beta.app/Contents/SharedFrameworks/LLDB.framework/Versions/A/LLDB |
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.
This is an interesting requirement and will hamper normal usage. Maybe we should find a way to specify this always dynamically for a debug session. Again, not directly for this PR, but as a future improvement. Until now, I believe this was optional and the in-built lldb version in CodeLLDB also used to work well.
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 agree that it is an additional hurdle, but I can not think of an easy or clean way to programmatically set this, since we'd be messing with the settings of another extension, and the path to Apple's LLDB library might also vary?
Maybe the cleanest option is to just send a clear alert to the user explaining what to do (whenever they are using iOS 17 or when lldb
complains about device
not being a command).
|
This pull request is addressing the issues #18 and #24 and contains a minimal example of what is needed to get the extension to work with Apple LLDB, and hence compatible with iOS 17. It requires the XCode 16 beta.
It's only using Apple LLDB and
xcrun devicectl
when really needed, the functions using ios-deploy are left as-is.