Re: [PATCH 00/12] git p4: submit conflict handling

From: Luke Diamand <luke@diamand.org>
Date: Aug 17, 2012, 06:04 UTC
Message-ID: <502DDEF5.4090405@diamand.org>
In-Reply-To: <1345160114-27654-1-git-send-email-pw@padd.com>

On 17/08/12 00:35, Pete Wyckoff wrote:

Show 24 quoted lines

> 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.

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.

Show 43 quoted lines

>
> 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
>