{"thread":{"id":"31265","subject":"[PATCH 00/12] git p4: submit conflict handling","startedAt":"2012-08-16T23:35:02Z","lastAt":"2012-08-17T12:21:34Z","messageCount":21,"participants":["Pete Wyckoff","Junio C Hamano","Luke Diamand","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"197137","messageId":"1345160114-27654-1-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":null,"subject":"[PATCH 00/12] git p4: submit conflict handling","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:02Z","receivedAt":"2012-08-16T23:35:02Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"These patches rework how git p4 deals with conflicts that\narise during a \"git p4 submit\".  These may arise due to\nchanges that happened in p4 since the last \"git p4 sync\".\n\nLuke: I especially wanted to get this out as you suggested\nthat you had a different way of dealing with skipped commits.\n\nThe part that needs the most attention is the interaction\nloop that happens when a commit failed.  Currently, three\noptions are offered:\n\n    [s]kip this commit, but continue to apply others\n    [a]pply the commit forcefully, generating .rej files\n    [w]rite the commit to a patch.txt file\n    and the implicit <ctrl-c> to stop\n\nAfter this series, it offers two:\n\n    [c]ontinue to apply others\n    [q]uit to stop\n\nThis feels more natural to me, and I like the term \"continue\" rather\nthan \"skip\" as it matches what rebase uses.  I'd like to know what\nothers think of the new flow.\n\nOther observable changes are new command-line options:\n\nAlias -v for --verbose, similar to other git commands.\n\nThe --dry-run option addresses Luke's concern in\n\n    http://thread.gmane.org/gmane.comp.version-control.git/201004/focus=201022\n\nwhen I removed an unused \"self.interactive\" variable\nthat did a similar thing if you edited the code.  It prints\ncommits that would be applied to p4.\n\nOption --prepare-p4-only is similar to --dry-run, in that\nit does not submit anything to p4, but it does prepare the\np4 workspace, then prints long instructions about how to submit\neverything properly.  It also serves, perhaps, as a replacement for\nthe [a]pply option in the submit-conflict loop.\n\nPete Wyckoff (12):\n  git p4 test: remove bash-ism of combined export/assignment\n  git p4 test: use p4d -L option to suppress log messages\n  git p4: gracefully fail if some commits could not be applied\n  git p4: remove submit failure options [a]pply and [w]rite\n  git p4: move conflict prompt into run, use [c]ontinue and [q]uit\n  git p4: standardize submit cancel due to unchanged template\n  git p4: test clean-up after failed submit, fix added files\n  git p4: rearrange submit template construction\n  git p4: revert deleted files after submit cancel\n  git p4: accept -v for --verbose\n  git p4: add submit --dry-run option\n  git p4: add submit --prepare-p4-only option\n\n Documentation/git-p4.txt           |  13 +-\n git-p4.py                          | 213 +++++++++++++++------\n t/lib-git-p4.sh                    |  10 +-\n t/t9805-git-p4-skip-submit-edit.sh |   2 +-\n t/t9807-git-p4-submit.sh           |  65 +++++++\n t/t9810-git-p4-rcs.sh              |  50 +----\n t/t9815-git-p4-submit-fail.sh      | 367 +++++++++++++++++++++++++++++++++++++\n 7 files changed, 612 insertions(+), 108 deletions(-)\n create mode 100755 t/t9815-git-p4-submit-fail.sh\n\n-- \n1.7.11.4\n"},{"id":"197138","messageId":"1345160114-27654-2-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 01/12] git p4 test: remove bash-ism of combined export/assignment","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:03Z","receivedAt":"2012-08-16T23:35:03Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 2d753ab..482eeac 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -26,9 +26,10 @@ testid=${this_test#t}\n git_p4_test_start=9800\n P4DPORT=$((10669 + ($testid - $git_p4_test_start)))\n \n-export P4PORT=localhost:$P4DPORT\n-export P4CLIENT=client\n-export P4EDITOR=:\n+P4PORT=localhost:$P4DPORT\n+P4CLIENT=client\n+P4EDITOR=:\n+export P4PORT P4CLIENT P4EDITOR\n \n db=\"$TRASH_DIRECTORY/db\"\n cli=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli\")\n-- \n1.7.11.4\n"},{"id":"197139","messageId":"1345160114-27654-3-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 02/12] git p4 test: use p4d -L option to suppress log messages","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:04Z","receivedAt":"2012-08-16T23:35:04Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Send p4d output to a logfile in the $TRASH_DIRECTORY.\nIts messages add no value to testing.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 482eeac..edb4033 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -35,12 +35,13 @@ db=\"$TRASH_DIRECTORY/db\"\n cli=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli\")\n git=\"$TRASH_DIRECTORY/git\"\n pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n+logfile=\"$TRASH_DIRECTORY/p4d.log\"\n \n start_p4d() {\n \tmkdir -p \"$db\" \"$cli\" \"$git\" &&\n \trm -f \"$pidfile\" &&\n \t(\n-\t\tp4d -q -r \"$db\" -p $P4DPORT &\n+\t\tp4d -q -r \"$db\" -p $P4DPORT -L \"$logfile\" &\n \t\techo $! >\"$pidfile\"\n \t) &&\n \n-- \n1.7.11.4\n"},{"id":"197140","messageId":"1345160114-27654-4-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 03/12] git p4: gracefully fail if some commits could not be applied","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:05Z","receivedAt":"2012-08-16T23:35:05Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"If a commit fails to apply cleanly to the p4 tree, an interactive\nprompt asks what to do next.  In all cases (skip, apply, write),\nthe behavior after the prompt had a few problems.\n\nChange it so that it does not claim erroneously that all commits\nwere applied.  Instead list the set of the patches under\nconsideration, and mark with an asterisk those that were\napplied successfully.  Like this example:\n\n    Applying 592f1f9 line5 in file1 will conflict\n    ...\n    Unfortunately applying the change failed!\n    What do you want to do?\n    [s]kip this patch / [a]pply the patch forcibly and with .rej files / [w]rite the patch to a file (patch.txt) s\n    Skipping! Good luck with the next patches...\n    //depot/file1#4 - was edit, reverted\n    Applying b8db1c6 okay_commit_after_skip\n    ...\n    Change 6 submitted.\n    Applied only the commits marked with '*':\n      592f1f9 line5 in file1 will conflict\n    * b8db1c6 okay_commit_after_skip\n\nDo not try to sync and rebase unless all patches were applied.\nIf there was a conflict during the submit, there is sure to be one\nat the rebase.  Let the user to do the sync and rebase manually.\n\nThis changes how a couple tets in t9810-git-p4-rcs.sh behave:\n\n    - git p4 now does not leave files open and edited in the\n      client\n\n    - If a git commit contains a change to a file that was\n      deleted in p4, the test used to check that the sync/rebase\n      loop happened after the failure to apply the change.  Since\n      now sync/rebase does not happen after failure, do not test\n      this.  Normal rebase machinery, outside of git p4, will let\n      rebase --skip work.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                     | 42 ++++++++++++++-----\n t/t9810-git-p4-rcs.sh         | 50 ++---------------------\n t/t9815-git-p4-submit-fail.sh | 93 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 129 insertions(+), 56 deletions(-)\n create mode 100755 t/t9815-git-p4-submit-fail.sh\n\ndiff --git a/git-p4.py b/git-p4.py\nindex e67d37d..2405f38 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1088,7 +1088,10 @@ class P4Submit(Command, P4UserMap):\n                 return False\n \n     def applyCommit(self, id):\n-        print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n+        \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n+\n+        print \"Applying\", read_pipe([\"git\", \"show\", \"-s\",\n+                                     \"--format=format:%h %s\", id])\n \n         (p4User, gitEmail) = self.p4UserForCommit(id)\n \n@@ -1206,7 +1209,7 @@ class P4Submit(Command, P4UserMap):\n                     p4_revert(f)\n                 for f in filesToAdd:\n                     os.remove(f)\n-                return\n+                return False\n             elif response == \"a\":\n                 os.system(applyPatchCmd)\n                 if len(filesToAdd) > 0:\n@@ -1312,6 +1315,7 @@ class P4Submit(Command, P4UserMap):\n                 os.remove(f)\n \n         os.remove(fileName)\n+        return True  # success\n \n     # Export git tags as p4 labels. Create a p4 label and then tag\n     # with that.\n@@ -1487,14 +1491,16 @@ class P4Submit(Command, P4UserMap):\n         if gitConfig(\"git-p4.detectCopiesHarder\", \"--bool\") == \"true\":\n             self.diffOpts += \" --find-copies-harder\"\n \n-        while len(commits) > 0:\n-            commit = commits[0]\n-            commits = commits[1:]\n-            self.applyCommit(commit)\n+        applied = []\n+        for commit in commits:\n+            ok = self.applyCommit(commit)\n+            if ok:\n+                applied.append(commit)\n \n-        if len(commits) == 0:\n-            print \"All changes applied!\"\n-            chdir(self.oldWorkingDirectory)\n+        chdir(self.oldWorkingDirectory)\n+\n+        if len(commits) == len(applied):\n+            print \"All commits applied!\"\n \n             sync = P4Sync()\n             sync.run([])\n@@ -1502,6 +1508,20 @@ class P4Submit(Command, P4UserMap):\n             rebase = P4Rebase()\n             rebase.rebase()\n \n+        else:\n+            if len(applied) == 0:\n+                print \"No commits applied.\"\n+            else:\n+                print \"Applied only the commits marked with '*':\"\n+                for c in commits:\n+                    if c in applied:\n+                        star = \"*\"\n+                    else:\n+                        star = \" \"\n+                    print star, read_pipe([\"git\", \"show\", \"-s\",\n+                                           \"--format=format:%h %s\",  c])\n+                print \"You will have to do 'git p4 sync' and rebase.\"\n+\n         if gitConfig(\"git-p4.exportLabels\", \"--bool\") == \"true\":\n             self.exportLabels = True\n \n@@ -1512,6 +1532,10 @@ class P4Submit(Command, P4UserMap):\n             missingGitTags = gitTags - p4Labels\n             self.exportGitTags(missingGitTags)\n \n+        # exit with error unless everything applied perfecly\n+        if len(commits) != len(applied):\n+                sys.exit(1)\n+\n         return True\n \n class View(object):\ndiff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh\nindex e9daa9c..fe30ad8 100755\n--- a/t/t9810-git-p4-rcs.sh\n+++ b/t/t9810-git-p4-rcs.sh\n@@ -160,9 +160,6 @@ test_expect_success 'cleanup after failure' '\n # the cli file so that submit will get a conflict.  Make sure that\n # scrubbing doesn't make a mess of things.\n #\n-# Assumes that git-p4 exits leaving the p4 file open, with the\n-# conflict-generating patch unapplied.\n-#\n # This might happen only if the git repo is behind the p4 repo at\n # submit time, and there is a conflict.\n #\n@@ -181,14 +178,11 @@ test_expect_success 'do not scrub plain text' '\n \t\t\tsed -i \"s/^line5/line5 p4 edit/\" file_text &&\n \t\t\tp4 submit -d \"file5 p4 edit\"\n \t\t) &&\n-\t\t! git p4 submit &&\n+\t\techo s | test_expect_code 1 git p4 submit &&\n \t\t(\n-\t\t\t# exepct something like:\n-\t\t\t#    file_text - file(s) not opened on this client\n-\t\t\t# but not copious diff output\n+\t\t\t# make sure the file is not left open\n \t\t\tcd \"$cli\" &&\n-\t\t\tp4 diff file_text >wc &&\n-\t\t\ttest_line_count = 1 wc\n+\t\t\t! p4 fstat -T action file_text\n \t\t)\n \t)\n '\n@@ -343,44 +337,6 @@ test_expect_failure 'Add keywords in git which do not match the default p4 value\n \t)\n '\n \n-# Check that the existing merge conflict handling still works.\n-# Modify kwfile1.c in git, and delete in p4. We should be able\n-# to skip the git commit.\n-#\n-test_expect_success 'merge conflict handling still works' '\n-\ttest_when_finished cleanup_git &&\n-\t(\n-\t\tcd \"$cli\" &&\n-\t\techo \"Hello:\\$Id\\$\" >merge2.c &&\n-\t\techo \"World\" >>merge2.c &&\n-\t\tp4 add -t ktext merge2.c &&\n-\t\tp4 submit -d \"add merge test file\"\n-\t) &&\n-\tgit p4 clone --dest=\"$git\" //depot &&\n-\t(\n-\t\tcd \"$git\" &&\n-\t\tsed -e \"/Hello/d\" merge2.c >merge2.c.tmp &&\n-\t\tmv merge2.c.tmp merge2.c &&\n-\t\tgit add merge2.c &&\n-\t\tgit commit -m \"Modifying merge2.c\"\n-\t) &&\n-\t(\n-\t\tcd \"$cli\" &&\n-\t\tp4 delete merge2.c &&\n-\t\tp4 submit -d \"remove merge test file\"\n-\t) &&\n-\t(\n-\t\tcd \"$git\" &&\n-\t\ttest -f merge2.c &&\n-\t\tgit config git-p4.skipSubmitEdit true &&\n-\t\tgit config git-p4.attemptRCSCleanup true &&\n-\t\t!(echo \"s\" | git p4 submit) &&\n-\t\tgit rebase --skip &&\n-\t\t! test -f merge2.c\n-\t)\n-'\n-\n-\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\ndiff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\nnew file mode 100755\nindex 0000000..8a02c3b\n--- /dev/null\n+++ b/t/t9815-git-p4-submit-fail.sh\n@@ -0,0 +1,93 @@\n+\n+#!/bin/sh\n+\n+test_description='git p4 submit failure handling'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:unix/\" | p4 client -i &&\n+\t\techo line1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"line1 in file1\"\n+\t)\n+'\n+\n+test_expect_success 'conflict on one commit, skip' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 open file1 &&\n+\t\techo line2 >>file1 &&\n+\t\tp4 submit -d \"line2 in file1\"\n+\t) &&\n+\t(\n+\t\t# now this commit should cause a conflict\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\techo line3 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line3 in file1 will conflict\" &&\n+\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_i18ngrep \"No commits applied\" out\n+\t)\n+'\n+\n+test_expect_success 'conflict on second of two commits, skip' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 open file1 &&\n+\t\techo line3 >>file1 &&\n+\t\tp4 submit -d \"line3 in file1\"\n+\t) &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# this commit is okay\n+\t\ttest_commit \"first_commit_okay\" &&\n+\t\t# now this submit should cause a conflict\n+\t\techo line4 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line4 in file1 will conflict\" &&\n+\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_i18ngrep \"Applied only the commits\" out\n+\t)\n+'\n+\n+test_expect_success 'conflict on first of two commits, skip' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 open file1 &&\n+\t\techo line4 >>file1 &&\n+\t\tp4 submit -d \"line4 in file1\"\n+\t) &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# this submit should cause a conflict\n+\t\techo line5 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line5 in file1 will conflict\" &&\n+\t\t# but this commit is okay\n+\t\ttest_commit \"okay_commit_after_skip\" &&\n+\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_i18ngrep \"Applied only the commits\" out\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.11.4\n"},{"id":"197141","messageId":"1345160114-27654-5-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 04/12] git p4: remove submit failure options [a]pply and [w]rite","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:06Z","receivedAt":"2012-08-16T23:35:06Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When a patch failed to apply, these interactive options offered\nto:\n\n    1) apply the patch anyway, leaving reject (.rej) files around, or,\n    2) write the patch to a file (patch.txt)\n\nIn both cases it suggested to invoke \"git p4 submit --continue\",\nan unimplemented option.\n\nWhile manually fixing the rejects and submitting the result might\nwork, there are many steps that must be done to the job properly:\n\n    * apply patch\n    * invoke p4 add and delete\n    * change executable bits\n    * p4 sync -f renamed/copied files\n    * extract commit message into p4 change description and\n      move Jobs lines out of description section\n    * set changelist owner for --preserve-user\n\nPlus the following manual sync/rebase will cause conflicts too,\nwhich must be resolved once again.\n\nDrop these workflows.  Instead users should do a sync/rebase in\ngit, fix the conflicts there, and do a clean \"git p4 submit\".\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 20 ++------------------\n 1 file changed, 2 insertions(+), 18 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 2405f38..e08fea1 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1200,9 +1200,8 @@ class P4Submit(Command, P4UserMap):\n         if not patch_succeeded:\n             print \"What do you want to do?\"\n             response = \"x\"\n-            while response != \"s\" and response != \"a\" and response != \"w\":\n-                response = raw_input(\"[s]kip this patch / [a]pply the patch forcibly \"\n-                                     \"and with .rej files / [w]rite the patch to a file (patch.txt) \")\n+            while response != \"s\":\n+                response = raw_input(\"[s]kip this patch \")\n             if response == \"s\":\n                 print \"Skipping! Good luck with the next patches...\"\n                 for f in editedFiles:\n@@ -1210,21 +1209,6 @@ class P4Submit(Command, P4UserMap):\n                 for f in filesToAdd:\n                     os.remove(f)\n                 return False\n-            elif response == \"a\":\n-                os.system(applyPatchCmd)\n-                if len(filesToAdd) > 0:\n-                    print \"You may also want to call p4 add on the following files:\"\n-                    print \" \".join(filesToAdd)\n-                if len(filesToDelete):\n-                    print \"The following files should be scheduled for deletion with p4 delete:\"\n-                    print \" \".join(filesToDelete)\n-                die(\"Please resolve and submit the conflict manually and \"\n-                    + \"continue afterwards with git p4 submit --continue\")\n-            elif response == \"w\":\n-                system(diffcmd + \" > patch.txt\")\n-                print \"Patch saved to patch.txt in %s !\" % self.clientPath\n-                die(\"Please resolve and submit the conflict manually and \"\n-                    \"continue afterwards with git p4 submit --continue\")\n \n         system(applyPatchCmd)\n \n-- \n1.7.11.4\n"},{"id":"197143","messageId":"1345160114-27654-6-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 05/12] git p4: move conflict prompt into run, use [c]ontinue and [q]uit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:07Z","receivedAt":"2012-08-16T23:35:07Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When applying a commit to the p4 workspace fails, a prompt\nasks what to do next.  This belongs up in run() instead\nof in applyCommit(), where run() can notice, for instance,\nthat the prompt is unnecessary because this is the last commit.\n\nRemove the [s]kip option in favor of two new ones: [c]ontinue and\n[q]uit.  Continue means the same as skip, but is more similar to\nthe --continue option of rebase.  Option [q]uit stops processing.\nThis is an improvement on the current requirement of ctrl-c, s an\nexplicit \"quit\" gives git p4 a chance to clean up, show the\napplied-commit summary, and do tag export.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                     | 41 +++++++++++++++++++++++++++++------------\n t/t9815-git-p4-submit-fail.sh | 37 ++++++++++++++++++++++++++++++-------\n 2 files changed, 59 insertions(+), 19 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex e08fea1..1d5194d 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1198,17 +1198,11 @@ class P4Submit(Command, P4UserMap):\n                     patch_succeeded = True\n \n         if not patch_succeeded:\n-            print \"What do you want to do?\"\n-            response = \"x\"\n-            while response != \"s\":\n-                response = raw_input(\"[s]kip this patch \")\n-            if response == \"s\":\n-                print \"Skipping! Good luck with the next patches...\"\n-                for f in editedFiles:\n-                    p4_revert(f)\n-                for f in filesToAdd:\n-                    os.remove(f)\n-                return False\n+            for f in editedFiles:\n+                p4_revert(f)\n+            for f in filesToAdd:\n+                os.remove(f)\n+            return False\n \n         system(applyPatchCmd)\n \n@@ -1475,11 +1469,34 @@ class P4Submit(Command, P4UserMap):\n         if gitConfig(\"git-p4.detectCopiesHarder\", \"--bool\") == \"true\":\n             self.diffOpts += \" --find-copies-harder\"\n \n+        #\n+        # Apply the commits, one at a time.  On failure, ask if should\n+        # continue to try the rest of the patches, or quit.\n+        #\n         applied = []\n-        for commit in commits:\n+        last = len(commits) - 1\n+        for i, commit in enumerate(commits):\n             ok = self.applyCommit(commit)\n             if ok:\n                 applied.append(commit)\n+            else:\n+                if i < last:\n+                    quit = False\n+                    while True:\n+                        print \"What do you want to do?\"\n+                        response = raw_input(\n+                            \"[c]ontinue to submit other patches, or [q]uit? \")\n+                        if not response:\n+                            continue\n+                        if response[0] == \"c\":\n+                            print \"Continuing to submit the rest of the patches\"\n+                            break\n+                        if response[0] == \"q\":\n+                            print \"Quitting\"\n+                            quit = True\n+                            break\n+                    if quit:\n+                        break\n \n         chdir(self.oldWorkingDirectory)\n \ndiff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\nindex 8a02c3b..f6204eb 100755\n--- a/t/t9815-git-p4-submit-fail.sh\n+++ b/t/t9815-git-p4-submit-fail.sh\n@@ -19,7 +19,7 @@ test_expect_success 'init depot' '\n \t)\n '\n \n-test_expect_success 'conflict on one commit, skip' '\n+test_expect_success 'conflict on one commit' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n \t(\n@@ -35,12 +35,12 @@ test_expect_success 'conflict on one commit, skip' '\n \t\techo line3 >>file1 &&\n \t\tgit add file1 &&\n \t\tgit commit -m \"line3 in file1 will conflict\" &&\n-\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_expect_code 1 git p4 submit >out &&\n \t\ttest_i18ngrep \"No commits applied\" out\n \t)\n '\n \n-test_expect_success 'conflict on second of two commits, skip' '\n+test_expect_success 'conflict on second of two commits' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n \t(\n@@ -58,12 +58,12 @@ test_expect_success 'conflict on second of two commits, skip' '\n \t\techo line4 >>file1 &&\n \t\tgit add file1 &&\n \t\tgit commit -m \"line4 in file1 will conflict\" &&\n-\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_expect_code 1 git p4 submit >out &&\n \t\ttest_i18ngrep \"Applied only the commits\" out\n \t)\n '\n \n-test_expect_success 'conflict on first of two commits, skip' '\n+test_expect_success 'conflict on first of two commits, continue' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n \t(\n@@ -80,12 +80,35 @@ test_expect_success 'conflict on first of two commits, skip' '\n \t\tgit add file1 &&\n \t\tgit commit -m \"line5 in file1 will conflict\" &&\n \t\t# but this commit is okay\n-\t\ttest_commit \"okay_commit_after_skip\" &&\n-\t\techo s | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_commit \"okay_commit_after_continue\" &&\n+\t\techo c | test_expect_code 1 git p4 submit >out &&\n \t\ttest_i18ngrep \"Applied only the commits\" out\n \t)\n '\n \n+test_expect_success 'conflict on first of two commits, quit' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 open file1 &&\n+\t\techo line7 >>file1 &&\n+\t\tp4 submit -d \"line7 in file1\"\n+\t) &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# this submit should cause a conflict\n+\t\techo line8 >>file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"line8 in file1 will conflict\" &&\n+\t\t# but this commit is okay\n+\t\ttest_commit \"okay_commit_after_quit\" &&\n+\t\techo q | test_expect_code 1 git p4 submit >out &&\n+\t\ttest_i18ngrep \"No commits applied\" out\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.11.4\n"},{"id":"197144","messageId":"1345160114-27654-7-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 06/12] git p4: standardize submit cancel due to unchanged template","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:08Z","receivedAt":"2012-08-16T23:35:08Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When editing the submit template, if no change was made to it,\ngit p4 offers a prompt \"Submit anyway?\".  Answering \"no\" cancels\nthe submit.\n\nPreviously, a \"no\" answer behaves like a \"[s]kip\" answer to the\nfailed-patch prompt, in that it proceeded to try to apply the\nrest of the commits.  Instead, put users back into the new\n\"[s]kip / [c]ontinue\" loop so that they can decide.  This makes\nboth cases of patch failure behave identically.\n\nThe return code of git p4 after a \"no\" answer is now the same\nas that for a \"skip\" due to failed patch; update a test to\nunderstand this.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                          | 4 +++-\n t/t9805-git-p4-skip-submit-edit.sh | 2 +-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 1d5194d..075f477 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1262,6 +1262,7 @@ class P4Submit(Command, P4UserMap):\n \n         if self.edit_template(fileName):\n             # read the edited message and submit\n+            ret = True\n             tmpFile = open(fileName, \"rb\")\n             message = tmpFile.read()\n             tmpFile.close()\n@@ -1285,6 +1286,7 @@ class P4Submit(Command, P4UserMap):\n \n         else:\n             # skip this patch\n+            ret = False\n             print \"Submission cancelled, undoing p4 changes.\"\n             for f in editedFiles:\n                 p4_revert(f)\n@@ -1293,7 +1295,7 @@ class P4Submit(Command, P4UserMap):\n                 os.remove(f)\n \n         os.remove(fileName)\n-        return True  # success\n+        return ret\n \n     # Export git tags as p4 labels. Create a p4 label and then tag\n     # with that.\ndiff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh\nindex fb3c8ec..ff2cc79 100755\n--- a/t/t9805-git-p4-skip-submit-edit.sh\n+++ b/t/t9805-git-p4-skip-submit-edit.sh\n@@ -38,7 +38,7 @@ test_expect_success 'no config, unedited, say no' '\n \t\tcd \"$git\" &&\n \t\techo line >>file1 &&\n \t\tgit commit -a -m \"change 3 (not really)\" &&\n-\t\tprintf \"bad response\\nn\\n\" | git p4 submit &&\n+\t\tprintf \"bad response\\nn\\n\" | test_expect_code 1 git p4 submit &&\n \t\tp4 changes //depot/... >wc &&\n \t\ttest_line_count = 2 wc\n \t)\n-- \n1.7.11.4\n"},{"id":"197145","messageId":"1345160114-27654-8-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 07/12] git p4: test clean-up after failed submit, fix added files","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:09Z","receivedAt":"2012-08-16T23:35:09Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Test a variety of cases where a patch failed to apply to\np4 and had to be cleaned up.\n\nIf the patch failed to apply cleanly, do not try to remove\nto-be-added files, as they have not really been added yet.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                     |   2 -\n t/t9815-git-p4-submit-fail.sh | 132 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 132 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 075f477..13c62c6 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1200,8 +1200,6 @@ class P4Submit(Command, P4UserMap):\n         if not patch_succeeded:\n             for f in editedFiles:\n                 p4_revert(f)\n-            for f in filesToAdd:\n-                os.remove(f)\n             return False\n \n         system(applyPatchCmd)\ndiff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\nindex f6204eb..876b90f 100755\n--- a/t/t9815-git-p4-submit-fail.sh\n+++ b/t/t9815-git-p4-submit-fail.sh\n@@ -109,6 +109,138 @@ test_expect_success 'conflict on first of two commits, quit' '\n \t)\n '\n \n+#\n+# Cleanup after submit fail, all cases.  Some modifications happen\n+# before trying to apply the patch.  Make sure these are unwound\n+# properly.  Put each one in a diff along with something that will\n+# obviously conflict.  Make sure it is back to normal after.\n+#\n+\n+test_expect_success 'cleanup edit p4 populate' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo text file >text &&\n+\t\tp4 add text &&\n+\t\techo text+x file >text+x &&\n+\t\tchmod 755 text+x &&\n+\t\tp4 add text+x &&\n+\t\tp4 submit -d \"populate p4\"\n+\t)\n+'\n+\n+setup_conflict() {\n+\t# clone before modifying file1 to force it to conflict\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t# ticks outside subshells\n+\ttest_tick &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 open file1 &&\n+\t\techo $test_tick >>file1 &&\n+\t\tp4 submit -d \"$test_tick in file1\"\n+\t) &&\n+\ttest_tick &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# easy conflict\n+\t\techo $test_tick >>file1 &&\n+\t\tgit add file1\n+\t\t# caller will add more and submit\n+\t)\n+}\n+\n+test_expect_success 'cleanup edit after submit fail' '\n+\tsetup_conflict &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo another line >>text &&\n+\t\tgit add text &&\n+\t\tgit commit -m \"conflict\" &&\n+\t\ttest_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t# make sure it is not open\n+\t\t! p4 fstat -T action text\n+\t)\n+'\n+\n+test_expect_success 'cleanup add after submit fail' '\n+\tsetup_conflict &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo new file >textnew &&\n+\t\tgit add textnew &&\n+\t\tgit commit -m \"conflict\" &&\n+\t\ttest_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t# make sure it is not there\n+\t\t# and that p4 thinks it is not added\n+\t\t#   P4 returns 0 both for \"not there but added\" and\n+\t\t#   \"not there\", so grep.\n+\t\ttest_path_is_missing textnew &&\n+\t\tp4 fstat -T action textnew 2>&1 | grep \"no such file\"\n+\t)\n+'\n+\n+test_expect_success 'cleanup delete after submit fail' '\n+\tsetup_conflict &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit rm text+x &&\n+\t\tgit commit -m \"conflict\" &&\n+\t\ttest_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t# make sure it is there\n+\t\ttest_path_is_file text+x &&\n+\t\t! p4 fstat -T action text+x\n+\t)\n+'\n+\n+test_expect_success 'cleanup copy after submit fail' '\n+\tsetup_conflict &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tcp text text2 &&\n+\t\tgit add text2 &&\n+\t\tgit commit -m \"conflict\" &&\n+\t\tgit config git-p4.detectCopies true &&\n+\t\tgit config git-p4.detectCopiesHarder true &&\n+\t\t# make sure setup is okay\n+\t\tgit diff-tree -r -C --find-copies-harder HEAD | grep text2 | grep C100 &&\n+\t\ttest_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing text2 &&\n+\t\tp4 fstat -T action text2 2>&1 | grep \"no such file\"\n+\t)\n+'\n+\n+test_expect_success 'cleanup rename after submit fail' '\n+\tsetup_conflict &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit mv text text2 &&\n+\t\tgit commit -m \"conflict\" &&\n+\t\tgit config git-p4.detectRenames true &&\n+\t\t# make sure setup is okay\n+\t\tgit diff-tree -r -M HEAD | grep text2 | grep R100 &&\n+\t\ttest_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing text2 &&\n+\t\tp4 fstat -T action text2 2>&1 | grep \"no such file\"\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.11.4\n"},{"id":"197146","messageId":"1345160114-27654-9-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 08/12] git p4: rearrange submit template construction","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:10Z","receivedAt":"2012-08-16T23:35:10Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Put all items in order as they appear, and add comments.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 29 +++++++++++++++++++++--------\n 1 file changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 13c62c6..0e874cb 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1202,6 +1202,9 @@ class P4Submit(Command, P4UserMap):\n                 p4_revert(f)\n             return False\n \n+        #\n+        # Apply the patch for real, and do add/delete/+x handling.\n+        #\n         system(applyPatchCmd)\n \n         for f in filesToAdd:\n@@ -1215,6 +1218,10 @@ class P4Submit(Command, P4UserMap):\n             mode = filesToChangeExecBit[f]\n             setP4ExecBit(f, mode)\n \n+        #\n+        # Build p4 change description, starting with the contents\n+        # of the git commit message.\n+        #\n         logMessage = extractLogMessageFromGitCommit(id)\n         logMessage = logMessage.strip()\n         (logMessage, jobs) = self.separate_jobs_from_description(logMessage)\n@@ -1223,8 +1230,16 @@ class P4Submit(Command, P4UserMap):\n         submitTemplate = self.prepareLogMessage(template, logMessage, jobs)\n \n         if self.preserveUser:\n-           submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\n+           submitTemplate += \"\\n######## Actual user %s, modified after commit\\n\" % p4User\n+\n+        if self.checkAuthorship and not self.p4UserIsMe(p4User):\n+            submitTemplate += \"######## git author %s does not match your p4 account.\\n\" % gitEmail\n+            submitTemplate += \"######## Use option --preserve-user to modify authorship.\\n\"\n+            submitTemplate += \"######## Variable git-p4.skipUserNameCheck hides this message.\\n\"\n+\n+        separatorLine = \"######## everything below this line is just the diff #######\\n\"\n \n+        # diff\n         if os.environ.has_key(\"P4DIFF\"):\n             del(os.environ[\"P4DIFF\"])\n         diff = \"\"\n@@ -1232,6 +1247,7 @@ class P4Submit(Command, P4UserMap):\n             diff += p4_read_pipe(['diff', '-du',\n                                   wildcard_encode(editedFile)])\n \n+        # new file diff\n         newdiff = \"\"\n         for newFile in filesToAdd:\n             newdiff += \"==== new file ====\\n\"\n@@ -1242,13 +1258,7 @@ class P4Submit(Command, P4UserMap):\n                 newdiff += \"+\" + line\n             f.close()\n \n-        if self.checkAuthorship and not self.p4UserIsMe(p4User):\n-            submitTemplate += \"######## git author %s does not match your p4 account.\\n\" % gitEmail\n-            submitTemplate += \"######## Use option --preserve-user to modify authorship.\\n\"\n-            submitTemplate += \"######## Variable git-p4.skipUserNameCheck hides this message.\\n\"\n-\n-        separatorLine = \"######## everything below this line is just the diff #######\\n\"\n-\n+        # change description file: submitTemplate, separatorLine, diff, newdiff\n         (handle, fileName) = tempfile.mkstemp()\n         tmpFile = os.fdopen(handle, \"w+\")\n         if self.isWindows:\n@@ -1258,6 +1268,9 @@ class P4Submit(Command, P4UserMap):\n         tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n         tmpFile.close()\n \n+        #\n+        # Let the user edit the change description, then submit it.\n+        #\n         if self.edit_template(fileName):\n             # read the edited message and submit\n             ret = True\n-- \n1.7.11.4\n"},{"id":"197147","messageId":"1345160114-27654-10-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 09/12] git p4: revert deleted files after submit cancel","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:11Z","receivedAt":"2012-08-16T23:35:11Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The user can decide not to continue with a submission,\nby not saving the p4 submit template, then answering \"no\" to\nthe \"Submit anyway?\" prompt.  In this case, be sure to\nreturn the p4 client to its initial state.\n\nDeleted files were not reverted; fix this and test all cases.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                     |   2 +\n t/t9815-git-p4-submit-fail.sh | 119 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 121 insertions(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 0e874cb..02b4e44 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1304,6 +1304,8 @@ class P4Submit(Command, P4UserMap):\n             for f in filesToAdd:\n                 p4_revert(f)\n                 os.remove(f)\n+            for f in filesToDelete:\n+                p4_revert(f)\n \n         os.remove(fileName)\n         return ret\ndiff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\nindex 876b90f..d2e7e54 100755\n--- a/t/t9815-git-p4-submit-fail.sh\n+++ b/t/t9815-git-p4-submit-fail.sh\n@@ -241,6 +241,125 @@ test_expect_success 'cleanup rename after submit fail' '\n \t)\n '\n \n+#\n+# Cleanup after deciding not to submit during editTemplate.  This\n+# involves unwinding more work, because files have been added, deleted\n+# and chmod-ed now.  Same approach as above.\n+#\n+\n+test_expect_success 'cleanup edit after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line >>text &&\n+\t\tgit add text &&\n+\t\tgit commit -m text &&\n+\t\techo n | test_expect_code 1 git p4 submit &&\n+\t\tgit reset --hard HEAD^\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\t! p4 fstat -T action text &&\n+\t\ttest_cmp \"$git\"/text text\n+\t)\n+'\n+\n+test_expect_success 'cleanup add after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line >textnew &&\n+\t\tgit add textnew &&\n+\t\tgit commit -m textnew &&\n+\t\techo n | test_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing textnew &&\n+\t\tp4 fstat -T action textnew 2>&1 | grep \"no such file\"\n+\t)\n+'\n+\n+test_expect_success 'cleanup delete after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit rm text &&\n+\t\tgit commit -m \"rm text\" &&\n+\t\techo n | test_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file text &&\n+\t\t! p4 fstat -T action text\n+\t)\n+'\n+\n+test_expect_success 'cleanup copy after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tcp text text2 &&\n+\t\tgit add text2 &&\n+\t\tgit commit -m text2 &&\n+\t\tgit config git-p4.detectCopies true &&\n+\t\tgit config git-p4.detectCopiesHarder true &&\n+\t\tgit diff-tree -r -C --find-copies-harder HEAD | grep text2 | grep C100 &&\n+\t\techo n | test_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing text2 &&\n+\t\tp4 fstat -T action text2 2>&1 | grep \"no such file\"\n+\t)\n+'\n+\n+test_expect_success 'cleanup rename after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit mv text text2 &&\n+\t\tgit commit -m text2 &&\n+\t\tgit config git-p4.detectRenames true &&\n+\t\tgit diff-tree -r -M HEAD | grep text2 | grep R100 &&\n+\t\techo n | test_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing text2 &&\n+\t\tp4 fstat -T action text2 2>&1 | grep \"no such file\"\n+\t\ttest_path_is_file text &&\n+\t\t! p4 fstat -T action text\n+\t)\n+'\n+\n+test_expect_success 'cleanup chmod after submit cancel' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tchmod u+x text &&\n+\t\tchmod u-x text+x &&\n+\t\tgit add text text+x &&\n+\t\tgit commit -m \"chmod texts\" &&\n+\t\techo n | test_expect_code 1 git p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file text &&\n+\t\t! p4 fstat -T action text &&\n+\t\tstat --format=%A text | egrep ^-r-- &&\n+\t\ttest_path_is_file text+x &&\n+\t\t! p4 fstat -T action text+x &&\n+\t\tstat --format=%A text+x | egrep ^-r-x\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.11.4\n"},{"id":"197148","messageId":"1345160114-27654-11-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 10/12] git p4: accept -v for --verbose","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:12Z","receivedAt":"2012-08-16T23:35:12Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The short form \"-v\" is common in many git commands as an\nalias for \"--verbose\".\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n Documentation/git-p4.txt | 2 +-\n git-p4.py                | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 8228f33..4b03356 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -163,7 +163,7 @@ All commands except clone accept these options.\n --git-dir <dir>::\n \tSet the 'GIT_DIR' environment variable.  See linkgit:git[1].\n \n---verbose::\n+--verbose, -v::\n \tProvide more progress information.\n \n Sync options\ndiff --git a/git-p4.py b/git-p4.py\nindex 02b4e44..c844d00 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3028,7 +3028,7 @@ def main():\n \n     args = sys.argv[2:]\n \n-    options.append(optparse.make_option(\"--verbose\", dest=\"verbose\", action=\"store_true\"))\n+    options.append(optparse.make_option(\"--verbose\", \"-v\", dest=\"verbose\", action=\"store_true\"))\n     if cmd.needsGit:\n         options.append(optparse.make_option(\"--git-dir\", dest=\"gitdir\"))\n \n-- \n1.7.11.4\n"},{"id":"197149","messageId":"1345160114-27654-12-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 11/12] git p4: add submit --dry-run option","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:13Z","receivedAt":"2012-08-16T23:35:13Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"A new option, \"git p4 submit --dry-run\" can be used to verify\nwhat commits and labels would be moved into p4.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n Documentation/git-p4.txt |  4 ++++\n git-p4.py                | 43 ++++++++++++++++++++++++++++++-------------\n t/t9807-git-p4-submit.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 75 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 4b03356..1f32b8e 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -269,6 +269,10 @@ These options can be used to modify 'git p4 submit' behavior.\n \tExport tags from git as p4 labels. Tags found in git are applied\n \tto the perforce working directory.\n \n+--dry-run, -n::\n+\tShow just what commits would be submitted to p4; do not change\n+\tstate in git or p4.\n+\n Rebase options\n ~~~~~~~~~~~~~~\n These options can be used to modify 'git p4 rebase' behavior.\ndiff --git a/git-p4.py b/git-p4.py\nindex c844d00..161d106 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -853,12 +853,14 @@ class P4Submit(Command, P4UserMap):\n                 # preserve the user, requires relevant p4 permissions\n                 optparse.make_option(\"--preserve-user\", dest=\"preserveUser\", action=\"store_true\"),\n                 optparse.make_option(\"--export-labels\", dest=\"exportLabels\", action=\"store_true\"),\n+                optparse.make_option(\"--dry-run\", \"-n\", dest=\"dry_run\", action=\"store_true\"),\n         ]\n         self.description = \"Submit changes from git to the perforce depot.\"\n         self.usage += \" [name of git branch to submit into perforce depot]\"\n         self.origin = \"\"\n         self.detectRenames = False\n         self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n+        self.dry_run = False\n         self.isWindows = (platform.system() == \"Windows\")\n         self.exportLabels = False\n         self.p4HasMoveCommand = p4_has_command(\"move\")\n@@ -1366,14 +1368,17 @@ class P4Submit(Command, P4UserMap):\n             for mapping in clientSpec.mappings:\n                 labelTemplate += \"\\t%s\\n\" % mapping.depot_side.path\n \n-            p4_write_pipe([\"label\", \"-i\"], labelTemplate)\n+            if self.dry_run:\n+                print \"Would create p4 label %s for tag\" % name\n+            else:\n+                p4_write_pipe([\"label\", \"-i\"], labelTemplate)\n \n-            # Use the label\n-            p4_system([\"tag\", \"-l\", name] +\n-                      [\"%s@%s\" % (mapping.depot_side.path, changelist) for mapping in clientSpec.mappings])\n+                # Use the label\n+                p4_system([\"tag\", \"-l\", name] +\n+                          [\"%s@%s\" % (mapping.depot_side.path, changelist) for mapping in clientSpec.mappings])\n \n-            if verbose:\n-                print \"created p4 label for tag %s\" % name\n+                if verbose:\n+                    print \"created p4 label for tag %s\" % name\n \n     def run(self, args):\n         if len(args) == 0:\n@@ -1432,12 +1437,15 @@ class P4Submit(Command, P4UserMap):\n             os.makedirs(self.clientPath)\n \n         chdir(self.clientPath)\n-        print \"Synchronizing p4 checkout...\"\n-        if new_client_dir:\n-            # old one was destroyed, and maybe nobody told p4\n-            p4_sync(\"...\", \"-f\")\n+        if self.dry_run:\n+            print \"Would synchronize p4 checkout in %s\" % self.clientPath\n         else:\n-            p4_sync(\"...\")\n+            print \"Synchronizing p4 checkout...\"\n+            if new_client_dir:\n+                # old one was destroyed, and maybe nobody told p4\n+                p4_sync(\"...\", \"-f\")\n+            else:\n+                p4_sync(\"...\")\n         self.check()\n \n         commits = []\n@@ -1488,10 +1496,17 @@ class P4Submit(Command, P4UserMap):\n         # Apply the commits, one at a time.  On failure, ask if should\n         # continue to try the rest of the patches, or quit.\n         #\n+        if self.dry_run:\n+            print \"Would apply\"\n         applied = []\n         last = len(commits) - 1\n         for i, commit in enumerate(commits):\n-            ok = self.applyCommit(commit)\n+            if self.dry_run:\n+                print \" \", read_pipe([\"git\", \"show\", \"-s\",\n+                                      \"--format=format:%h %s\", commit])\n+                ok = True\n+            else:\n+                ok = self.applyCommit(commit)\n             if ok:\n                 applied.append(commit)\n             else:\n@@ -1515,7 +1530,9 @@ class P4Submit(Command, P4UserMap):\n \n         chdir(self.oldWorkingDirectory)\n \n-        if len(commits) == len(applied):\n+        if self.dry_run:\n+            pass\n+        elif len(commits) == len(applied):\n             print \"All commits applied!\"\n \n             sync = P4Sync()\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 9394fd4..9cb6aa7 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -54,6 +54,47 @@ test_expect_success 'submit --origin' '\n \t)\n '\n \n+test_expect_success 'submit --dry-run' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit \"dry-run1\" &&\n+\t\ttest_commit \"dry-run2\" &&\n+\t\tgit p4 submit --dry-run >out &&\n+\t\ttest_i18ngrep \"Would apply\" out\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing \"dry-run1.t\" &&\n+\t\ttest_path_is_missing \"dry-run2.t\"\n+\t)\n+'\n+\n+test_expect_success 'submit --dry-run --export-labels' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo dry-run1 >dry-run1 &&\n+\t\tgit add dry-run1 &&\n+\t\tgit commit -m \"dry-run1\" dry-run1 &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit &&\n+\t\techo dry-run2 >dry-run2 &&\n+\t\tgit add dry-run2 &&\n+\t\tgit commit -m \"dry-run2\" dry-run2 &&\n+\t\tgit tag -m \"dry-run-tag1\" dry-run-tag1 HEAD^ &&\n+\t\tgit p4 submit --dry-run --export-labels >out &&\n+\t\ttest_i18ngrep \"Would create p4 label\" out\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file \"dry-run1\" &&\n+\t\ttest_path_is_missing \"dry-run2\"\n+\t)\n+'\n+\n test_expect_success 'submit with allowSubmit' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n-- \n1.7.11.4\n"},{"id":"197150","messageId":"1345160114-27654-13-git-send-email-pw@padd.com","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"[PATCH 12/12] git p4: add submit --prepare-p4-only option","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-16T23:35:14Z","receivedAt":"2012-08-16T23:35:14Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This option can be used to prepare the client workspace for\nsubmission, only.  It does not invoke the final \"p4 submit\".\nA message describes how to proceed, either submitting the\nchanges or reverting.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n Documentation/git-p4.txt |  7 +++++++\n git-p4.py                | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t9807-git-p4-submit.sh | 24 ++++++++++++++++++++++++\n 3 files changed, 77 insertions(+)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 1f32b8e..4be4290 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -273,6 +273,13 @@ These options can be used to modify 'git p4 submit' behavior.\n \tShow just what commits would be submitted to p4; do not change\n \tstate in git or p4.\n \n+--prepare-p4-only::\n+\tApply a commit to the p4 workspace, opening, adding and deleting\n+\tfiles in p4 as for a normal submit operation.  Do not issue the\n+\tfinal \"p4 submit\", but instead print a message about how to\n+\tsubmit manually or revert.  This option always stops after the\n+\tfirst (oldest) commit.  Git tags are not exported to p4.\n+\n Rebase options\n ~~~~~~~~~~~~~~\n These options can be used to modify 'git p4 rebase' behavior.\ndiff --git a/git-p4.py b/git-p4.py\nindex 161d106..6d2f47e 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -854,6 +854,7 @@ class P4Submit(Command, P4UserMap):\n                 optparse.make_option(\"--preserve-user\", dest=\"preserveUser\", action=\"store_true\"),\n                 optparse.make_option(\"--export-labels\", dest=\"exportLabels\", action=\"store_true\"),\n                 optparse.make_option(\"--dry-run\", \"-n\", dest=\"dry_run\", action=\"store_true\"),\n+                optparse.make_option(\"--prepare-p4-only\", dest=\"prepare_p4_only\", action=\"store_true\"),\n         ]\n         self.description = \"Submit changes from git to the perforce depot.\"\n         self.usage += \" [name of git branch to submit into perforce depot]\"\n@@ -861,6 +862,7 @@ class P4Submit(Command, P4UserMap):\n         self.detectRenames = False\n         self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n         self.dry_run = False\n+        self.prepare_p4_only = False\n         self.isWindows = (platform.system() == \"Windows\")\n         self.exportLabels = False\n         self.p4HasMoveCommand = p4_has_command(\"move\")\n@@ -1270,6 +1272,41 @@ class P4Submit(Command, P4UserMap):\n         tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n         tmpFile.close()\n \n+        if self.prepare_p4_only:\n+            #\n+            # Leave the p4 tree prepared, and the submit template around\n+            # and let the user decide what to do next\n+            #\n+            print\n+            print \"P4 workspace prepared for submission.\"\n+            print \"To submit or revert, go to client workspace\"\n+            print \"  \" + self.clientPath\n+            print\n+            print \"To submit, use \\\"p4 submit\\\" to write a new description,\"\n+            print \"or \\\"p4 submit -i %s\\\" to use the one prepared by\" \\\n+                  \" \\\"git p4\\\".\" % fileName\n+            print \"You can delete the file \\\"%s\\\" when finished.\" % fileName\n+\n+            if self.preserveUser and p4User and not self.p4UserIsMe(p4User):\n+                print \"To preserve change ownership by user %s, you must\\n\" \\\n+                      \"do \\\"p4 change -f <change>\\\" after submitting and\\n\" \\\n+                      \"edit the User field.\"\n+            if pureRenameCopy:\n+                print \"After submitting, renamed files must be re-synced.\"\n+                print \"Invoke \\\"p4 sync -f\\\" on each of these files:\"\n+                for f in pureRenameCopy:\n+                    print \"  \" + f\n+\n+            print\n+            print \"To revert the changes, use \\\"p4 revert ...\\\", and delete\"\n+            print \"the submit template file \\\"%s\\\"\" % fileName\n+            if filesToAdd:\n+                print \"Since the commit adds new files, they must be deleted:\"\n+                for f in filesToAdd:\n+                    print \"  \" + f\n+            print\n+            return True\n+\n         #\n         # Let the user edit the change description, then submit it.\n         #\n@@ -1370,6 +1407,9 @@ class P4Submit(Command, P4UserMap):\n \n             if self.dry_run:\n                 print \"Would create p4 label %s for tag\" % name\n+            elif self.prepare_p4_only:\n+                print \"Not creating p4 label %s for tag due to option\" \\\n+                      \" --prepare-p4-only\" % name\n             else:\n                 p4_write_pipe([\"label\", \"-i\"], labelTemplate)\n \n@@ -1510,6 +1550,10 @@ class P4Submit(Command, P4UserMap):\n             if ok:\n                 applied.append(commit)\n             else:\n+                if self.prepare_p4_only and i < last:\n+                    print \"Processing only the first commit due to option\" \\\n+                          \" --prepare-p4-only\"\n+                    break\n                 if i < last:\n                     quit = False\n                     while True:\n@@ -1532,6 +1576,8 @@ class P4Submit(Command, P4UserMap):\n \n         if self.dry_run:\n             pass\n+        elif self.prepare_p4_only:\n+            pass\n         elif len(commits) == len(applied):\n             print \"All commits applied!\"\n \ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 9cb6aa7..0ae048f 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -375,6 +375,30 @@ test_expect_success 'description with Jobs section and bogus following text' '\n \t\tmake_job $(cat jobname) &&\n \t\ttest_must_fail git p4 submit 2>err &&\n \t\ttest_i18ngrep \"Unknown field name\" err\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 revert desc6 &&\n+\t\trm desc6\n+\t)\n+'\n+\n+test_expect_success 'submit --prepare-p4-only' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo prep-only-add >prep-only-add &&\n+\t\tgit add prep-only-add &&\n+\t\tgit commit -m \"prep only add\" &&\n+\t\tgit p4 submit --prepare-p4-only >out &&\n+\t\ttest_i18ngrep \"prepared for submission\" out &&\n+\t\ttest_i18ngrep \"must be deleted\" out\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file prep-only-add &&\n+\t\tp4 fstat -T action prep-only-add | grep -w add\n \t)\n '\n \n-- \n1.7.11.4\n"},{"id":"197155","messageId":"7vboiaxlj1.fsf@alter.siamese.dyndns.org","threadId":"31265","inReplyTo":"1345160114-27654-2-git-send-email-pw@padd.com","subject":"Re: [PATCH 01/12] git p4 test: remove bash-ism of combined export/assignment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-17T03:08:50Z","receivedAt":"2012-08-17T03:08:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"},{"id":"197162","messageId":"502DDEF5.4090405@diamand.org","threadId":"31265","inReplyTo":"1345160114-27654-1-git-send-email-pw@padd.com","subject":"Re: [PATCH 00/12] git p4: submit conflict handling","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-08-17T06:04:37Z","receivedAt":"2012-08-17T06:04:37Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17/08/12 00:35, Pete Wyckoff wrote:\n> These patches rework how git p4 deals with conflicts that\n> arise during a \"git p4 submit\".  These may arise due to\n> changes that happened in p4 since the last \"git p4 sync\".\n>\n> Luke: I especially wanted to get this out as you suggested\n> that you had a different way of dealing with skipped commits.\n>\n> The part that needs the most attention is the interaction\n> loop that happens when a commit failed.  Currently, three\n> options are offered:\n>\n>      [s]kip this commit, but continue to apply others\n>      [a]pply the commit forcefully, generating .rej files\n>      [w]rite the commit to a patch.txt file\n>      and the implicit<ctrl-c>  to stop\n>\n> After this series, it offers two:\n>\n>      [c]ontinue to apply others\n>      [q]uit to stop\n>\n> This feels more natural to me, and I like the term \"continue\" rather\n> than \"skip\" as it matches what rebase uses.  I'd like to know what\n> others think of the new flow.\n\nThe skip is still needed. In my workflow, git-p4 gets run periodically \nand does the usual sync+rebase on behalf of all the people who have \npushed to the git repo.\n\nIf someone pushes a change which conflicts with something from Perforce \nland, then what I want to happen is for the script to discard the \noffending commit (git rebase --skip) and then carry on with the others.\n\nIn 99% of cases this does exactly what I need, as conflicting commits \nare usually caused by people committing the same fix to both p4 and git \nat around the same time (someone breaks top-of-tree with an obvious \nerror, two separate people check in slightly different fixes). \nDiscarding the git commit then means that everything carries on working.\n\nI've got a small patch which makes skipping work non-interactively; the \nthing it's missing is reporting the commits which are skipped.\n\n>\n> Other observable changes are new command-line options:\n>\n> Alias -v for --verbose, similar to other git commands.\n>\n> The --dry-run option addresses Luke's concern in\n>\n>      http://thread.gmane.org/gmane.comp.version-control.git/201004/focus=201022\n>\n> when I removed an unused \"self.interactive\" variable\n> that did a similar thing if you edited the code.  It prints\n> commits that would be applied to p4.\n>\n> Option --prepare-p4-only is similar to --dry-run, in that\n> it does not submit anything to p4, but it does prepare the\n> p4 workspace, then prints long instructions about how to submit\n> everything properly.  It also serves, perhaps, as a replacement for\n> the [a]pply option in the submit-conflict loop.\n>\n> Pete Wyckoff (12):\n>    git p4 test: remove bash-ism of combined export/assignment\n>    git p4 test: use p4d -L option to suppress log messages\n>    git p4: gracefully fail if some commits could not be applied\n>    git p4: remove submit failure options [a]pply and [w]rite\n>    git p4: move conflict prompt into run, use [c]ontinue and [q]uit\n>    git p4: standardize submit cancel due to unchanged template\n>    git p4: test clean-up after failed submit, fix added files\n>    git p4: rearrange submit template construction\n>    git p4: revert deleted files after submit cancel\n>    git p4: accept -v for --verbose\n>    git p4: add submit --dry-run option\n>    git p4: add submit --prepare-p4-only option\n>\n>   Documentation/git-p4.txt           |  13 +-\n>   git-p4.py                          | 213 +++++++++++++++------\n>   t/lib-git-p4.sh                    |  10 +-\n>   t/t9805-git-p4-skip-submit-edit.sh |   2 +-\n>   t/t9807-git-p4-submit.sh           |  65 +++++++\n>   t/t9810-git-p4-rcs.sh              |  50 +----\n>   t/t9815-git-p4-submit-fail.sh      | 367 +++++++++++++++++++++++++++++++++++++\n>   7 files changed, 612 insertions(+), 108 deletions(-)\n>   create mode 100755 t/t9815-git-p4-submit-fail.sh\n>\n"},{"id":"197163","messageId":"502DDFB7.2010408@diamand.org","threadId":"31265","inReplyTo":"1345160114-27654-3-git-send-email-pw@padd.com","subject":"Re: [PATCH 02/12] git p4 test: use p4d -L option to suppress log messages","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-08-17T06:07:51Z","receivedAt":"2012-08-17T06:07:51Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17/08/12 00:35, Pete Wyckoff wrote:\n> Send p4d output to a logfile in the $TRASH_DIRECTORY.\n> Its messages add no value to testing.\n\nI'm not totally sold on this; I still fairly frequently see weird errors \nfrom p4d and these help me work out what's going on. For example, at the \nmoment if you run a test too quickly after the last one, then it won't \nstart up (or something like that).\n\nThe problem with hiding the error messages is that I don't think I will \nthink to look in this log file if tests start failing.\n\n>\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   t/lib-git-p4.sh | 3 ++-\n>   1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 482eeac..edb4033 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -35,12 +35,13 @@ db=\"$TRASH_DIRECTORY/db\"\n>   cli=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli\")\n>   git=\"$TRASH_DIRECTORY/git\"\n>   pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n> +logfile=\"$TRASH_DIRECTORY/p4d.log\"\n>\n>   start_p4d() {\n>   \tmkdir -p \"$db\" \"$cli\" \"$git\"&&\n>   \trm -f \"$pidfile\"&&\n>   \t(\n> -\t\tp4d -q -r \"$db\" -p $P4DPORT&\n> +\t\tp4d -q -r \"$db\" -p $P4DPORT -L \"$logfile\"&\n>   \t\techo $!>\"$pidfile\"\n>   \t)&&\n>\n"},{"id":"197167","messageId":"502DEA6F.5080406@viscovery.net","threadId":"31265","inReplyTo":"1345160114-27654-4-git-send-email-pw@padd.com","subject":"Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-08-17T06:53:35Z","receivedAt":"2012-08-17T06:53:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 8/17/2012 1:35, schrieb Pete Wyckoff:\n> +++ b/t/t9815-git-p4-submit-fail.sh\n> @@ -0,0 +1,93 @@\n> +\n> +#!/bin/sh\n\nThis initial blank line is an accident, right? ;-)\n\n-- Hannes\n"},{"id":"197170","messageId":"502DF10A.2040306@diamand.org","threadId":"31265","inReplyTo":"1345160114-27654-4-git-send-email-pw@padd.com","subject":"Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-08-17T07:21:46Z","receivedAt":"2012-08-17T07:21:46Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17/08/12 00:35, Pete Wyckoff wrote:\n> If a commit fails to apply cleanly to the p4 tree, an interactive\n> prompt asks what to do next.  In all cases (skip, apply, write),\n> the behavior after the prompt had a few problems.\n>\n> Change it so that it does not claim erroneously that all commits\n> were applied.  Instead list the set of the patches under\n> consideration, and mark with an asterisk those that were\n> applied successfully.  Like this example:\n\nI could be wrong about this, but this change doesn't seem to help out \nwith \"git p4 rebase\", which for me at least, is where the conflicts \nusually get picked up first.\n\nI modified a file in p4, and the same file in git, and then did 'git p4 \nrebase' and it just failed in the rebase in the usual way with a big 'ol \npython backtrace.\n\nIf this patch series is intended to sort out conflict handling, then it \nneeds a bit more work.\n\n(Says Luke, trying not to sound too confrontational, as I'm rubbish at \nhandling conflict....)\n\nThanks!\nLuke\n"},{"id":"197177","messageId":"20120817114956.GA29214@padd.com","threadId":"31265","inReplyTo":"502DEA6F.5080406@viscovery.net","subject":"Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-17T11:49:56Z","receivedAt":"2012-08-17T11:49:56Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"j.sixt@viscovery.net wrote on Fri, 17 Aug 2012 08:53 +0200:\n> Am 8/17/2012 1:35, schrieb Pete Wyckoff:\n> > +++ b/t/t9815-git-p4-submit-fail.sh\n> > @@ -0,0 +1,93 @@\n> > +\n> > +#!/bin/sh\n> \n> This initial blank line is an accident, right? ;-)\n\nYes, the paint on the font was still wet.  Thanks!\n\n\t\t-- Pete\n"},{"id":"197179","messageId":"20120817115857.GB29214@padd.com","threadId":"31265","inReplyTo":"502DF10A.2040306@diamand.org","subject":"Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-17T11:58:57Z","receivedAt":"2012-08-17T11:58:57Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Fri, 17 Aug 2012 08:21 +0100:\n> On 17/08/12 00:35, Pete Wyckoff wrote:\n> >If a commit fails to apply cleanly to the p4 tree, an interactive\n> >prompt asks what to do next.  In all cases (skip, apply, write),\n> >the behavior after the prompt had a few problems.\n> >\n> >Change it so that it does not claim erroneously that all commits\n> >were applied.  Instead list the set of the patches under\n> >consideration, and mark with an asterisk those that were\n> >applied successfully.  Like this example:\n> \n> I could be wrong about this, but this change doesn't seem to help\n> out with \"git p4 rebase\", which for me at least, is where the\n> conflicts usually get picked up first.\n\nRight, this is only about the submit path.  I wasn't thinking\nabout rebase when I worked on this code (or read your message\nabout rebase ORIG_HEAD).\n\n> I modified a file in p4, and the same file in git, and then did 'git\n> p4 rebase' and it just failed in the rebase in the usual way with a\n> big 'ol python backtrace.\n\nThe backtraces are not pretty, and should be fixed.  I confess I\nnever use git p4 rebase, because it should be only git p4 sync +\ngit rebase @{u}.  There's no conflict handling at all in the git\np4 code.\n\n> If this patch series is intended to sort out conflict handling, then\n> it needs a bit more work.\n\nThis patch series tries to fix the conflict handling in the\nsubmit path only.  Have to start somewhere.\n\nWhat do you think we might do about the rebase path?  It feels\nlike a situation that belongs to native git.  Are there\np4-specific things like $Id$ tags that need help?  We could\njust catch the errors from git rebase more gracefully, or exec\ndirectly into git rebase.\n\n\t\t-- Pete\n"},{"id":"197180","messageId":"20120817122134.GA29257@padd.com","threadId":"31265","inReplyTo":"502DDEF5.4090405@diamand.org","subject":"Re: [PATCH 00/12] git p4: submit conflict handling","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-08-17T12:21:34Z","receivedAt":"2012-08-17T12:21:34Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Fri, 17 Aug 2012 07:04 +0100:\n> On 17/08/12 00:35, Pete Wyckoff wrote:\n> >These patches rework how git p4 deals with conflicts that\n> >arise during a \"git p4 submit\".  These may arise due to\n> >changes that happened in p4 since the last \"git p4 sync\".\n> >\n> >Luke: I especially wanted to get this out as you suggested\n> >that you had a different way of dealing with skipped commits.\n> >\n> >The part that needs the most attention is the interaction\n> >loop that happens when a commit failed.  Currently, three\n> >options are offered:\n> >\n> >     [s]kip this commit, but continue to apply others\n> >     [a]pply the commit forcefully, generating .rej files\n> >     [w]rite the commit to a patch.txt file\n> >     and the implicit<ctrl-c>  to stop\n> >\n> >After this series, it offers two:\n> >\n> >     [c]ontinue to apply others\n> >     [q]uit to stop\n> >\n> >This feels more natural to me, and I like the term \"continue\" rather\n> >than \"skip\" as it matches what rebase uses.  I'd like to know what\n> >others think of the new flow.\n> \n> The skip is still needed. In my workflow, git-p4 gets run\n> periodically and does the usual sync+rebase on behalf of all the\n> people who have pushed to the git repo.\n> \n> If someone pushes a change which conflicts with something from\n> Perforce land, then what I want to happen is for the script to\n> discard the offending commit (git rebase --skip) and then carry on\n> with the others.\n> \n> In 99% of cases this does exactly what I need, as conflicting\n> commits are usually caused by people committing the same fix to both\n> p4 and git at around the same time (someone breaks top-of-tree with\n> an obvious error, two separate people check in slightly different\n> fixes). Discarding the git commit then means that everything carries\n> on working.\n> \n> I've got a small patch which makes skipping work non-interactively;\n> the thing it's missing is reporting the commits which are skipped.\n\nThis \"discard offending commits\" part I had not thought anyone\nwould ever do.  Instead, why not do \"git p4 rebase\" on its own\nand use \"git rebase --skip\" to discard the offending ones\nexplicitly.  It seems dangerous to do it implicitly as part\nof a multi-commit submit to p4.\n\nThanks for sending your RFC work.  I see what you are thinking\nabout.\n\nAssuming that it really would be good to have a way to\n_automatically_ discard conflicting commits, then sure, keeping a\nlist in submit and plumbing that into the rebase would work.  It\nstill scares me.  There are quite a few special cases where it\nfails, of course, like if future commits involve dependencies on\nthe one you want to skip.\n\nWould this alternative approach work: \"git p4 submit\n--discard-conflicting-commits\" (and/or the option).  It\nautomatically hits \"skip\" after every submit failure.  When done,\nit does \"git p4 sync\" to get a report on what ended up in tree.\nThen instead of rebasing, the HEAD is simply taken to the top of\nthe p4 tree.  No need to rebase if the rule is to discard all\nskipped patches.  Plus some reporting to say what was lost.\n\nI will reroll my series once we've figured out how we want these\nto co-exist.\n\n\t\t-- Pete\n"}]}