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

>
>> On 30 Jun 2017, at 11:41, Miguel Torroja <miguel.torroja@gmail.com> wrote:
>>
>> On Fri, Jun 30, 2017 at 10:26 AM, Lars Schneider
>> <larsxschneider@gmail.com> wrote:
>>>
>>>> On 30 Jun 2017, at 00:46, miguel torroja <miguel.torroja@gmail.com> wrote:
>>>>
>>>> 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>
>>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>>> ---
>>>> ...
>>>
>>> I have never worked with p4 triggers and that might be
>>> the reason why I don't understand your test case.
>>> Maybe you can help me?
>>>
>>>> +test_expect_success 'description with extra lines from verbose p4 trigger' '
>>>> +     test_when_finished cleanup_git &&
>>>> +     git p4 clone --dest="$git" //depot &&
>>>> +     (
>>>> +             p4 triggers -i <<-EOF
>>>> +             Triggers: p4triggertest-command command pre-user-change "echo verbose trigger"
>>>> +             EOF
>>>> +     ) &&
>>>
>>> You clone the test repo and install a trigger.
>>>
>>>> +     (
>>>> +             cd "$git" &&
>>>> +             git config git-p4.skipSubmitEdit true &&
>>>> +             echo file20 >file20 &&
>>>> +             git add file20 &&
>>>> +             git commit -m file20 &&
>>>> +             git p4 submit
>>>> +     ) &&
>>>
>>> You make a new commit. This should run the "echo verbose trigger", right?
>>
>> Yes, that's correct. In this case the trigger is run with p4 change
>> and p4 changes
>>
>>>
>>>> +     (
>>>> +             p4 triggers -i <<-EOF
>>>> +             Triggers:
>>>> +             EOF
>>>> +     ) &&
>>>
>>> You delete the trigger.
>>>
>>>> +     (
>>>> +             cd "$cli" &&
>>>> +             test_path_is_file file20
>>>> +     )
>>>
>>> You check that the file20 is available in P4.
>>>
>>>
>>> What would happen if I run this test case without your patch?
>>> Wouldn't it pass just fine?
>>
>> If you run it without the patch for git-p4.py, the test doesn't pass
>
> You are right. I did not run "make" properly before running the test :)
>
>
>>> Wouldn't we need to check that no warning/error is in the
>>> real change description?
>>>
>>
>> that can also be added, something like this: 'p4 change -o | grep
>> "verbose trigger"' after setting the trigger?
>
> Yeah, maybe. I hope this is no stupid question, but: If you clone the
> repo with git-p4 *again* ... would you see the "verbose trigger" output
> in the Git commit message?
>