[Lldb-commits] [PATCH] D41902: Remove Platform references from the Host module

[Pavel Labath via lldb-commits]

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
quo.