[Lldb-commits] [PATCH] D41902: Remove Platform references from the Host module
Davide Italiano via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 11 14:42:28 PST 2018
The test Jim committed broke the public bots, so I went ahead and reverted it.
Committing to https://llvm.org/svn/llvm-project/lldb/trunk ...
Author: Davide Italiano <ditaliano at apple.com>
Date: Thu Jan 11 14:37:49 2018 -0800
[testsuite] Remove a broken test which tried to find App in bundles.
That never really worked, and the change associated isn't yet
committed, so, let's try to make the bots green for now.
Let's make sure we add a test before we commit your change :)
On Thu, Jan 11, 2018 at 9:51 AM, Pavel Labath <labath at google.com> wrote:
> On 11 January 2018 at 16:22, Greg Clayton <clayborg at gmail.com> wrote:
>>> On Jan 11, 2018, at 8:10 AM, Pavel Labath <labath at google.com> wrote:
>>> On 11 January 2018 at 16:04, Greg Clayton <clayborg at gmail.com> wrote:
>>>>> On Jan 11, 2018, at 5:12 AM, Pavel Labath <labath at google.com> wrote:
>>>>> Thanks for adding the test Jim. I have confirmed that it passes with
>>>>> this patch applied (because the bundle is resolved during target
>>>>> creating, not launching).
>>>>> However, this did make me aware of the fact that this patch removed
>>>>> the bundle resolution logic from the launcher itself, so I'm going to
>>>>> add back that part.
>>>> The platform really does need a part in this. Each platform might have different files. APK files for Android, .ipa files for iTunes, .app bundles for all darwin operating systems and more. So the platform really is the right place for this kind of logic.
>>> Yes, but this is in the host layer, which already assumes the
>>> knowledge of the host platform -- there no sense in trying to "launch"
>>> an APK file on iOS. It actually fits quite nicely, since the code that
>>> actually resolved the executable in the bundle was already present in
>>> the host layer.
>> I am all for detangling the Host and Platform code. As long as we maintain the ability to resolve a binary inside a bundle when setting the executable in a Target, I am not picky where this happens.
> I fully agree with that. To elaborate on my previous statement:
> The code that handles the resolving of executables for debugging is in
> the Platform class and I'm not changing that.
> The code I'm editing is only used when you actually go on the exec()
> the executable. This only makes sense for executables referring to the
> host platform. For executables we launching for debugging, it doesn't
> even matter if we resolve stuff here, because it will already be
> pre-resolved. So, this only applies to other things we are launching,
> such as the argdumper, lldb-server and such... It's not fully clear to
> me whether the extra resolving magic is actually desirable here, but I
> couldn't see a reason why we shouldn't have it, so I kept the status
More information about the lldb-commits