Re: [PATCH] git-p4: Handle p4 submit failure
Hello, > Note: this time, you do not need to resend the patch; just > please let me know if you want me to do the equivalent of > the above while applying to make your murex address and name > appear as the author in "git log" and "git shortlog" output. I'd like my murex address to appear on the log please, if it is not too much trouble. Thank you for all these tips on the submitting process. >> The p4 submit command may fail, for example if the changelist contains >> a job that does not exist in the Jobs section. If this is the case, >> p4_write_pipe will exit the script. This change makes it so that the >> workspace is reverted before letting the script die. > > Some of the information contained in this paragraph deserves to be > in the log message proper. How about > > From: GIRARD Etienne > Subject: git-p4: clean up after p4 submit failure > > When "p4 submit" command fails in P4Submit.applyCommit, the > workspace is left with the changes. We already have a code > to revert the changes to the workspace when the user decides > to cancel submission by aborting the editor that edits the > change description, and we should treat the "p4 submit" > failure the same way. > > Clean the workspace if p4_write_pipe raised SystemExit, > so that the user don't have to do it themselves. > > Signed-off-by: GIRARD Etienne > > or something like that? It seems like a good description. Please let me know if I should submit another patch with the proper log message > > While trying to come up with the above description, I started > wondering if the error message wants to differentiate the two cases. > > When self.edit_template() returns false, we know that the user > actively said "no I do not want to submit", and "Submission > cancelled" is perfectly fine, but when "p4 submit" fails because it > did not like the change description or if it found some other > issues, it is not necessarily that the user said "I want to cancel"; > it is more like "Submission failed". Yes, however if `p4 submit` fails the corresponding "Command failed" error message is displayed, and the p4 error message itself is displayed if any. Tthe script will also terminate successfully if self.edit_template returns false but it will exit with error code 1 if p4 submit fails. So the user will get "Command failed: [...]" followed by "Submission cancelled, undoing p4 changes", to let him know that the script failed because of p4 and that nothing was submitted. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: Handle p4 submit failure
Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.
Signed-off-by: GIRARD Etienne
---
git-p4.py | 71 +--
1 file changed, 37 insertions(+), 34 deletions(-)
The p4 submit command may fail, for example if the changelist contains
a job that does not exist in the Jobs section. If this is the case,
p4_write_pipe will exit the script. This change makes it so that the
workspace is reverted before letting the script die.
diff --git a/git-p4.py b/git-p4.py
index 0093fa3..d535904 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1538,44 +1538,47 @@ class P4Submit(Command, P4UserMap):
#
# Let the user edit the change description, then submit it.
#
-if self.edit_template(fileName):
-# read the edited message and submit
-ret = True
-tmpFile = open(fileName, "rb")
-message = tmpFile.read()
-tmpFile.close()
-if self.isWindows:
-message = message.replace("\r\n", "\n")
-submitTemplate = message[:message.index(separatorLine)]
-p4_write_pipe(['submit', '-i'], submitTemplate)
-
-if self.preserveUser:
-if p4User:
-# Get last changelist number. Cannot easily get it from
-# the submit command output as the output is
-# unmarshalled.
-changelist = self.lastP4Changelist()
-self.modifyChangelistUser(changelist, p4User)
-
-# The rename/copy happened by applying a patch that created a
-# new file. This leaves it writable, which confuses p4.
-for f in pureRenameCopy:
-p4_sync(f, "-f")
+submitted = False
-else:
+try:
+if self.edit_template(fileName):
+# read the edited message and submit
+tmpFile = open(fileName, "rb")
+message = tmpFile.read()
+tmpFile.close()
+if self.isWindows:
+message = message.replace("\r\n", "\n")
+submitTemplate = message[:message.index(separatorLine)]
+p4_write_pipe(['submit', '-i'], submitTemplate)
+
+if self.preserveUser:
+if p4User:
+# Get last changelist number. Cannot easily get it from
+# the submit command output as the output is
+# unmarshalled.
+changelist = self.lastP4Changelist()
+self.modifyChangelistUser(changelist, p4User)
+
+# The rename/copy happened by applying a patch that created a
+# new file. This leaves it writable, which confuses p4.
+for f in pureRenameCopy:
+p4_sync(f, "-f")
+submitted = True
+
+finally:
# skip this patch
-ret = False
-print "Submission cancelled, undoing p4 changes."
-for f in editedFiles:
-p4_revert(f)
-for f in filesToAdd:
-p4_revert(f)
-os.remove(f)
-for f in filesToDelete:
-p4_revert(f)
+if not submitted:
+print "Submission cancelled, undoing p4 changes."
+for f in editedFiles:
+p4_revert(f)
+for f in filesToAdd:
+p4_revert(f)
+os.remove(f)
+for f in filesToDelete:
+p4_revert(f)
os.remove(fileName)
-return ret
+return submitted
# Export git tags as p4 labels. Create a p4 label and then tag
# with that.
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ancestor and descendant ~ clarification needed
Hello, I think you're right, branch A is a descendant of master. We could change the misleading sentence to "However, if the current branch is a descendant of the other - if its head is a descendant of the other's head - [...]", to link back to the definition of descendant for commits. 2015-10-22 11:06 GMT+02:00 Xue Fuqiao : > Hi, > > In Documentation/user-manual.txt: > >In the following, we say that commit X is "reachable" from commit Y >if commit X is an ancestor of commit Y. Equivalently, you could say >that Y is a descendant of X, or that there is a chain of parents >leading from commit Y to commit X. > [...] >However, if the current branch is a descendant of the other--so every >commit present in the one is already contained in the other--then Git >just performs a "fast-forward"; the head of the current branch is >moved forward to point at the head of the merged-in branch, without >any new commits being created. > > I'm a Git newbie. According to my understanding, the "descendant" in > the second paragraph above should be "ancestor". I attempt to represent > my understanding using the following diagram (please see it in a > monospaced font): > > > > o--o--o <-- Branch A > / > o--o--o <-- master > > > > "master" is the current branch, and (as I understand it) it is an > ancestor of "Branch A", because there is a chain of parents leading from > "Branch A" to master. So "Branch A" (i.e., "the other" branch, or the > "merged-in" branch) is a descendant of master. I even set up a test > repository and attempted to test the above diagram with "git merge-base > --is-ancestor" (and "echo $?"), but it seems to me that the master > branch is *not* a descendant of "Branch A". > > I hope you can understand my words here (English is not my native > language). Can anyone point me in the right direction (what am I > missing)? > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: import the ctypes module
I was wrong, the script doesn't work on my machine if ctypes is not imported regardless of python version. I guess I was confused by using a version of git-p4 before ctypes was introduced, the failing version and the patched version, as well as several python versions. Sorry for this misleading claim, and thanks for the quick fix. 2015-10-21 10:23 GMT+02:00 Etienne Girard : > Hello, > > I couldn't work further on this yesterday (but I read > Documentation/SubmittingPatches, which is a good start I guess). The > diff proposed by Dennis works on my machine, I'll try to figure out > why the original script worked with 2.7.10. > > Thanks > > 2015-10-21 1:00 GMT+02:00 Luke Diamand : >> On 20/10/15 20:36, Junio C Hamano wrote: >>> >>> Dennis Kaarsemaker writes: >>> >>>>> I do not follow Python development, but does the above mean that >>>>> with recent 2.x you can say ctypes without first saying "import >>>>> ctypes"? It feels somewhat non-pythonesque that identifiers like >>>>> this is given to you without you asking with an explicit 'import', >>>>> so I am puzzled. >>>> >>>> >>>> No, you cannot do that. The reason others may not have noticed this bug >>>> is that >>>> in git-p4.py, ctypes is only used on windows. >>>> >>>> 111 if platform.system() == 'Windows': >>>> 112 free_bytes = ctypes.c_ulonglong(0) >>>> 113 >>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), >>>> None, None, ctypes.pointer(free_bytes)) >>>> >>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that >>>> it's >>>> on the same system). >>> >>> >>> Exactly. That is where my "I am puzzled" comes from. >>> >>> The patch looks obviously the right thing to do. Luke? Lars? >> >> >> It looks sensible to me, and works fine on Linux, thanks. ack. >> >> I can't test on Windows today but I can't see why it wouldn't work. >> >> Luke >> >> -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: import the ctypes module
Hello, I couldn't work further on this yesterday (but I read Documentation/SubmittingPatches, which is a good start I guess). The diff proposed by Dennis works on my machine, I'll try to figure out why the original script worked with 2.7.10. Thanks 2015-10-21 1:00 GMT+02:00 Luke Diamand : > On 20/10/15 20:36, Junio C Hamano wrote: >> >> Dennis Kaarsemaker writes: >> I do not follow Python development, but does the above mean that with recent 2.x you can say ctypes without first saying "import ctypes"? It feels somewhat non-pythonesque that identifiers like this is given to you without you asking with an explicit 'import', so I am puzzled. >>> >>> >>> No, you cannot do that. The reason others may not have noticed this bug >>> is that >>> in git-p4.py, ctypes is only used on windows. >>> >>> 111 if platform.system() == 'Windows': >>> 112 free_bytes = ctypes.c_ulonglong(0) >>> 113 >>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), >>> None, None, ctypes.pointer(free_bytes)) >>> >>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that >>> it's >>> on the same system). >> >> >> Exactly. That is where my "I am puzzled" comes from. >> >> The patch looks obviously the right thing to do. Luke? Lars? > > > It looks sensible to me, and works fine on Linux, thanks. ack. > > I can't test on Windows today but I can't see why it wouldn't work. > > Luke > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git-p4 fails with NameError with python 2.7.2
Hello, Git-p4 fail when I try to rebase with the error: "NameError: global name 'ctypes' is not defined". The error occurs when I use python 2.7.2 that is installed by default on my company's computers (it goes without saying that everything works fine with python 2.7.10). I'm a beginner in python, but simply importing ctypes at the beginning of the script does the trick. I was wondering if submitting a patch for this issue is worth the trouble, when a satisfying solution is not using a 4 years old version of python. Best regards -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
