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

From: miguel torroja <miguel.torroja@gmail.com>
Date: Jun 28, 2017, 13:14 UTC
Message-ID: <CAKYtbVaLkt6_rFgehgSsrLzo-oO3sEVoMLBtS5XX59ymYYS7=w@mail.gmail.com>
In-Reply-To: <CAE5ih78VwBVT+XHnwgnt-JcLB-c4d_Gf+9Wfb_bL=LcgkjDrUQ@mail.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:

Show 48 quoted lines

> 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