Re: [PATCH 00/12] git p4: submit conflict handling
On 17/08/12 00:35, Pete Wyckoff wrote: These patches rework how git p4 deals with conflicts that arise during a git p4 submit. These may arise due to changes that happened in p4 since the last git p4 sync. Luke: I especially wanted to get this out as you suggested that you had a different way of dealing with skipped commits. The part that needs the most attention is the interaction loop that happens when a commit failed. Currently, three options are offered: [s]kip this commit, but continue to apply others [a]pply the commit forcefully, generating .rej files [w]rite the commit to a patch.txt file and the implicitctrl-c to stop After this series, it offers two: [c]ontinue to apply others [q]uit to stop This feels more natural to me, and I like the term continue rather than skip as it matches what rebase uses. I'd like to know what others think of the new flow. The skip is still needed. In my workflow, git-p4 gets run periodically and does the usual sync+rebase on behalf of all the people who have pushed to the git repo. If someone pushes a change which conflicts with something from Perforce land, then what I want to happen is for the script to discard the offending commit (git rebase --skip) and then carry on with the others. In 99% of cases this does exactly what I need, as conflicting commits are usually caused by people committing the same fix to both p4 and git at around the same time (someone breaks top-of-tree with an obvious error, two separate people check in slightly different fixes). Discarding the git commit then means that everything carries on working. I've got a small patch which makes skipping work non-interactively; the thing it's missing is reporting the commits which are skipped. Other observable changes are new command-line options: Alias -v for --verbose, similar to other git commands. The --dry-run option addresses Luke's concern in http://thread.gmane.org/gmane.comp.version-control.git/201004/focus=201022 when I removed an unused self.interactive variable that did a similar thing if you edited the code. It prints commits that would be applied to p4. Option --prepare-p4-only is similar to --dry-run, in that it does not submit anything to p4, but it does prepare the p4 workspace, then prints long instructions about how to submit everything properly. It also serves, perhaps, as a replacement for the [a]pply option in the submit-conflict loop. Pete Wyckoff (12): git p4 test: remove bash-ism of combined export/assignment git p4 test: use p4d -L option to suppress log messages git p4: gracefully fail if some commits could not be applied git p4: remove submit failure options [a]pply and [w]rite git p4: move conflict prompt into run, use [c]ontinue and [q]uit git p4: standardize submit cancel due to unchanged template git p4: test clean-up after failed submit, fix added files git p4: rearrange submit template construction git p4: revert deleted files after submit cancel git p4: accept -v for --verbose git p4: add submit --dry-run option git p4: add submit --prepare-p4-only option Documentation/git-p4.txt | 13 +- git-p4.py | 213 +++-- t/lib-git-p4.sh| 10 +- t/t9805-git-p4-skip-submit-edit.sh | 2 +- t/t9807-git-p4-submit.sh | 65 +++ t/t9810-git-p4-rcs.sh | 50 + t/t9815-git-p4-submit-fail.sh | 367 + 7 files changed, 612 insertions(+), 108 deletions(-) create mode 100755 t/t9815-git-p4-submit-fail.sh -- 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 00/12] git p4: submit conflict handling
l...@diamand.org wrote on Fri, 17 Aug 2012 07:04 +0100: On 17/08/12 00:35, Pete Wyckoff wrote: These patches rework how git p4 deals with conflicts that arise during a git p4 submit. These may arise due to changes that happened in p4 since the last git p4 sync. Luke: I especially wanted to get this out as you suggested that you had a different way of dealing with skipped commits. The part that needs the most attention is the interaction loop that happens when a commit failed. Currently, three options are offered: [s]kip this commit, but continue to apply others [a]pply the commit forcefully, generating .rej files [w]rite the commit to a patch.txt file and the implicitctrl-c to stop After this series, it offers two: [c]ontinue to apply others [q]uit to stop This feels more natural to me, and I like the term continue rather than skip as it matches what rebase uses. I'd like to know what others think of the new flow. The skip is still needed. In my workflow, git-p4 gets run periodically and does the usual sync+rebase on behalf of all the people who have pushed to the git repo. If someone pushes a change which conflicts with something from Perforce land, then what I want to happen is for the script to discard the offending commit (git rebase --skip) and then carry on with the others. In 99% of cases this does exactly what I need, as conflicting commits are usually caused by people committing the same fix to both p4 and git at around the same time (someone breaks top-of-tree with an obvious error, two separate people check in slightly different fixes). Discarding the git commit then means that everything carries on working. I've got a small patch which makes skipping work non-interactively; the thing it's missing is reporting the commits which are skipped. This discard offending commits part I had not thought anyone would ever do. Instead, why not do git p4 rebase on its own and use git rebase --skip to discard the offending ones explicitly. It seems dangerous to do it implicitly as part of a multi-commit submit to p4. Thanks for sending your RFC work. I see what you are thinking about. Assuming that it really would be good to have a way to _automatically_ discard conflicting commits, then sure, keeping a list in submit and plumbing that into the rebase would work. It still scares me. There are quite a few special cases where it fails, of course, like if future commits involve dependencies on the one you want to skip. Would this alternative approach work: git p4 submit --discard-conflicting-commits (and/or the option). It automatically hits skip after every submit failure. When done, it does git p4 sync to get a report on what ended up in tree. Then instead of rebasing, the HEAD is simply taken to the top of the p4 tree. No need to rebase if the rule is to discard all skipped patches. Plus some reporting to say what was lost. I will reroll my series once we've figured out how we want these to co-exist. -- 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 00/12] git p4: submit conflict handling
These patches rework how git p4 deals with conflicts that arise during a git p4 submit. These may arise due to changes that happened in p4 since the last git p4 sync. Luke: I especially wanted to get this out as you suggested that you had a different way of dealing with skipped commits. The part that needs the most attention is the interaction loop that happens when a commit failed. Currently, three options are offered: [s]kip this commit, but continue to apply others [a]pply the commit forcefully, generating .rej files [w]rite the commit to a patch.txt file and the implicit ctrl-c to stop After this series, it offers two: [c]ontinue to apply others [q]uit to stop This feels more natural to me, and I like the term continue rather than skip as it matches what rebase uses. I'd like to know what others think of the new flow. Other observable changes are new command-line options: Alias -v for --verbose, similar to other git commands. The --dry-run option addresses Luke's concern in http://thread.gmane.org/gmane.comp.version-control.git/201004/focus=201022 when I removed an unused self.interactive variable that did a similar thing if you edited the code. It prints commits that would be applied to p4. Option --prepare-p4-only is similar to --dry-run, in that it does not submit anything to p4, but it does prepare the p4 workspace, then prints long instructions about how to submit everything properly. It also serves, perhaps, as a replacement for the [a]pply option in the submit-conflict loop. Pete Wyckoff (12): git p4 test: remove bash-ism of combined export/assignment git p4 test: use p4d -L option to suppress log messages git p4: gracefully fail if some commits could not be applied git p4: remove submit failure options [a]pply and [w]rite git p4: move conflict prompt into run, use [c]ontinue and [q]uit git p4: standardize submit cancel due to unchanged template git p4: test clean-up after failed submit, fix added files git p4: rearrange submit template construction git p4: revert deleted files after submit cancel git p4: accept -v for --verbose git p4: add submit --dry-run option git p4: add submit --prepare-p4-only option Documentation/git-p4.txt | 13 +- git-p4.py | 213 +++-- t/lib-git-p4.sh| 10 +- t/t9805-git-p4-skip-submit-edit.sh | 2 +- t/t9807-git-p4-submit.sh | 65 +++ t/t9810-git-p4-rcs.sh | 50 + t/t9815-git-p4-submit-fail.sh | 367 + 7 files changed, 612 insertions(+), 108 deletions(-) create mode 100755 t/t9815-git-p4-submit-fail.sh -- 1.7.11.4 -- 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