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

From: Luke Diamand <luke@diamand.org>
Date: Jun 28, 2017, 09:54 UTC
Message-ID: <CAE5ih78VwBVT+XHnwgnt-JcLB-c4d_Gf+9Wfb_bL=LcgkjDrUQ@mail.gmail.com>
In-Reply-To: <xmqqk23wycso.fsf@gitster.mtv.corp.google.com>

On 28 June 2017 at 05:08, Junio C Hamano <gitster@pobox.com> wrote:

Show 28 quoted lines

> 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