| Message ID | d608f529a0e01e99c97e895ab483000da068a7ac.1575901009.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-p4: Usability enhancements | expand |
"Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Ben Keene <seraphire@gmail.com> > > 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@gmail.com> > --- > 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")
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")