git-p4.py: fix --prepare-p4-only error with multiple commits

From: Ben Keene <[email protected]>

When using git p4 submit with the --prepare-p4-only option, the program should prepare a single p4 changelist and notify the user that more commits are pending and then stop processing.

A bug has been introduced by the p4-changelist hook feature that causes the program to continue to try and process all pending changelists at the same time.

The function applyCommit should return True when applying the commit was successful and the program should continue. In the case of the --prepare-p4-only flag, the function should return False, alerting the caller that the program should not proceed with additional commits.

Change the return value from True to False in the applyCommit function when git-p4 is executed with --prepare-p4-only flag.

Signed-off-by Ben Keene <[email protected]>

---

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-775%2Fseraphire%2Fseraphire%2Fp4-hook-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-775/seraphire/seraphire/p4-hook-v1
Pull-Request: https://github.com/git/git/pull/775

 git-p4.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

git-p4.py +2 -2

index b8b2a1679e7..d9ced1bf552 100755
@@ -1984,7 +1984,7 @@ def get_diff_description(self, editedFiles, filesToAdd, symlinks):
         return (diff + newdiff).replace('\r\n', '\n')
 
     def applyCommit(self, id):
-        """Apply one commit, return True if it succeeded."""
+        """Apply one commit, return True if it should continue processing."""
 
         print("Applying", read_pipe(["git", "show", "-s",
                                      "--format=format:%h %s", id]))

@@ -2222,7 +2222,7 @@ def applyCommit(self, id):
                         print("  " + f)
                 print("")
                 sys.stdout.flush()
-                return True
+                return False
 
             if self.edit_template(fileName):
                 if not self.no_verify:

base-commit: d61d20c9b413225793f8a0b491bbbec61c184e26

From: Ben Keene <[email protected]>

When using git p4 submit with the --prepare-p4-only option, the program should prepare a single p4 changelist and notify the user that more commits are pending and then stop processing.
...

...
Be aware - this change will fix the existing test error in t9807.23 for --prepare-p4-only. However there is insufficent coverage for this flag. If more than 1 commit is pending submission to P4, the method will properly prepare the P4 changelist, however it will still exit the application with an exitcode of 1.

The current documentation does not define what the exit code should be in this condition.
(See: https://git-scm.com/docs/git-p4#Documentation/git-p4.txt---prepare-p4-only)

Signed-off-by Ben Keene <[email protected]>

---

git-p4.py: fix --prepare-p4-only error with multiple commits

When using git p4 submit with the --prepare-p4-only option, the program should prepare a single p4 changelist and notify the user that more commits are pending and then stop processing.

A bug has been introduced by the p4-changelist hook feature that causes the program to continue to try and process all pending changelists at the same time.

The function applyCommit should return True when applying the commit was successful and the program should continue. In the case of the --prepare-p4-only flag, the function should return False, alerting the caller that the program should not proceed with additional commits.

Change the return value from True to False in the applyCommit function when git-p4 is executed with --prepare-p4-only flag.

 git-p4.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

git-p4.py +0 -0

index b8b2a1679e7..c4a4012bcc1 100755

git-p4.py +0 -0

                 if i < last:
                     # prompt for what to do, or use the option/variable
                     if self.conflict_behavior == "ask":

base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17

Quoted 7 lines · Ben Keene via GitGitGadget

...
Be aware - this change will fix the existing test error in t9807.23 for --prepare-p4-only. However there is insufficent coverage for this flag. If more than 1 commit is pending submission to P4, the method will properly prepare the P4 changelist, however it will still exit the application with an exitcode of 1.

Quoted 3 lines

The current documentation does not define what the exit code should be in this condition.
(See: https://git-scm.com/docs/git-p4#Documentation/git-p4.txt---prepare-p4-only)

Then some proposal to define what the behaviour should be is needed, the consensus implements and then documented.

Quoted 16 lines

Signed-off-by Ben Keene <[email protected]>

---

git-p4.py: fix --prepare-p4-only error with multiple commits

When using git p4 submit with the --prepare-p4-only option, the program should prepare a single p4 changelist and notify the user that more commits are pending and then stop processing.

A bug has been introduced by the p4-changelist hook feature that causes the program to continue to try and process all pending changelists at the same time.

Quoted 7 lines

The function applyCommit should return True when applying the commit was successful and the program should continue. In the case of the --prepare-p4-only flag, the function should return False, alerting the caller that the program should not proceed with additional commits.

Change the return value from True to False in the applyCommit function when git-p4 is executed with --prepare-p4-only flag.

This space below the three-dash-line is the best place to describe the difference between the previous version and this one. It seems that the above text is not such a "here are what was bad/missing in v1 that got fixed/extended", or a copy of the log message (like many patches that come from GGG has). I am a bit puzzled what it is, but for now let's pretend there wasn't any text below the three-dash-line and read on.

Quoted 8 lines

 git-p4.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

git-p4.py +0 -0

index b8b2a1679e7..c4a4012bcc1 100755

git-p4.py def run(self, args): · line 2543

Quoted 7 lines

git-p4.py +0 -4

@@ -2537,11 +2537,12 @@ def run(self, args):
                 ok = self.applyCommit(commit)
             if ok:
                 applied.append(commit)
-            else:
-                if self.prepare_p4_only and i < last:
-                    print("Processing only the first commit due to option" \
-                          " --prepare-p4-only")

So, it used to be that after failing to apply a commit, unless we are at the last step, we gave a message and left the loop under the prepare-p4-only mode. We did not do anything special under the prepare-p4-only mode if applyCommit returned a success.

git-p4.py def run(self, args): · line 2543

Quoted 4 lines

git-p4.py +4 -0

+                if self.prepare_p4_only:
+                    if i < last:
+                        print("Processing only the first commit due to option" \
+                                " --prepare-p4-only")

Now, after successfully applying, we leave the loop under the prepare-p4-only mode. We give the message only when we are not at the last step.

git-p4.py def run(self, args): · line 2545

Quoted 2 lines

git-p4.py +1 -0

                     break
+            else:

So..., what happens when the first step fails to apply and then the user tells us to skip the commit? We'll go on to the next commit and then applyCommit() may say 'ok' this time around. Does that count as "processed only the first commit and we are done"?

Quoted 5 lines

git-p4.py +0 -0

                 if i < last:
                     # prompt for what to do, or use the option/variable
                     if self.conflict_behavior == "ask":

base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17