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

Greg Clayton gclayton at apple.com
Fri Feb 20 09:47:36 PST 2015

> On Feb 19, 2015, at 7:55 PM, Zachary Turner <zturner at google.com> wrote:
> Also, it seems to me there should be a function in Host called ShellExpand(). On Mac or whichever other platform wanted to, it could call in to argdumper. Other platforms could implement this some other way if they wanted to.  This way, after checking whether the flag is set, Platform would just need to call Host::ShellExpand()

Agreed. We actually should:
1 - Change the eLaunchFlagGlobArguments to be eLaunchFlagShellExpansion
2 - Change all GlobArguments calls to be ExpandShellArguments
3 - Add a Host::ExpandShellArguments() that takes a ProcessLaunchInfo and returns a new one

Another options is to keep eLaunchFlagGlobArguments and make that do what Zachary was talking about: internal glob expansion that would be consistent between all LLDB releases, and let eLaunchFlagShellExpansion perform shell expansion on each system differently.

> On Thu, Feb 19, 2015 at 5:57 PM Zachary Turner <zturner at google.com> wrote:
> I'm ok with this as long as we don't actually rely on globbing in any tests except for tests which are explicitly platform tests.  It's currently difficult to enforce this, because LaunchWithGlobbing is just an argument that anyone can specify from anywhere to the ProcessLaunchInfo.  And as a result of this design choice, you end up with situations where certain codepaths that aren't immediately clear to the external user ignore the launch flags because the codepath didn't make its way through the Platform.
> Even the original test written for argdumper had this problem, where you could specify the flag to launch with argdumper, and then because the code decided to take a different path, it would ignore the argument and the test would fail.  So, if we can:
> a) write the original argdumper test against SBPlatform, and not against any other public class
> b) agree that argdumper should not be used in future tests unless it is a test against SBPlatform
> c) If at all possible, make this something other than a generic process launch flag, since it is possible for LLDB to disrespect it without warning or error if you go through the process plugin
> d) If c is not possible, fix the case of launching a process through the process plugin so that it will ask the platform if globbing is supported, and if it says no, it will fail the launch.
> Then perhaps this is ok
> On Thu Feb 19 2015 at 5:55:16 PM Zachary Turner <zturner at google.com> wrote:
> On Thu Feb 19 2015 at 5:43:55 PM Greg Clayton <gclayton at apple.com> wrote:
> >
> > That which will be part of testing the platform-specific bits of your platform
> >
> >  There's inherent value in having as few platform specific bits as possible.  When the same rules apply to every platform,
> > a) everyone gets better test coverage
> > b) everyone gets to benefit from everyone else's changes and bugfixes
> > c) as a user, it makes it easier and more comfortable to switch platforms since you don't have to do a mental switch.
> > d) It makes the code easier to maintain
> With the detriment that glowing might not behave like the shell does on your system. It would be nice to also support other shells (tcsh, zsh, ksh, etc) so that things would expand as the user expects them to.
> We already have SBPlatformShellCommand for that right?

More information about the lldb-commits mailing list