Re: [PATCH] git-p4: parse marshal output "p4 -G" in p4 changes

> On 29 June 2017 at 23:41, miguel torroja <miguel.torroja@gmail.com> wrote:
>> On Thu, Jun 29, 2017 at 8:59 AM, Luke Diamand <luke@diamand.org> wrote:
>>> On 28 June 2017 at 14:14, miguel torroja <miguel.torroja@gmail.com> wrote:
>>>> Thanks Luke,
>>>>
>>>> regarding the error in t9800 (not ok 18 - unresolvable host in P4PORT
>>>> should display error), for me it's very weird too as it doesn't seem
>>>> to be related to this particular change, as the patch changes are not
>>>> exercised with that test.
>>>
>>> I had a look at this. The problem is that the old code uses
>>> p4_read_pipe_lines() which calls sys.exit() if the subprocess fails.
>>>
>>> But the new code calls p4CmdList() which has does error handling by
>>> setting "p4ExitCode" to a non-zero value in the returned dictionary.
>>>
>>> I think if you just check for that case, the test will then pass
>>
>> Thank you for debugging this,  I did as you suggested and it passed that test!
>>
>>>>
>>>> The test 21 in t9807 was precisely the new test added to test the
>>>> change (it was passing with local setup), the test log is truncated
>>>> before the output of test 21 in t9807 but I'm afraid I'm not very
>>>> familiar with Travis, so maybe I'm missing something. Is there a way
>>>> to have the full logs or they are always truncated after some number
>>>> of lines?
>>>
>>> For me, t9807 is working fine.
>>>
>>>>
>>>> I think you get an error with git diff --check because I added spaces
>>>> after a tab, but those spaces are intentional, the tabs are for the
>>>> "<<-EOF" and spaces are for the "p4 triggers" specificiation.
>>>
>>> OK.
>>>
>>
>> In the end, ,the reason t9807 was not passing was precisely the tabs
>> and spaces of the patch. the original patch had:
>> <tab><tab><spaces>....., as I explained, the tabs were supposed to be
>> ignored by "<<-EOF" and the spaces were supposed to be sent to stdin
>> of p4 triggers, but when the patch was applied to upstream the
>> <spaces> were substituted by tabs what led to a malformed  "p4
>> trigger" description. I just collapsed the description in one single
>> line and now it's passing
>>>
>>> Luke
>>
>>
>> I'm sending a new patch with the two changes I just mentioned.
>
> Looks good to me, Ack. Can we squash the two changes together?
>
> Luke