From: Pete Wyckoff <pw@padd•com>
To: Michael Horowitz <michael.horowitz@ieee•org>
Cc: Luke Diamand <luke@diamand•org>,
"L. A. Linden Levy" <alevy@mobitv•com>,
git@vger•kernel.org, Junio C Hamano <gitster@pobox•com>
Subject: [PATCH] git-p4: fix skipSubmitEdit regression
Date: Sat, 17 Dec 2011 12:39:03 -0500 [thread overview]
Message-ID: <20111217173903.GA13674@padd.com> (raw)
In-Reply-To: <CAFLRboqJAC0h27m=B9Tw5SFcupEgn4fe9YvWksgqxOVs20nFHw@mail.gmail.com>
Commit 7c766e5 (git-p4: introduce skipSubmitEdit, 2011-12-04)
made it easier to automate submission to p4, but broke the most
common case.
Add a test for when the user really does edit and save the change
template, and fix the bug that causes the test to fail.
Also add a confirmation message when submission is cancelled.
Reported-by: Michael Horowitz <michael.horowitz@ieee•org>
Signed-off-by: Pete Wyckoff <pw@padd•com>
---
michael.horowitz@ieee•org wrote on Fri, 16 Dec 2011 19:49 -0500:
> Oh, and in the case where you do edit the template, it doesn't give
> you an error or anything, it looks like it succeeded, but you'll
> notice the change never got submitted to Perforce. If you look
> carefully though, you can see it reverting each of your edited files
> in the P4 tree.
[..]
> >> On 16/12/11 15:38, Michael Horowitz wrote:
> >>>
> >>> It appears that this change has introduced a bug that causes submit to
> >>> fail every time if you do not skip the submit edit.
> >>>
> >>> From what I can tell, this is because the new edit_template method
> >>> does not return True at the end.
Oops. In adding this code, I failed to test what should be the
normal code path. How sad.
Junio: this bug is in master. Could you apply this fix?
-- Pete
contrib/fast-import/git-p4 | 18 +++++++++++-------
t/t9805-skip-submit-edit.sh | 24 +++++++++++++++++++++++-
2 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3571362..d501eac 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -878,13 +878,16 @@ class P4Submit(Command, P4UserMap):
if gitConfig("git-p4.skipSubmitEditCheck") == "true":
return True
- if os.stat(template_file).st_mtime <= mtime:
- while True:
- response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
- if response == 'y':
- return True
- if response == 'n':
- return False
+ # modification time updated means user saved the file
+ if os.stat(template_file).st_mtime > mtime:
+ return True
+
+ while True:
+ response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
+ if response == 'y':
+ return True
+ if response == 'n':
+ return False
def applyCommit(self, id):
print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
@@ -1068,6 +1071,7 @@ class P4Submit(Command, P4UserMap):
self.modifyChangelistUser(changelist, p4User)
else:
# skip this patch
+ print "Submission cancelled, undoing p4 changes."
for f in editedFiles:
p4_revert(f)
for f in filesToAdd:
diff --git a/t/t9805-skip-submit-edit.sh b/t/t9805-skip-submit-edit.sh
index 734ccf2..df929e0 100755
--- a/t/t9805-skip-submit-edit.sh
+++ b/t/t9805-skip-submit-edit.sh
@@ -38,7 +38,7 @@ test_expect_success 'no config, unedited, say no' '
cd "$git" &&
echo line >>file1 &&
git commit -a -m "change 3 (not really)" &&
- printf "bad response\nn\n" | "$GITP4" submit
+ printf "bad response\nn\n" | "$GITP4" submit &&
p4 changes //depot/... >wc &&
test_line_count = 2 wc
)
@@ -74,6 +74,28 @@ test_expect_success 'skipSubmitEditCheck' '
)
'
+# check the normal case, where the template really is edited
+test_expect_success 'no config, edited' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ ed="$TRASH_DIRECTORY/ed.sh" &&
+ test_when_finished "rm \"$ed\"" &&
+ cat >"$ed" <<-EOF &&
+ #!$SHELL_PATH
+ sleep 1
+ touch "\$1"
+ exit 0
+ EOF
+ chmod 755 "$ed" &&
+ (
+ cd "$git" &&
+ echo line >>file1 &&
+ git commit -a -m "change 5" &&
+ EDITOR="\"$ed\"" "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 5 wc
+ )
+'
test_expect_success 'kill p4d' '
kill_p4d
--
1.7.8.154.g767b7.dirty
prev parent reply other threads:[~2011-12-17 17:41 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-08 20:40 git-p4.skipSubmitEdit L. A. Linden Levy
2011-09-09 10:05 ` git-p4.skipSubmitEdit Vitor Antunes
2011-09-09 17:47 ` git-p4.skipSubmitEdit Luke Diamand
2011-09-09 17:52 ` git-p4.skipSubmitEdit L. A. Linden Levy
2011-09-10 6:10 ` git-p4.skipSubmitEdit Luke Diamand
2011-09-12 7:34 ` git-p4.skipSubmitEdit Luke Diamand
2011-09-12 17:12 ` git-p4.skipSubmitEdit L. A. Linden Levy
2011-10-18 0:45 ` git-p4.skipSubmitEdit Pete Wyckoff
2011-10-18 16:51 ` git-p4.skipSubmitEdit L. A. Linden Levy
2011-10-18 17:35 ` git-p4.skipSubmitEdit Pete Wyckoff
2011-10-18 17:53 ` git-p4.skipSubmitEdit Luke Diamand
2011-10-20 1:16 ` git-p4.skipSubmitEdit Pete Wyckoff
2011-12-16 15:38 ` git-p4.skipSubmitEdit Michael Horowitz
2011-12-16 19:50 ` git-p4.skipSubmitEdit Luke Diamand
2011-12-17 0:46 ` git-p4.skipSubmitEdit Michael Horowitz
2011-12-17 0:49 ` git-p4.skipSubmitEdit Michael Horowitz
2011-12-17 17:39 ` Pete Wyckoff [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111217173903.GA13674@padd.com \
--to=pw@padd$(echo .)com \
--cc=alevy@mobitv$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=luke@diamand$(echo .)org \
--cc=michael.horowitz@ieee$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox