Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied
Am 8/17/2012 1:35, schrieb Pete Wyckoff: +++ b/t/t9815-git-p4-submit-fail.sh @@ -0,0 +1,93 @@ + +#!/bin/sh This initial blank line is an accident, right? ;-) -- Hannes -- 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 03/12] git p4: gracefully fail if some commits could not be applied
On 17/08/12 00:35, Pete Wyckoff wrote: 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: I could be wrong about this, but this change doesn't seem to help out with git p4 rebase, which for me at least, is where the conflicts usually get picked up first. I modified a file in p4, and the same file in git, and then did 'git p4 rebase' and it just failed in the rebase in the usual way with a big 'ol python backtrace. If this patch series is intended to sort out conflict handling, then it needs a bit more work. (Says Luke, trying not to sound too confrontational, as I'm rubbish at handling conflict) Thanks! 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 03/12] git p4: gracefully fail if some commits could not be applied
j.s...@viscovery.net wrote on Fri, 17 Aug 2012 08:53 +0200: Am 8/17/2012 1:35, schrieb Pete Wyckoff: +++ b/t/t9815-git-p4-submit-fail.sh @@ -0,0 +1,93 @@ + +#!/bin/sh This initial blank line is an accident, right? ;-) Yes, the paint on the font was still wet. Thanks! -- Pete -- 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 03/12] git p4: gracefully fail if some commits could not be applied
l...@diamand.org wrote on Fri, 17 Aug 2012 08:21 +0100:
On 17/08/12 00:35, Pete Wyckoff wrote:
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:
I could be wrong about this, but this change doesn't seem to help
out with git p4 rebase, which for me at least, is where the
conflicts usually get picked up first.
Right, this is only about the submit path. I wasn't thinking
about rebase when I worked on this code (or read your message
about rebase ORIG_HEAD).
I modified a file in p4, and the same file in git, and then did 'git
p4 rebase' and it just failed in the rebase in the usual way with a
big 'ol python backtrace.
The backtraces are not pretty, and should be fixed. I confess I
never use git p4 rebase, because it should be only git p4 sync +
git rebase @{u}. There's no conflict handling at all in the git
p4 code.
If this patch series is intended to sort out conflict handling, then
it needs a bit more work.
This patch series tries to fix the conflict handling in the
submit path only. Have to start somewhere.
What do you think we might do about the rebase path? It feels
like a situation that belongs to native git. Are there
p4-specific things like $Id$ tags that need help? We could
just catch the errors from git rebase more gracefully, or exec
directly into git rebase.
-- Pete
--
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 03/12] 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 p...@padd.com --- git-p4.py | 42 ++- t/t9810-git-p4-rcs.sh | 50 ++- t/t9815-git-p4-submit-fail.sh | 93 +++ 3 files changed, 129 insertions(+), 56 deletions(-) create mode 100755 t/t9815-git-p4-submit-fail.sh diff --git a/git-p4.py b/git-p4.py index e67d37d..2405f38 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1088,7 +1088,10 @@ class P4Submit(Command, P4UserMap): return False def applyCommit(self, id): -print Applying %s % (read_pipe(git log --max-count=1 --pretty=oneline %s % id)) +Apply one commit, return True if it succeeded. + +print Applying, read_pipe([git, show, -s, + --format=format:%h %s, id]) (p4User, gitEmail) = self.p4UserForCommit(id) @@ -1206,7 +1209,7 @@ class P4Submit(Command, P4UserMap): p4_revert(f) for f in filesToAdd: os.remove(f) -return +return False elif response == a: os.system(applyPatchCmd) if len(filesToAdd) 0: @@ -1312,6 +1315,7 @@ class P4Submit(Command, P4UserMap): os.remove(f) os.remove(fileName) +return True # success # Export git tags as p4 labels. Create a p4 label and then tag # with that. @@ -1487,14 +1491,16 @@ class P4Submit(Command, P4UserMap): if gitConfig(git-p4.detectCopiesHarder, --bool) == true: self.diffOpts += --find-copies-harder -while len(commits) 0: -commit = commits[0] -commits = commits[1:] -self.applyCommit(commit) +applied = [] +for commit in commits: +ok = self.applyCommit(commit) +if ok: +applied.append(commit) -if len(commits) == 0: -print All changes applied! -chdir(self.oldWorkingDirectory) +chdir(self.oldWorkingDirectory) + +if len(commits) == len(applied): +print All commits applied! sync = P4Sync() sync.run([]) @@ -1502,6 +1508,20 @@ class P4Submit(Command, P4UserMap): rebase = P4Rebase() rebase.rebase() +else: +if len(applied) == 0: +print No commits applied. +else: +print Applied only the commits marked with '*': +for c in commits: +if c in applied: +star = * +else: +star = +print star, read_pipe([git, show, -s, + --format=format:%h %s, c]) +print You will have to do 'git p4 sync' and rebase. + if gitConfig(git-p4.exportLabels, --bool) == true: self.exportLabels = True @@ -1512,6 +1532,10 @@ class P4Submit(Command, P4UserMap): missingGitTags = gitTags - p4Labels self.exportGitTags(missingGitTags) +# exit with error unless everything applied perfecly +if len(commits) != len(applied): +sys.exit(1) + return True class View(object): diff --git
