Add support for using extended-remote with launch configurations

[esben]

This extends gdb launch configurations to support extended-remote. It can be used for remote debugging where code is compiled on host, and transferred and debugged on target using a remote gdbserver (or something else speaking GDB/MI).

Note, this is not touching the code added in commit 318ece44cf3a ("Added special commands for extended-remote fix #91"), as that is only related to gdb attach configuration, although the example in package.json is confusingly put with launch, which is opposite of the code change. I did not fixup that change, as I am not sure exactly what to put instead.

[esben]

This is my first Node.js code ever, so please bear with me. I did not manage to run the test code, so I have no idea if I needed to do any changes in that.

[esben]

Changed string quoting to backticks for better readability and rebased to master. Figured out how to run the test code :)

[esben]

Please take a look at 318ece44cf3a5d51222a9c2e8031b7c1fdc46894. It adds support for extended-remote for AttachRequest, although confusingly, it shows it for launch in package.json, which is clearly an error. This PR adds support for extended-remote in LaunchRequest, without making any changes to the existing support for extended-remote in AttachRequest.

Why do you want to have this in attach instead of launch? Logically (and practically), attach is for attaching to an existing process, and launch is for starting a new process and debugging that. What I do here is copying the executable to the remote, launching it and debugging it from there. I believe doing this as an AttachRequest will be confusing, and also complicate the code, as attach will then have to do the same as launch already does.

[WebFreak001]

I see, launch does make sense for actually starting the application, but it feels like it could be confusing adding an additional "executable" field to the launch settings. Connecting to a remote and executing the launch commands there also feels a little bit like it's not actually launching. Using attach will also show a stop button with some kind of cable icon in the stop button which I think fits extended remote more. Displaying it with the attach UI also implies that it is not running locally or that the extension is not in direct control over the executable (as the remote is), which would be a little bit more clear I think.

Adding the file copy commands, which are a great addition for remote debugging, to the attach code instead would make the code changes extremely minimal. See the isExtendedRemote code in mi2.ts for this which would just need the changed commands. Attach also already has separate target and executable properties in the JSON schema, which would easily be usable. I think it would just be best to add an optional "copy files to remote" boolean to this.

[esben]

While I do agree with you that the executable argument added to LauncRequestArguments is likely to cause some confusion, I belive it is directly linked to the way that the target argument is already dfined for launch and attach. The launch target argument basically has the same meaning as the attach argument.

Looking forward, it might be best to fix this now, renaming the launch target argument to executable, which allow for a much clearer way to add support for extended-remote. A normal (local) launch request would simply specify the executable, where as a remote (extended-remote) launch would specify both executable and a-new-to-be-named argument specifying the remote to connect to. Logically, that argument should probably be named "remote", or maybe better "extendedRemote".

I don't share your feeling that a remote launch does not feel like an actually launch. Yes, seen from a network centric point of view, extended-remote launch is an "attach" action, but from a debug point of view, extended-remote launch is a "launch" action.

As for the code structure, handling extended-remote launch in attachRequest() would mean that start() must be called there (only when extended-remote launch style attachRequest is done of-course). Implementing extended-remote launch in launchRequest() is simply the least intrusive solution.

What should we do?

[WebFreak001]

I too think a new executable argument with backwards compatibility from target (and deprecation message) would be very nice for the user indeed.

With a new clear key for this I agree this change makes sense and would make a good addition.

However I think if we add the copy files commands, they should also be added to attach for consistency because that does seem like a very useful command for debugging.

For a plan I would now suggest:

• add executable value in launch, deprecate old target value, add new key for possible other functionality
• add new attributes needed for this PR
• add the copy commands to attach (under a configuration flag)

Also please add the new configuration attributes to the JSON schema of the GDB section and change the keys in LLDB and Mago-MI if you rename executable. See https://json-schema.org/

[esben]

Should we leave the attach arguments as they are now? The target and remote arguments there are also kind of tricky/confusing. Maybe deprecate the target argument in attach also, adding new arguments for pid and remote. Unfortunately, it would be really nice if they could be named "pid" and "remote", but the "remote" argument is already defined as a boolean.

[WebFreak001]

hm for attach I don't think "target" is such a bad name

[esben]

Ok, I have updated PR with the changes regarding launch target/executable argument. I will not be adding generic copy commands in this PR. I believe it is better handled in another PR (and I am not sure how the configuration parameter for that should look like).

[ghost]

Is there any progress in this PR? Can it be merged? Would be very helpful to have this feature.