[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Thanks for your efforts here, very much appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://reviews.llvm.org/D136465 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Landed as https://github.com/llvm/llvm-project/commit/1e210abf9925ad08fb7c79894b4ec5ef8f0ef173. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://reviews.llvm.org/D136465

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1e210abf9925: [LLDB] Make remote-android local ports configurable (authored by mark2185, committed by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 added a comment. In D136465#3884755 , @DavidSpickett wrote: >> Great! I'd just like to note that I do not have commit access, per the >> guide's instructions. > > What name/email address do you want on the commit? Luka Markušić

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Great! I'd just like to note that I do not have commit access, per the > guide's instructions. What name/email address do you want on the commit? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 added a comment. In D136465#3884047 , @clayborg wrote: > Looks ok to me. I don't do Android stuff on a daily basis. As long as the old > way of connecting still works I think this is ok. Great! I'd just like to note that I do not have commit

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Looks ok to me. I don't do Android stuff on a daily basis. As long as the old way of connecting still works I think this is ok. It would be great to get more stuff in the Android remove platform connection working at some point. Users still have to manually install

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 updated this revision to Diff 470421. mark2185 added a comment. Make remote-android local ports configurable The local ports for `platform connect` and `attach` were always random, this allows the user to configure them. This is useful for debugging a truly remote android (when the

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Yes, somehow, and yes just update again. Happens to us all, I have learned to not try to be clever with arc and just keep one commit per review. Tip: if you look in the "Revision Contents" box there is a "history" and you can diff between versions of the patch,

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 marked 2 inline comments as done. mark2185 added a comment. Did I just overwrite the initial commit with a new one, instead of just creating a diff based on the two comments? I'm terribly sorry, should I just squash my two commits and run `arc diff --update=D136465` again?

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 updated this revision to Diff 470381. mark2185 added a comment. Remove {} for single line if statements per LLVM coding guidelines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://reviews.llvm.org/D136465 Files:

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 added a comment. In D136465#3880809 , @clayborg wrote: > Is there any way to test this? Through the test suite? I couldn't find //any// `android platform` related tests, and this would require `adb` to be running in the background, so I'm not

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Is there any way to test this? Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:95-97 + if (gdbstub_port) { +local_port = std::stoi(gdbstub_port); + } remove {} for single line if statements

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 added a comment. In D136465#3878640 , @DavidSpickett wrote: > Ok, so this change means that the randomisation still happens, unless you > override it with these environment variables? This seems like a good way to > do it. Exactly, this is

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Ok, so this change means that the randomisation still happens, unless you override it with these environment variables? This seems like a good way to do it. Where do these env vars need to be set? On the local machine, on the lldb-server machine, on the device?

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 added a comment. In D136465#3878581 , @DavidSpickett wrote: > Does this account for the situation where you spawn many gdbserver from the > platform and therefore need more ports? Does it even need to? (just guessing > that that could have

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I don't have the expertise to review, but have one question. Does this account for the situation where you spawn many gdbserver from the platform and therefore need more ports? Does it even need to? (just guessing that that could have been the reason for

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-21 Thread Luka Markušić via Phabricator via lldb-commits
mark2185 created this revision. Herald added a subscriber: danielkiss. Herald added a project: All. mark2185 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The local ports for `platform connect` and `attach` were always random, this