On 11 July 2017 at 23:53, 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 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)
Is there any reason this change sets skip_info to False in this one
place, rather than defaulting to False (the original behavior) and
setting it to True where it's needed?
I worry that there might be other unexpected side effects in places
not covered by the tests.
Thanks
Luke
