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

[Zachary Turner]

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()

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?
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150220/2204892e/attachment.html>