Re: [PATCH] git-p4: changelist template with p4 -G change -o
Hi Lars/Luke,
I tried a first test extending t9807-git-p4-submit.sh. I set this p4
trigger: 'p4test command pre-user-change "echo verbose trigger" '. I'm
able to reproduce the issue I wanted to fix. However I found yet
another issue, in this case when reading the result from
p4_read_pipe_lines (in function p4ChangesForPaths), the
pre-user-change is triggered with any "p4 change" and "p4 changes"
command (the p4 server we have in production, only shows "extra"
messages with p4 change).
I'll collapse in one single commit the fix for p4 change/p4 changes
and the new test.
Thanks,
Miguel
On Sat, Jun 24, 2017 at 10:37 PM, miguel torroja
wrote:
> Hi Lars,
>
> I think it's doable to set a custom p4 trigger, created by the test case,
> that outputs "extra info" when requesting a changelist description.
> I'll do a specific test and post it to this thread.
>
>
> Thanks,
>
>
> El 24 jun. 2017 7:36 p. m., "Lars Schneider"
> escribió:
>
>
>> On 24 Jun 2017, at 13:49, Luke Diamand wrote:
>>
>> On 22 June 2017 at 18:32, Junio C Hamano wrote:
>>> Miguel Torroja 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 plugin/hooks in the server side generates 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'}
>>
>> I think this seems like a reasonable thing to do if "p4 change -o" is
>> jumbling up output.
>>
>> One thing I notice trying it out by hand is that we seem to have lost
>> the annotation of the Perforce per-file modification type (is there a
>> proper name for this?).
>>
>> For example, if I add a file called "baz", then the original version
>> creates a template which looks like this:
>>
>> //depot/baz# add
>>
>> But the new one creates a template which looks like:
>>
>> //depot/baz
>
> @Miguel: You wrote that p4 plugins/hooks generate these warnings.
> I wonder if you see a way to replicate that in a test case. Either
> in t9800 or a new t98XX test case file?
>
> - Lars
>
>
Re: [PATCH] git-p4: changelist template with p4 -G change -o
> On 24 Jun 2017, at 13:49, Luke Diamand wrote:
>
> On 22 June 2017 at 18:32, Junio C Hamano wrote:
>> Miguel Torroja 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 plugin/hooks in the server side generates 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'}
>
> I think this seems like a reasonable thing to do if "p4 change -o" is
> jumbling up output.
>
> One thing I notice trying it out by hand is that we seem to have lost
> the annotation of the Perforce per-file modification type (is there a
> proper name for this?).
>
> For example, if I add a file called "baz", then the original version
> creates a template which looks like this:
>
> //depot/baz# add
>
> But the new one creates a template which looks like:
>
> //depot/baz
@Miguel: You wrote that p4 plugins/hooks generate these warnings.
I wonder if you see a way to replicate that in a test case. Either
in t9800 or a new t98XX test case file?
- Lars
Re: [PATCH] git-p4: changelist template with p4 -G change -o
You are right about the "# add"comment. I couldn't find any extra info
in the marshaled output that I can use to add the change action
comment after the path. That's one downside of that change.
On Sat, Jun 24, 2017 at 1:49 PM, Luke Diamand wrote:
> On 22 June 2017 at 18:32, Junio C Hamano wrote:
>> Miguel Torroja 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 plugin/hooks in the server side generates 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'}
>
> I think this seems like a reasonable thing to do if "p4 change -o" is
> jumbling up output.
>
> One thing I notice trying it out by hand is that we seem to have lost
> the annotation of the Perforce per-file modification type (is there a
> proper name for this?).
>
> For example, if I add a file called "baz", then the original version
> creates a template which looks like this:
>
>//depot/baz# add
>
> But the new one creates a template which looks like:
>
>//depot/baz
>
> Luke
Re: [PATCH] git-p4: changelist template with p4 -G change -o
On 22 June 2017 at 18:32, Junio C Hamano wrote:
> Miguel Torroja 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 plugin/hooks in the server side generates 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'}
I think this seems like a reasonable thing to do if "p4 change -o" is
jumbling up output.
One thing I notice trying it out by hand is that we seem to have lost
the annotation of the Perforce per-file modification type (is there a
proper name for this?).
For example, if I add a file called "baz", then the original version
creates a template which looks like this:
//depot/baz# add
But the new one creates a template which looks like:
//depot/baz
Luke
Re: [PATCH] git-p4: changelist template with p4 -G change -o
Miguel Torroja 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 plugin/hooks in the server side generates 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'}
>
> Signed-off-by: Miguel Torroja
> ---
Asking for help reviewing to those who have worked on git-p4 in the
past...
Thanks.
> git-p4.py | 77
> ++-
> 1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 8d151da..a300474 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1526,37 +1526,62 @@ class P4Submit(Command, P4UserMap):
>
> [upstream, settings] = findUpstreamBranchPoint()
>
> -template = ""
> +template = """\
> +# A Perforce Change Specification.
> +#
> +# Change: The change number. 'new' on a new changelist.
> +# Date:The date this specification was last modified.
> +# Client: The client on which the changelist was created. Read-only.
> +# User:The user who created the changelist.
> +# Status: Either 'pending' or 'submitted'. Read-only.
> +# Type:Either 'public' or 'restricted'. Default is 'public'.
> +# Description: Comments about the changelist. Required.
> +# Jobs:What opened jobs are to be closed by this changelist.
> +# You may delete jobs from this list. (New changelists only.)
> +# Files: What opened files from the default changelist are to be added
> +# to this changelist. You may delete files from this list.
> +# (New changelists only.)
> +"""
> +files_list = []
> inFilesSection = False
> +change_entry = None
> args = ['change', '-o']
> if changelist:
> args.append(str(changelist))
> -
> -for line in p4_read_pipe_lines(args):
> -if line.endswith("\r\n"):
> -line = line[:-2] + "\n"
> -if inFilesSection:
> -if line.startswith("\t"):
> -# path starts and ends with a tab
> -path = line[1:]
> -lastTab = path.rfind("\t")
> -if lastTab != -1:
> -path = path[:lastTab]
> -if settings.has_key('depot-paths'):
> -if not [p for p in settings['depot-paths']
> -if p4PathStartsWith(path, p)]:
> -continue
> -else:
> -if not p4PathStartsWith(path, self.depotPath):
> -continue
> +for entry in p4CmdList(args):
> +if not entry.has_key('code'):
> +continue
> +if entry['code'] == 'stat':
> +change_entry = entry
> +break
> +if not change_entry:
> +die('Failed to decode output of p4 change -o')
> +for key, value in change_entry.iteritems():
> +if key.startswith('File'):
> +if settings.has_key('depot-paths'):
> +if not [p for p in settings['depot-paths']
> +if p4PathStartsWith(value, p)]:
> +continue
> else:
> -inFilesSection = False
> -else:
> -if line.startswith("Files:"):
> -inFilesSection = True
> -
> -template += line
> -
> +if not p4PathStartsWith(value, self.depotPath):
> +continue
> +files_list.append(value)
> +continue
> +# Output in the order expected by prepareLogMessage
> +for key in ['Change','Client','User','Status','Description','Jobs']:
> +if not change_entry.has_key(key):
> +continue
> +template += '\n'
> +template += key + ':'
> +if key == 'Description':
> +template += '\n'
> +for field_line in change_entry[key].splitlines():
> +template += '\t'+field_line+'\n'
> +if len(files_list) > 0:
> +template += '\n'
> +template += 'Files:\n'
> +for path in files_list:
> +template += '\t'+path+'\n'
> return template
>
> def edit_template(self, template_file):
