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 in 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> --- git-p4.py | 92 ++++++++++++++++++++++++++++++++---------------- t/t9807-git-p4-submit.sh | 30 ++++++++++++++++ 2 files changed, 92 insertions(+), 30 deletions(-) diff --git a/git-p4.py b/git-p4.py index 8d151da91..1facf32db 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 @@ -1494,7 +1501,7 @@ class P4Submit(Command, P4UserMap): c['User'] = newUser input = marshal.dumps(c) - result = p4CmdList("change -f -i", stdin=input) + result = p4CmdList("change -f -i", stdin=input,skip_info=False) for r in result: if r.has_key('code'): if r['code'] == 'error': @@ -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