git p4: gracefully fail if some commits could not be applied
If a commit fails to apply cleanly to the p4 tree, an interactive prompt asks what to do next. In all cases (skip, apply, write), the behavior after the prompt had a few problems. Change it so that it does not claim erroneously that all commits were applied. Instead list the set of the patches under consideration, and mark with an asterisk those that were applied successfully. Like this example: Applying 592f1f9 line5 in file1 will conflict ... Unfortunately applying the change failed! What do you want to do? [s]kip this patch / [a]pply the patch forcibly and with .rej files / [w]rite the patch to a file (patch.txt) s Skipping! Good luck with the next patches... //depot/file1#4 - was edit, reverted Applying b8db1c6 okay_commit_after_skip ... Change 6 submitted. Applied only the commits marked with '*': 592f1f9 line5 in file1 will conflict * b8db1c6 okay_commit_after_skip Do not try to sync and rebase unless all patches were applied. If there was a conflict during the submit, there is sure to be one at the rebase. Let the user to do the sync and rebase manually. This changes how a couple tets in t9810-git-p4-rcs.sh behave: - git p4 now does not leave files open and edited in the client - If a git commit contains a change to a file that was deleted in p4, the test used to check that the sync/rebase loop happened after the failure to apply the change. Since now sync/rebase does not happen after failure, do not test this. Normal rebase machinery, outside of git p4, will let rebase --skip work. Signed-off-by: Pete Wyckoff <pw@padd.com> Acked-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pete Wyckoff committed
Sep 9, 2012 at 16:16 UTC
67b0fe2eb6b70f530be347260c3792509a900feb
3 files changed
+128
-56
git-p4.py
+33
-9
@@ -1088,7 +1088,10 @@ class P4Submit(Command, P4UserMap):
1088
return False
1089
1090
def applyCommit(self, id):
1091
- print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
1091
+ """Apply one commit, return True if it succeeded."""
1092
+
1093
+ print "Applying", read_pipe(["git", "show", "-s",
1094
+ "--format=format:%h %s", id])
1095
1096
(p4User, gitEmail) = self.p4UserForCommit(id)
1097
@@ -1206,7 +1209,7 @@ class P4Submit(Command, P4UserMap):
1209
p4_revert(f)
1210
for f in filesToAdd:
1211
os.remove(f)
1209
- return
1212
+ return False
1213
elif response == "a":
1214
os.system(applyPatchCmd)
1215
if len(filesToAdd) > 0:
@@ -1312,6 +1315,7 @@ class P4Submit(Command, P4UserMap):
1315
os.remove(f)
1316
1317
os.remove(fileName)
1318
+ return True # success
1319
1320
# Export git tags as p4 labels. Create a p4 label and then tag
1321
# with that.
@@ -1487,14 +1491,16 @@ class P4Submit(Command, P4UserMap):
1491
if gitConfig("git-p4.detectCopiesHarder", "--bool") == "true":
1492
self.diffOpts += " --find-copies-harder"
1493
1490
- while len(commits) > 0:
1491
- commit = commits[0]
1492
- commits = commits[1:]
1493
- self.applyCommit(commit)
1494
+ applied = []
1495
+ for commit in commits:
1496
+ ok = self.applyCommit(commit)
1497
+ if ok:
1498
+ applied.append(commit)
1499
1495
- if len(commits) == 0:
1496
- print "All changes applied!"
1497
- chdir(self.oldWorkingDirectory)
1500
+ chdir(self.oldWorkingDirectory)
1501
+
1502
+ if len(commits) == len(applied):
1503
+ print "All commits applied!"
1504
1505
sync = P4Sync()
1506
sync.run([])
@@ -1502,6 +1508,20 @@ class P4Submit(Command, P4UserMap):
1508
rebase = P4Rebase()
1509
rebase.rebase()
1510
1511
+ else:
1512
+ if len(applied) == 0:
1513
+ print "No commits applied."
1514
+ else:
1515
+ print "Applied only the commits marked with '*':"
1516
+ for c in commits:
1517
+ if c in applied:
1518
+ star = "*"
1519
+ else:
1520
+ star = " "
1521
+ print star, read_pipe(["git", "show", "-s",
1522
+ "--format=format:%h %s", c])
1523
+ print "You will have to do 'git p4 sync' and rebase."
1524
+
1525
if gitConfig("git-p4.exportLabels", "--bool") == "true":
1526
self.exportLabels = True
1527
@@ -1512,6 +1532,10 @@ class P4Submit(Command, P4UserMap):
1532
missingGitTags = gitTags - p4Labels
1533
self.exportGitTags(missingGitTags)
1534
1535
+ # exit with error unless everything applied perfecly
1536
+ if len(commits) != len(applied):
1537
+ sys.exit(1)
1538
+
1539
return True
1540
1541
class View(object):
t/t9810-git-p4-rcs.sh
+3
-47
@@ -160,9 +160,6 @@ test_expect_success 'cleanup after failure' '
160
# the cli file so that submit will get a conflict. Make sure that
161
# scrubbing doesn't make a mess of things.
162
#
163
-# Assumes that git-p4 exits leaving the p4 file open, with the
164
-# conflict-generating patch unapplied.
165
-#
163
# This might happen only if the git repo is behind the p4 repo at
164
# submit time, and there is a conflict.
165
#
@@ -181,14 +178,11 @@ test_expect_success 'do not scrub plain text' '
178
sed -i "s/^line5/line5 p4 edit/" file_text &&
179
p4 submit -d "file5 p4 edit"
180
) &&
184
- ! git p4 submit &&
181
+ echo s | test_expect_code 1 git p4 submit &&
182
(
186
- # exepct something like:
187
- # file_text - file(s) not opened on this client
188
- # but not copious diff output
183
+ # make sure the file is not left open
184
cd "$cli" &&
190
- p4 diff file_text >wc &&
191
- test_line_count = 1 wc
185
+ ! p4 fstat -T action file_text
186
)
187
)
188
'
@@ -343,44 +337,6 @@ test_expect_failure 'Add keywords in git which do not match the default p4 value
337
)
338
'
339
346
-# Check that the existing merge conflict handling still works.
347
-# Modify kwfile1.c in git, and delete in p4. We should be able
348
-# to skip the git commit.
349
-#
350
-test_expect_success 'merge conflict handling still works' '
351
- test_when_finished cleanup_git &&
352
- (
353
- cd "$cli" &&
354
- echo "Hello:\$Id\$" >merge2.c &&
355
- echo "World" >>merge2.c &&
356
- p4 add -t ktext merge2.c &&
357
- p4 submit -d "add merge test file"
358
- ) &&
359
- git p4 clone --dest="$git" //depot &&
360
- (
361
- cd "$git" &&
362
- sed -e "/Hello/d" merge2.c >merge2.c.tmp &&
363
- mv merge2.c.tmp merge2.c &&
364
- git add merge2.c &&
365
- git commit -m "Modifying merge2.c"
366
- ) &&
367
- (
368
- cd "$cli" &&
369
- p4 delete merge2.c &&
370
- p4 submit -d "remove merge test file"
371
- ) &&
372
- (
373
- cd "$git" &&
374
- test -f merge2.c &&
375
- git config git-p4.skipSubmitEdit true &&
376
- git config git-p4.attemptRCSCleanup true &&
377
- !(echo "s" | git p4 submit) &&
378
- git rebase --skip &&
379
- ! test -f merge2.c
380
- )
381
-'
382
-
383
-
340
test_expect_success 'kill p4d' '
341
kill_p4d
342
'
t/t9815-git-p4-submit-fail.sh
new
+92
@@ -0,0 +1,92 @@
1
+#!/bin/sh
2
+
3
+test_description='git p4 submit failure handling'
4
+
5
+. ./lib-git-p4.sh
6
+
7
+test_expect_success 'start p4d' '
8
+ start_p4d
9
+'
10
+
11
+test_expect_success 'init depot' '
12
+ (
13
+ cd "$cli" &&
14
+ p4 client -o | sed "/LineEnd/s/:.*/:unix/" | p4 client -i &&
15
+ echo line1 >file1 &&
16
+ p4 add file1 &&
17
+ p4 submit -d "line1 in file1"
18
+ )
19
+'
20
+
21
+test_expect_success 'conflict on one commit, skip' '
22
+ test_when_finished cleanup_git &&
23
+ git p4 clone --dest="$git" //depot &&
24
+ (
25
+ cd "$cli" &&
26
+ p4 open file1 &&
27
+ echo line2 >>file1 &&
28
+ p4 submit -d "line2 in file1"
29
+ ) &&
30
+ (
31
+ # now this commit should cause a conflict
32
+ cd "$git" &&
33
+ git config git-p4.skipSubmitEdit true &&
34
+ echo line3 >>file1 &&
35
+ git add file1 &&
36
+ git commit -m "line3 in file1 will conflict" &&
37
+ echo s | test_expect_code 1 git p4 submit >out &&
38
+ test_i18ngrep "No commits applied" out
39
+ )
40
+'
41
+
42
+test_expect_success 'conflict on second of two commits, skip' '
43
+ test_when_finished cleanup_git &&
44
+ git p4 clone --dest="$git" //depot &&
45
+ (
46
+ cd "$cli" &&
47
+ p4 open file1 &&
48
+ echo line3 >>file1 &&
49
+ p4 submit -d "line3 in file1"
50
+ ) &&
51
+ (
52
+ cd "$git" &&
53
+ git config git-p4.skipSubmitEdit true &&
54
+ # this commit is okay
55
+ test_commit "first_commit_okay" &&
56
+ # now this submit should cause a conflict
57
+ echo line4 >>file1 &&
58
+ git add file1 &&
59
+ git commit -m "line4 in file1 will conflict" &&
60
+ echo s | test_expect_code 1 git p4 submit >out &&
61
+ test_i18ngrep "Applied only the commits" out
62
+ )
63
+'
64
+
65
+test_expect_success 'conflict on first of two commits, skip' '
66
+ test_when_finished cleanup_git &&
67
+ git p4 clone --dest="$git" //depot &&
68
+ (
69
+ cd "$cli" &&
70
+ p4 open file1 &&
71
+ echo line4 >>file1 &&
72
+ p4 submit -d "line4 in file1"
73
+ ) &&
74
+ (
75
+ cd "$git" &&
76
+ git config git-p4.skipSubmitEdit true &&
77
+ # this submit should cause a conflict
78
+ echo line5 >>file1 &&
79
+ git add file1 &&
80
+ git commit -m "line5 in file1 will conflict" &&
81
+ # but this commit is okay
82
+ test_commit "okay_commit_after_skip" &&
83
+ echo s | test_expect_code 1 git p4 submit >out &&
84
+ test_i18ngrep "Applied only the commits" out
85
+ )
86
+'
87
+
88
+test_expect_success 'kill p4d' '
89
+ kill_p4d
90
+'
91
+
92
+test_done