"Ben Keene via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Ben Keene <seraphire@xxxxxxxxx>
>
> When applying a commit fails because of RCS keywords, Git
> will fail the P4 submit. It would help the user if Git suggested that
> the user set git-p4.attemptRCSCleanup to true.
>
> Change the applyCommit() method that when applying a commit fails
> becasue of the P4 RCS Keywords, the user should consider setting
s/becasue/because/
> git-p4.attemptRCSCleanup to true.
The above explains the new "else:" clause really well. The
original's logic was to
- tryPatchCmd to apply a commit, which might fail,
- when the above fails, only if attemptrcscleanup is set, munge
the lines with rcs keywords and rerun tryPatchCmd
but your new "else:" gives a suggestion to use the (experimental?)
attemptRCSCleanup feature.
However, it does not explain the change to the "if :" clause. I can
see that patchRCSKeywords() method does want to raise an exception,
and it is a good idea to prepare for the case and clean up the mess
it may create. At least that deserves a mention in the proposed log
message---I actually think that the new try/except is an equally
important improvement that deserves to be a separate patch.
> Signed-off-by: Ben Keene <seraphire@xxxxxxxxx>
> ---
> git-p4.py | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 0fa562fac9..856fe82079 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1950,8 +1950,23 @@ def applyCommit(self, id):
> # disable the read-only bit on windows.
> if self.isWindows and file not in editedFiles:
> os.chmod(file, stat.S_IWRITE)
> - self.patchRCSKeywords(file, kwfiles[file])
> - fixed_rcs_keywords = True
> +
> + try:
> + self.patchRCSKeywords(file, kwfiles[file])
> + fixed_rcs_keywords = True
> + except:
> + # We are throwing an exception, undo all open edits
> + for f in editedFiles:
> + p4_revert(f)
> + raise
> + else:
> + # They do not have attemptRCSCleanup set, this might be the fail point
> + # Check to see if the file has RCS keywords and suggest setting the property.
> + for file in editedFiles | filesToDelete:
> + if p4_keywords_regexp_for_file(file) != None:
> + print("At least one file in this commit has RCS Keywords that may be causing problems. ")
> + print("Consider:\ngit config git-p4.attemptRCSCleanup true")
> + break
>
> if fixed_rcs_keywords:
> print("Retrying the patch with RCS keywords cleaned up")
