This revision was automatically updated to reflect the committed changes.
Closed by commit rL322935: Remove Platform references from the Host module
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41902?vs=129435&i
On 11 January 2018 at 22:42, Davide Italiano wrote:
> The test Jim committed broke the public bots, so I went ahead and reverted it.
>
>
> Committing to https://llvm.org/svn/llvm-project/lldb/trunk ...
> commit 87eb2de0885d646e71d5848c1fa699b784bf5d2b
> Author: Davide Italiano
> Date: Thu Jan 1
The test Jim committed broke the public bots, so I went ahead and reverted it.
Committing to https://llvm.org/svn/llvm-project/lldb/trunk ...
commit 87eb2de0885d646e71d5848c1fa699b784bf5d2b
Author: Davide Italiano
Date: Thu Jan 11 14:37:49 2018 -0800
[testsuite] Remove a broken test which
On 11 January 2018 at 16:22, Greg Clayton wrote:
>
>> On Jan 11, 2018, at 8:10 AM, Pavel Labath wrote:
>>
>> On 11 January 2018 at 16:04, Greg Clayton wrote:
>>>
On Jan 11, 2018, at 5:12 AM, Pavel Labath wrote:
Thanks for adding the test Jim. I have confirmed that it passes with
> On Jan 11, 2018, at 8:10 AM, Pavel Labath wrote:
>
> On 11 January 2018 at 16:04, Greg Clayton wrote:
>>
>>> On Jan 11, 2018, at 5:12 AM, Pavel Labath wrote:
>>>
>>> Thanks for adding the test Jim. I have confirmed that it passes with
>>> this patch applied (because the bundle is resolved
On 11 January 2018 at 16:04, Greg Clayton wrote:
>
>> On Jan 11, 2018, at 5:12 AM, Pavel Labath 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 di
In that case I think Platform should implement all the logic it needs in
itself, instead of relying Host to callback into it.
I think the point of this patch is to separate dependencies, so Platform
should be able to depend on Host, but not vice versa.
On Thu, Jan 11, 2018 at 8:05 AM Greg Clayton
> On Jan 11, 2018, at 5:12 AM, Pavel Labath 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
> th
labath updated this revision to Diff 129435.
labath added a comment.
Add back bundle resolution to the mac launcher
https://reviews.llvm.org/D41902
Files:
source/Host/common/MonitoringProcessLauncher.cpp
source/Host/freebsd/Host.cpp
source/Host/macosx/Host.mm
source/Host/netbsd/Host.cpp
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
Eh, no, that wasn't right. I don't know how to build and link a mach-o binary
on some random other system. So I made this a Darwin only test till I can
figure out how to do that.
Jim
> On Jan 10, 2018, at 3:09 PM, Jim Ingham wrote:
>
> I added a simple test: macosx/find-app-in-bundle. On
I added a simple test: macosx/find-app-in-bundle. On non-Darwin systems it
just ensures we find the app in the
app bundle and can set a breakpoint in it. On Darwin, it also ensures we can
launch the app and hit our breakpoint.
When I get a chance I'll add an iOS app bundle and make a tricky on
App bundles are "just directories" but they are actually different on iOS & OS
X. The most interesting part of them is a plist that gives some information
about the bundle. lldb reads that plist to figure out what the real executable
is (it is usually the bundle name minus the .app, but it doe
On Wed, Jan 10, 2018 at 2:09 PM Jim Ingham wrote:
> The only hard part of writing any kind of test for this is actually
> getting a legitimate .app into the testsuite. Doesn't seem fair to ask
> Pavel to do that, since he doesn't work on macOS...
>
> Jim
>
What exactly *is* a .app file on disk?
I understand, but having components untested is not ideal :(
On Wed, Jan 10, 2018 at 2:09 PM, Jim Ingham wrote:
> The only hard part of writing any kind of test for this is actually getting a
> legitimate .app into the testsuite. Doesn't seem fair to ask Pavel to do
> that, since he doesn't wo
The only hard part of writing any kind of test for this is actually getting a
legitimate .app into the testsuite. Doesn't seem fair to ask Pavel to do that,
since he doesn't work on macOS...
Jim
> On Jan 10, 2018, at 1:59 PM, Davide Italiano via Phabricator
> wrote:
>
> davide added a comm
davide added a comment.
In https://reviews.llvm.org/D41902#972619, @zturner wrote:
> In https://reviews.llvm.org/D41902#972614, @clayborg wrote:
>
> > As long as:
> >
> > % lldb /path/to/Foo.app
> > (lldb) r
> >
> >
> > Still works, then I am fine with this. The resolve executable should find
zturner added a comment.
In https://reviews.llvm.org/D41902#972614, @clayborg wrote:
> As long as:
>
> % lldb /path/to/Foo.app
> (lldb) r
>
>
> Still works, then I am fine with this. The resolve executable should find the
> executable down inside the app bundle (like
> "/path/to/Foo.app/Con
clayborg added a comment.
As long as:
% lldb /path/to/Foo.app
(lldb) r
Still works, then I am fine with this. The resolve executable should find the
executable down inside the app bundle (like
"/path/to/Foo.app/Contents/MacOS/Foo" for desktop apps and
"/path/to/Foo.app/Foo" for iOS apps).
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
This LGTM but please wait for a second opinion.
https://reviews.llvm.org/D41902
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://
labath created this revision.
labath added reviewers: jingham, clayborg.
Herald added a subscriber: emaste.
These were used by Host::LaunchProcess to "resolve" the executable it
was about to launch. The only parts of Platform::ResolveExecutable, which
seem to be relevant here are the FileSpec::Res
21 matches
Mail list logo
