[Lldb-commits] [PATCH] Move argument globbing to Target::Launch from Platform::LaunchProcess.

[Oleksiy Vyalov]

Yes, you're right regarding glob but it seems there is an alternative - use
wordexp on Linux/OSX and _findfirst on Windows. In this case no need to
deploy a separate binary for this.

On Thu, Feb 19, 2015 at 4:33 PM, Zachary Turner <zturner at google.com> wrote:

> Is this not something we could solve by writing a function in Host that
> behaves the way we'd like it to behave on all platforms?  I mean it might
> be an ugly function, but the idea of passing stuff to an external program
> is kinda meh.  FWIW launching a process is not a lightweight operation on
> windows like it is on other platforms, and there's actually noticeable
> overhead.  So it would be great if we could remove dependencies on external
> programs.
>
> On Thu Feb 19 2015 at 4:30:24 PM Enrico Granata <egranata at apple.com>
> wrote:
>
>> IIUC, glob() does not perform environment variables expansion - and even
>> support for ~ expansion is an extension that is not generally POSIX
>> compliant
>> These are things we’d like to preserve (definitely we would like to have
>> these abilities on OS X)
>>
>> On Feb 19, 2015, at 4:23 PM, Oleksiy Vyalov <ovyalov at google.com> wrote:
>>
>> On Linux the platform is used to launch a process only when LLGS_LOCAL
>> mode is on - otherwise, process plugin is used.
>>
>> I'm wondering whether we need the standalone binary argdumper for
>> globbing - potentially, we may just use glob function on Linux and OSX. For
>> remote execution there might be a new protocol extension command (e.g.,
>> qGlobArgs) which either calls glob or runs argdumper.
>>
>> On Thu, Feb 19, 2015 at 3:12 PM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> There's currently a lot of ways to launch processes, and the
>>> interactions that lead to specific methods being chosen can be a bit
>>> confusing sometimes.
>>>
>>> Out of curiosity, if a plugin supports launching under a debugger
>>> directly, why is the separate start stopped / attach / resume algorithm
>>> used?  It seems more straightforward.
>>>
>>> If someone gets themselves through that codepath, argdumper won't be
>>> used.  Which is confusing if you're outside looking in at the API.  From
>>> the outside you just see the process gets launched using a supported
>>> mechanism for launching processes, and it ignores the flag.
>>>
>>> On Thu Feb 19 2015 at 3:03:16 PM Greg Clayton <gclayton at apple.com>
>>> wrote:
>>>
>>>> They will only if you teach your platform to launch your processes. I
>>>> believe the Windows platform is the only platform that doesn't do process
>>>> launching via the platform. Am I wrong? If your platform supports it, it
>>>> should launch your process in a stopped state and then your Process plug-in
>>>> is simply asked to attach to that process.
>>>>
>>>> So if you wanted this to be supported, you will need to teach your
>>>> windows platform to support the PlatformWindows::GlobArguments() which
>>>> means you will launch the arg_dumper though cmd.exe and let the cmd.exe do
>>>> any glowing and any expansion that it wants to, and parse the output.
>>>>
>>>> Enrico will check in a method for PlatformPOSIX::GlobArguments() which
>>>> launches through a shell. I can't remember where we can get the default
>>>> shell, but I seem to remember that we have the default shell that we can
>>>> get from the host, so we could add this implementation in
>>>> "Platform::GlobArguments()" and if "IsHost()" returns true, get the default
>>>> shell from the host and launch the arg_dumper and then parse the output.
>>>>
>>>> This probably should be a method on ProcessLaunchInfo so that it fixes
>>>> up its arguments and uses the target's platform to do the glob expansion.
>>>>
>>>> So the changes would be:
>>>> 1 - add a method to ProcessLaunchInfo that gets passed a target and
>>>> allows it to fixup arguments and environment variables in a new
>>>> ProcessLaunchInfo:
>>>>     Error ProcessLaunchInfo::GlobArguments(Target &target,
>>>> ProcessLaunchInfo &new_launch_info);
>>>> 2 - if eLaunchFlagGlobArguments is set, then call the new
>>>> ProcessLaunchInfo::GlobArguments() function before doing any launches
>>>> 3 - Add a Platform::GlobArguments() function that, for the current
>>>> host, knows how to run the arg_dumper on the current host using the
>>>> Host::GetDefaultShell().
>>>>
>>>> Then we need to figure out what to do for remote platforms. I am not
>>>> sure we can guarantee there is an arg_dumper on the other side. For the
>>>> lldb-platform, we could ensure that it has access to arg_dumper and can run
>>>> it as a remote packet and return the results.
>>>>
>>>> Greg
>>>>
>>>> > On Feb 19, 2015, at 2:29 PM, Zachary Turner <zturner at google.com>
>>>> wrote:
>>>> >
>>>> > Enrico, will your planned changes address the original issue?
>>>> Namely, that
>>>> > if a process is launched through the process plugin with the
>>>> > eLaunchFlagGlobArguments flag set, after your changes will the
>>>> arguments
>>>> > correctly go through the globber first?
>>>> >
>>>> >
>>>> > http://reviews.llvm.org/D7743
>>>> >
>>>> > EMAIL PREFERENCES
>>>> >  http://reviews.llvm.org/settings/panel/emailpreferences/
>>>> >
>>>> >
>>>> >
>>>> > _______________________________________________
>>>> > lldb-commits mailing list
>>>> > lldb-commits at cs.uiuc.edu
>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>>
>>>>
>>
>>
>> --
>> Oleksiy Vyalov | Software Engineer | ovyalov at google.com
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>>
>> Thanks,
>> *- Enrico*
>> [image: 📩] egranata@.com ☎️ 27683
>>
>>
>>
>>
>>


-- 
Oleksiy Vyalov | Software Engineer | ovyalov at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150219/5be409fe/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: emoji_u1f4e9.png
Type: image/png
Size: 1028 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150219/5be409fe/attachment.png>