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

[miguel torroja <miguel.torroja@gmail.com>]

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.

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?

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.

Thanks,


On Wed, Jun 28, 2017 at 11:54 AM, Luke Diamand <luke@diamand.org> wrote:
> On 28 June 2017 at 05:08, Junio C Hamano <gitster@pobox.com> wrote:
>> Miguel Torroja <miguel.torroja@gmail.com> writes:
>>
>>> The option -G of p4 (python marshal output) gives more context about the
>>> data being output. That's useful when using the command "change -o" as
>>> we can distinguish between warning/error line and real change description.
>>>
>>> Some p4 triggers in the server side generate some warnings when
>>> executed. Unfortunately those messages are mixed with the output of
>>> "p4 change -o". Those extra warning lines are reported as {'code':'info'}
>>> in python marshal output (-G). The real change output is reported as
>>> {'code':'stat'}
>>>
>>> A new test has been created to t9807-git-p4-submit.sh adding a p4 trigger
>>> that outputs extra lines with "p4 change -o" and "p4 changes"
>>>
>>> Signed-off-by: Miguel Torroja <miguel.torroja@gmail.com>
>>> ---
>>
>> It appears that https://travis-ci.org/git/git/builds/247724639
>> does not like this change.  For example:
>>
>>     https://travis-ci.org/git/git/jobs/247724642#L1848
>>
>> indicates that not just 9807 (new tests added by this patch) but
>> also 9800 starts to fail.
>>
>> I'd wait for git-p4 experts to comment and help guiding this change
>> forward.
>
> I only see a (very weird) failure in t9800. I wonder if there are some
> P4 version differences.
>
> Client: Rev. P4/LINUX26X86_64/2015.1/1024208 (2015/03/16).
> Server: P4D/LINUX26X86_64/2015.1/1028542 (2015/03/20)
>
> There's also a whitespace error according to "git diff --check".
> :
> Sadly I don't think there's any way to do this and yet keep the "#
> edit" comments. It looks like "p4 change -o" outputs lines with "'#
> edit" on the end, but the (supposedly semantically equivalent) "p4 -G
> change -o" command does not. I think that's a P4 bug.
>
> So we have a choice of fixing a garbled message in the face of scripts
> in the backend, or keeping the comments, or writing some extra Python
> to infer them. I vote for fixing the garbled message.
>
> Luke