On 3 July 2017 at 23:57, 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'}
>
> the function p4CmdList accepts a new argument: skip_info. When set to
> True it ignores any 'code':'info' entry (skip_info=True by default).
>
> 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"
The latest version of mt/p4-parse-G-output (09521c7a0) seems to break
t9813-git-p4-preserve-users.sh.
I don't quite know why, but I wonder if it's the change to p4CmdList() ?
Luke
>
> Signed-off-by: Miguel Torroja <miguel.torroja@xxxxxxxxx>
> ---
> git-p4.py | 90 ++++++++++++++++++++++++++++++++----------------
> t/t9807-git-p4-submit.sh | 30 ++++++++++++++++
> 2 files changed, 91 insertions(+), 29 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 8d151da91..a262e3253 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -509,7 +509,7 @@ def isModeExec(mode):
> def isModeExecChanged(src_mode, dst_mode):
> return isModeExec(src_mode) != isModeExec(dst_mode)
>
> -def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None):
> +def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=True):
>
> if isinstance(cmd,basestring):
> cmd = "-G " + cmd
> @@ -545,6 +545,9 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None):
> try:
> while True:
> entry = marshal.load(p4.stdout)
> + if skip_info:
> + if 'code' in entry and entry['code'] == 'info':
> + continue
> if cb is not None:
> cb(entry)
> else:
> @@ -879,8 +882,12 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
> cmd += ["%s...@%s" % (p, revisionRange)]
>
> # Insert changes in chronological order
> - for line in reversed(p4_read_pipe_lines(cmd)):
> - changes.add(int(line.split(" ")[1]))
> + for entry in reversed(p4CmdList(cmd)):
> + if entry.has_key('p4ExitCode'):
> + die('Error retrieving changes descriptions ({})'.format(entry['p4ExitCode']))
> + if not entry.has_key('change'):
> + continue
> + changes.add(int(entry['change']))
>
> if not block_size:
> break
> @@ -1526,37 +1533,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):
> diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
> index 3457d5db6..b630895a7 100755
> --- a/t/t9807-git-p4-submit.sh
> +++ b/t/t9807-git-p4-submit.sh
> @@ -409,6 +409,36 @@ test_expect_success 'description with Jobs section and bogus following text' '
> )
> '
>
> +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
> + ) &&
> + (
> + p4 change -o | grep -s "verbose trigger"
> + ) &&
> + (
> + cd "$git" &&
> + git config git-p4.skipSubmitEdit true &&
> + echo file20 >file20 &&
> + git add file20 &&
> + git commit -m file20 &&
> + git p4 submit
> + ) &&
> + (
> + p4 triggers -i <<-EOF
> + Triggers:
> + EOF
> + ) &&
> + (
> + cd "$cli" &&
> + test_path_is_file file20
> + )
> +'
> +
> test_expect_success 'submit --prepare-p4-only' '
> test_when_finished cleanup_git &&
> git p4 clone --dest="$git" //depot &&
> --
> 2.11.0
>
