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

Thu Feb 19 16:42:53 PST 2015

> On 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 don’t think LLDB should be in the business of trying to pretend to be a shell. The magic of argdumper is that it gets launched via a shell, so the shell does all the magic expansion for us, and we just dump our list of arguments
If you wanted to emulate that, you’d essentially be rewriting the bash command-line, nuances, corner cases, caveats and everything
There is a program already that does all that, it’s called a shell

Also, I am not sure there is a universal all-platforms notion of globbing. IIRC, on Windows, environment is referenced as %ENVVAR%, not $ENVVAR. You’d want to preserve those platform-specific conventions.

>   I mean it might be an ugly function, but the idea of passing stuff to an external program is kinda meh.

Why?

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

PlatformWindows could very well decide that it does not support globbing if this is such a big deal.

> 
> On Thu Feb 19 2015 at 4:30:24 PM Enrico Granata <egranata at apple.com <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <http://reviews.llvm.org/D7743>
>> >
>> > EMAIL PREFERENCES
>> >  http://reviews.llvm.org/settings/panel/emailpreferences/ <http://reviews.llvm.org/settings/panel/emailpreferences/>
>> >
>> >
>> >
>> > _______________________________________________
>> > lldb-commits mailing list
>> > lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits <http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits>
>> 
>> 
>> 
>> 
>> -- 
>> Oleksiy Vyalov | Software Engineer | ovyalov at google.com <mailto:ovyalov at google.com>_______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits <http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits>
> Thanks,
> - Enrico
> 📩 egranata@.com ☎️ 27683
> 
> 
> 
> 

Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150219/15737779/attachment.html>