P4: Add support for json output #38
Conversation
Simple json output support for p4 commands. A few handful functions: - reconcile current workspace in default cl - submit current default cl - get a file current revision in the workspace - print file content by revision
nimp/utils/p4.py
Outdated
| command = self._get_p4_command('submit', '-f', 'revertunchanged') | ||
| # descriptions could be too long for perforce limitations | ||
| command_length = len(' '.join(command)) | ||
| perforce_cmd_max_characters_limit = 4096 |
There was a problem hiding this comment.
Is this a real Perforce limitation or a windows commandline one ?
If it's a windows one, you can look into the p4 -x global option which allow providing arg in a file to by-pass this.
There was a problem hiding this comment.
I don't think the -x option can help us there.
My understanding from reading the docs it taht the p4 -x file_with_stuff.txt submit -m submit -d message would submit the list of files in file_with_stuff. This provides no workarround to the commandline length since we cannot pass the submit description as a file containing the desc.
4096 seems to be a bogus number (got it from pergit), 8191 seems to be the number for a cmd env.
I found no mention of a limiation to the submit description in the perforce documentation however.
What do you think?
Should we try and not limit the message length?
Or maybe expose it as an optionnal param for users of this function to handle?
There was a problem hiding this comment.
Just tried with a p4 -x test.txt print
with test.txt:
-q
//path/to/file
and it worked as expected.
I think it's worth looking into and always using a file for submit to avoid escape hell to handle quotes and such in the description text.
It returns None if the file is not synced.
revertunchanged is optionnal Remove useless assert Replace 4096 limit by 8091 cmd limit
nimp/utils/p4.py
Outdated
| command = self._get_p4_command('submit', '-f', 'revertunchanged') | ||
| # descriptions could be too long for perforce limitations | ||
| command_length = len(' '.join(command)) | ||
| perforce_cmd_max_characters_limit = 4096 |
There was a problem hiding this comment.
Just tried with a p4 -x test.txt print
with test.txt:
-q
//path/to/file
and it worked as expected.
I think it's worth looking into and always using a file for submit to avoid escape hell to handle quotes and such in the description text.
| # anything under that seems to be fine, whatever happened to this 1024 bytes limit... | ||
| # couldn't find more info on this subject in the perforce documentation | ||
| description_limit = 120000 | ||
| submit_args.extend(['-d', description[:description_limit]]) |
There was a problem hiding this comment.
Since the -x arg expect a file with one arg per line.
Should we escape the line endings (\n, \r\n, etc) from the desc?
There was a problem hiding this comment.
You're right.
Removing \n solves the issue. I haven't found way a to esacpe them though, while conserving line breaks.
There was a problem hiding this comment.
We're now using the utils.p4.submit(description=p4_desc):
- It should avoid the escaping hell issue by just updating the default changelist spec before submitting.
- I'll test some commit messages from our monorepo though to be sure nothing weird happens
The p4 print function can output the contents of path/to/pattern* We want this to be a handy function to print a given file data.
It lets us submit default cl from sractch as well as submitting knwon changelists (while still being able to update cl desc)
For better compatibility across systems
Any backslash would be interpreted as a regex. Use replacing method rather than re.sub to preserve initial desc.
|
|
||
| def print_file_data(self, file, revision=None): | ||
| ''' wrapper for p4 print command ''' | ||
| revision = f"#{revision}" if revision is not None else '' |
There was a problem hiding this comment.
I think it'd be better to leave this up to called as you can use the @file_rev pattern with print
| output = [json.loads(json_element) for json_element in output.splitlines() if json_element] | ||
| for output_chunk in output: | ||
| if 'data' in output_chunk: | ||
| data = output_chunk['data'] |
There was a problem hiding this comment.
Missing the pattern where print split the file content into multiple {'data': ...} blocks for some reason
| if use_ztag: | ||
| command += ['-z', 'tag'] | ||
| if use_json_format: | ||
| command.append('-Mj') |
There was a problem hiding this comment.
It's recommended to always use -ztag with -Mj
| matches = re.findall(pattern, cl_spec) | ||
| if not matches: | ||
| # It's possible that there is no following field | ||
| pattern = re.compile(rf'{spec_field}:{os.linesep}\t(.*)({os.linesep}{os.linesep})', re.DOTALL) |
There was a problem hiding this comment.
This could be achieved with a $ to the possible_following_fields like such ({possible_following_fields}|$)
Simple json output support for p4 commands.
A few handful functions: