On Fri, Jun 30, 2017 at 12:13 PM, Lars Schneider
<larsxschneider@xxxxxxxxx> wrote:
>
>> On 30 Jun 2017, at 11:41, Miguel Torroja <miguel.torroja@xxxxxxxxx> wrote:
>>
>> On Fri, Jun 30, 2017 at 10:26 AM, Lars Schneider
>> <larsxschneider@xxxxxxxxx> wrote:
>>>
>>>> On 30 Jun 2017, at 00:46, miguel torroja <miguel.torroja@xxxxxxxxx> 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@xxxxxxxxx>
>>>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>>>> ---
>>>> ...
>>>
>>> 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?
>
The commands that are affected are the ones that don't use the -G
option, as everything is sent to the standard output without being
able to filter out what is the real contents or just info messages.
That's not the case with the python output (-G). Having said that... I
tried what you just said (just to be sure) and the function
p4_last_change fails... as it expects the first dictionary returned by
p4CmdList is the one that contains the change:
"int(results[0]['change'])" and that's not the case as it's an info
entry (no 'change' key, that's in the next entry...) I'll update with
new patches
I didn't notice that before because the P4 server we have in our
office only outputs extra info messages with the command "p4 change".
> - Lars
