[PATCH 0/3] git p4: notice Jobs: section in submit

[PATCH 0/3] git p4: notice Jobs: section in submit

[Pete Wyckoff]

Perforce has an idea of a "job" and can connect changes to jobs
in order to connect code fixes to an external bug-tracking system,
for instance.  The job can be specified in a change by adding
a section that looks like:

    Jobs:
	project-6032

For code committed from git p4, it would be nice to use a similar
notation in the commit message and have the jobs wind up in the
right section in the p4 change description.

This was discussed in
http://thread.gmane.org/gmane.comp.version-control.git/200445 .

These commits are on origin/next, as they depend on changes
in pw/git-p4-tests that was merged into next on Jul 3.

Pete Wyckoff (3):
  git p4: remove unused P4Submit interactive setting
  git p4 test: refactor marshal_dump
  git p4: notice Jobs lines in git commit messages

 git-p4.py                | 188 ++++++++++++++++++++++++++---------------------
 t/lib-git-p4.sh          |  14 ++++
 t/t9800-git-p4-basic.sh  |   5 --
 t/t9807-git-p4-submit.sh | 155 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 274 insertions(+), 88 deletions(-)

-- 
1.7.11.1.125.g4a65fea

[PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Pete Wyckoff]

The code is unused.  Delete.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 144 ++++++++++++++++++++++++++++----------------------------------
 1 file changed, 66 insertions(+), 78 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index f895a24..542c20a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):
         ]
         self.description = "Submit changes from git to the perforce depot."
         self.usage += " [name of git branch to submit into perforce depot]"
-        self.interactive = True
         self.origin = ""
         self.detectRenames = False
         self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
@@ -1209,86 +1208,77 @@ class P4Submit(Command, P4UserMap):
 
         template = self.prepareSubmitTemplate()
 
-        if self.interactive:
-            submitTemplate = self.prepareLogMessage(template, logMessage)
+        submitTemplate = self.prepareLogMessage(template, logMessage)
 
-            if self.preserveUser:
-               submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
-
-            if os.environ.has_key("P4DIFF"):
-                del(os.environ["P4DIFF"])
-            diff = ""
-            for editedFile in editedFiles:
-                diff += p4_read_pipe(['diff', '-du',
-                                      wildcard_encode(editedFile)])
-
-            newdiff = ""
-            for newFile in filesToAdd:
-                newdiff += "==== new file ====\n"
-                newdiff += "--- /dev/null\n"
-                newdiff += "+++ %s\n" % newFile
-                f = open(newFile, "r")
-                for line in f.readlines():
-                    newdiff += "+" + line
-                f.close()
-
-            if self.checkAuthorship and not self.p4UserIsMe(p4User):
-                submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
-                submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
-                submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
-
-            separatorLine = "######## everything below this line is just the diff #######\n"
-
-            (handle, fileName) = tempfile.mkstemp()
-            tmpFile = os.fdopen(handle, "w+")
-            if self.isWindows:
-                submitTemplate = submitTemplate.replace("\n", "\r\n")
-                separatorLine = separatorLine.replace("\n", "\r\n")
-                newdiff = newdiff.replace("\n", "\r\n")
-            tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
+        if self.preserveUser:
+           submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
+
+        if os.environ.has_key("P4DIFF"):
+            del(os.environ["P4DIFF"])
+        diff = ""
+        for editedFile in editedFiles:
+            diff += p4_read_pipe(['diff', '-du',
+                                  wildcard_encode(editedFile)])
+
+        newdiff = ""
+        for newFile in filesToAdd:
+            newdiff += "==== new file ====\n"
+            newdiff += "--- /dev/null\n"
+            newdiff += "+++ %s\n" % newFile
+            f = open(newFile, "r")
+            for line in f.readlines():
+                newdiff += "+" + line
+            f.close()
+
+        if self.checkAuthorship and not self.p4UserIsMe(p4User):
+            submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
+            submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
+            submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
+
+        separatorLine = "######## everything below this line is just the diff #######\n"
+
+        (handle, fileName) = tempfile.mkstemp()
+        tmpFile = os.fdopen(handle, "w+")
+        if self.isWindows:
+            submitTemplate = submitTemplate.replace("\n", "\r\n")
+            separatorLine = separatorLine.replace("\n", "\r\n")
+            newdiff = newdiff.replace("\n", "\r\n")
+        tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
+        tmpFile.close()
+
+        if self.edit_template(fileName):
+            # read the edited message and submit
+            tmpFile = open(fileName, "rb")
+            message = tmpFile.read()
             tmpFile.close()
+            submitTemplate = message[:message.index(separatorLine)]
+            if self.isWindows:
+                submitTemplate = submitTemplate.replace("\r\n", "\n")
+            p4_write_pipe(['submit', '-i'], submitTemplate)
 
-            if self.edit_template(fileName):
-                # read the edited message and submit
-                tmpFile = open(fileName, "rb")
-                message = tmpFile.read()
-                tmpFile.close()
-                submitTemplate = message[:message.index(separatorLine)]
-                if self.isWindows:
-                    submitTemplate = submitTemplate.replace("\r\n", "\n")
-                p4_write_pipe(['submit', '-i'], submitTemplate)
-
-                if self.preserveUser:
-                    if p4User:
-                        # Get last changelist number. Cannot easily get it from
-                        # the submit command output as the output is
-                        # unmarshalled.
-                        changelist = self.lastP4Changelist()
-                        self.modifyChangelistUser(changelist, p4User)
-
-                # The rename/copy happened by applying a patch that created a
-                # new file.  This leaves it writable, which confuses p4.
-                for f in pureRenameCopy:
-                    p4_sync(f, "-f")
-
-            else:
-                # skip this patch
-                print "Submission cancelled, undoing p4 changes."
-                for f in editedFiles:
-                    p4_revert(f)
-                for f in filesToAdd:
-                    p4_revert(f)
-                    os.remove(f)
+            if self.preserveUser:
+                if p4User:
+                    # Get last changelist number. Cannot easily get it from
+                    # the submit command output as the output is
+                    # unmarshalled.
+                    changelist = self.lastP4Changelist()
+                    self.modifyChangelistUser(changelist, p4User)
+
+            # The rename/copy happened by applying a patch that created a
+            # new file.  This leaves it writable, which confuses p4.
+            for f in pureRenameCopy:
+                p4_sync(f, "-f")
 
-            os.remove(fileName)
         else:
-            fileName = "submit.txt"
-            file = open(fileName, "w+")
-            file.write(self.prepareLogMessage(template, logMessage))
-            file.close()
-            print ("Perforce submit template written as %s. "
-                   + "Please review/edit and then use p4 submit -i < %s to submit directly!"
-                   % (fileName, fileName))
+            # skip this patch
+            print "Submission cancelled, undoing p4 changes."
+            for f in editedFiles:
+                p4_revert(f)
+            for f in filesToAdd:
+                p4_revert(f)
+                os.remove(f)
+
+        os.remove(fileName)
 
     # Export git tags as p4 labels. Create a p4 label and then tag
     # with that.
@@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):
             commit = commits[0]
             commits = commits[1:]
             self.applyCommit(commit)
-            if not self.interactive:
-                break
 
         if len(commits) == 0:
             print "All changes applied!"
-- 
1.7.11.1.125.g4a65fea

Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Pete Wyckoff]

The "diff -w" version looks like this.  A bit easier to review.

--->8---

>From 3327945176be35081f2c86bc40b294ed9f73ea9a Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Sun, 24 Jun 2012 15:39:18 -0400
Subject: [PATCH] git p4: remove unused P4Submit interactive setting

The code is unused.  Delete.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index f895a24..542c20a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):
         ]
         self.description = "Submit changes from git to the perforce depot."
         self.usage += " [name of git branch to submit into perforce depot]"
-        self.interactive = True
         self.origin = ""
         self.detectRenames = False
         self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
@@ -1209,7 +1208,6 @@ class P4Submit(Command, P4UserMap):
 
         template = self.prepareSubmitTemplate()
 
-        if self.interactive:
         submitTemplate = self.prepareLogMessage(template, logMessage)
 
         if self.preserveUser:
@@ -1281,14 +1279,6 @@ class P4Submit(Command, P4UserMap):
                 os.remove(f)
 
         os.remove(fileName)
-        else:
-            fileName = "submit.txt"
-            file = open(fileName, "w+")
-            file.write(self.prepareLogMessage(template, logMessage))
-            file.close()
-            print ("Perforce submit template written as %s. "
-                   + "Please review/edit and then use p4 submit -i < %s to submit directly!"
-                   % (fileName, fileName))
 
     # Export git tags as p4 labels. Create a p4 label and then tag
     # with that.
@@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):
             commit = commits[0]
             commits = commits[1:]
             self.applyCommit(commit)
-            if not self.interactive:
-                break
 
         if len(commits) == 0:
             print "All changes applied!"
-- 
1.7.11.1.125.g4a65fea

Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Luke Diamand]

On 04/07/12 14:34, Pete Wyckoff wrote:
> The code is unused.  Delete.

I've used that non-interactive code path in the past, in the very early 
days of using it (setting interactive to false manually).

The nice thing about it is that if you're using git-p4 for the very 
first time it lets you do the final submission to p4 by hand, without 
having to trust the script to do the right thing. Once I convinced 
myself that git-p4 was doing the right thing, I then stopped using it.

Is it worth retaining, perhaps fixed so that it can be set on the 
command line and documented? Or just discard?

Thanks
Luke

>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   git-p4.py | 144 ++++++++++++++++++++++++++++----------------------------------
>   1 file changed, 66 insertions(+), 78 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f895a24..542c20a 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):
>           ]
>           self.description = "Submit changes from git to the perforce depot."
>           self.usage += " [name of git branch to submit into perforce depot]"
> -        self.interactive = True
>           self.origin = ""
>           self.detectRenames = False
>           self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
> @@ -1209,86 +1208,77 @@ class P4Submit(Command, P4UserMap):
>
>           template = self.prepareSubmitTemplate()
>
> -        if self.interactive:
> -            submitTemplate = self.prepareLogMessage(template, logMessage)
> +        submitTemplate = self.prepareLogMessage(template, logMessage)
>
> -            if self.preserveUser:
> -               submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
> -
> -            if os.environ.has_key("P4DIFF"):
> -                del(os.environ["P4DIFF"])
> -            diff = ""
> -            for editedFile in editedFiles:
> -                diff += p4_read_pipe(['diff', '-du',
> -                                      wildcard_encode(editedFile)])
> -
> -            newdiff = ""
> -            for newFile in filesToAdd:
> -                newdiff += "==== new file ====\n"
> -                newdiff += "--- /dev/null\n"
> -                newdiff += "+++ %s\n" % newFile
> -                f = open(newFile, "r")
> -                for line in f.readlines():
> -                    newdiff += "+" + line
> -                f.close()
> -
> -            if self.checkAuthorship and not self.p4UserIsMe(p4User):
> -                submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
> -                submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
> -                submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
> -
> -            separatorLine = "######## everything below this line is just the diff #######\n"
> -
> -            (handle, fileName) = tempfile.mkstemp()
> -            tmpFile = os.fdopen(handle, "w+")
> -            if self.isWindows:
> -                submitTemplate = submitTemplate.replace("\n", "\r\n")
> -                separatorLine = separatorLine.replace("\n", "\r\n")
> -                newdiff = newdiff.replace("\n", "\r\n")
> -            tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
> +        if self.preserveUser:
> +           submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
> +
> +        if os.environ.has_key("P4DIFF"):
> +            del(os.environ["P4DIFF"])
> +        diff = ""
> +        for editedFile in editedFiles:
> +            diff += p4_read_pipe(['diff', '-du',
> +                                  wildcard_encode(editedFile)])
> +
> +        newdiff = ""
> +        for newFile in filesToAdd:
> +            newdiff += "==== new file ====\n"
> +            newdiff += "--- /dev/null\n"
> +            newdiff += "+++ %s\n" % newFile
> +            f = open(newFile, "r")
> +            for line in f.readlines():
> +                newdiff += "+" + line
> +            f.close()
> +
> +        if self.checkAuthorship and not self.p4UserIsMe(p4User):
> +            submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
> +            submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
> +            submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
> +
> +        separatorLine = "######## everything below this line is just the diff #######\n"
> +
> +        (handle, fileName) = tempfile.mkstemp()
> +        tmpFile = os.fdopen(handle, "w+")
> +        if self.isWindows:
> +            submitTemplate = submitTemplate.replace("\n", "\r\n")
> +            separatorLine = separatorLine.replace("\n", "\r\n")
> +            newdiff = newdiff.replace("\n", "\r\n")
> +        tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
> +        tmpFile.close()
> +
> +        if self.edit_template(fileName):
> +            # read the edited message and submit
> +            tmpFile = open(fileName, "rb")
> +            message = tmpFile.read()
>               tmpFile.close()
> +            submitTemplate = message[:message.index(separatorLine)]
> +            if self.isWindows:
> +                submitTemplate = submitTemplate.replace("\r\n", "\n")
> +            p4_write_pipe(['submit', '-i'], submitTemplate)
>
> -            if self.edit_template(fileName):
> -                # read the edited message and submit
> -                tmpFile = open(fileName, "rb")
> -                message = tmpFile.read()
> -                tmpFile.close()
> -                submitTemplate = message[:message.index(separatorLine)]
> -                if self.isWindows:
> -                    submitTemplate = submitTemplate.replace("\r\n", "\n")
> -                p4_write_pipe(['submit', '-i'], submitTemplate)
> -
> -                if self.preserveUser:
> -                    if p4User:
> -                        # Get last changelist number. Cannot easily get it from
> -                        # the submit command output as the output is
> -                        # unmarshalled.
> -                        changelist = self.lastP4Changelist()
> -                        self.modifyChangelistUser(changelist, p4User)
> -
> -                # The rename/copy happened by applying a patch that created a
> -                # new file.  This leaves it writable, which confuses p4.
> -                for f in pureRenameCopy:
> -                    p4_sync(f, "-f")
> -
> -            else:
> -                # skip this patch
> -                print "Submission cancelled, undoing p4 changes."
> -                for f in editedFiles:
> -                    p4_revert(f)
> -                for f in filesToAdd:
> -                    p4_revert(f)
> -                    os.remove(f)
> +            if self.preserveUser:
> +                if p4User:
> +                    # Get last changelist number. Cannot easily get it from
> +                    # the submit command output as the output is
> +                    # unmarshalled.
> +                    changelist = self.lastP4Changelist()
> +                    self.modifyChangelistUser(changelist, p4User)
> +
> +            # The rename/copy happened by applying a patch that created a
> +            # new file.  This leaves it writable, which confuses p4.
> +            for f in pureRenameCopy:
> +                p4_sync(f, "-f")
>
> -            os.remove(fileName)
>           else:
> -            fileName = "submit.txt"
> -            file = open(fileName, "w+")
> -            file.write(self.prepareLogMessage(template, logMessage))
> -            file.close()
> -            print ("Perforce submit template written as %s. "
> -                   + "Please review/edit and then use p4 submit -i<  %s to submit directly!"
> -                   % (fileName, fileName))
> +            # skip this patch
> +            print "Submission cancelled, undoing p4 changes."
> +            for f in editedFiles:
> +                p4_revert(f)
> +            for f in filesToAdd:
> +                p4_revert(f)
> +                os.remove(f)
> +
> +        os.remove(fileName)
>
>       # Export git tags as p4 labels. Create a p4 label and then tag
>       # with that.
> @@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):
>               commit = commits[0]
>               commits = commits[1:]
>               self.applyCommit(commit)
> -            if not self.interactive:
> -                break
>
>           if len(commits) == 0:
>               print "All changes applied!"

Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Pete Wyckoff]

luke@diamand.org wrote on Thu, 05 Jul 2012 08:20 +0100:
> On 04/07/12 14:34, Pete Wyckoff wrote:
> >The code is unused.  Delete.
> 
> I've used that non-interactive code path in the past, in the very
> early days of using it (setting interactive to false manually).
> 
> The nice thing about it is that if you're using git-p4 for the very
> first time it lets you do the final submission to p4 by hand,
> without having to trust the script to do the right thing. Once I
> convinced myself that git-p4 was doing the right thing, I then
> stopped using it.
> 
> Is it worth retaining, perhaps fixed so that it can be set on the
> command line and documented? Or just discard?

My biggest complaint is that there's no way to enable the option.
You have to edit the code to change self.interactive to False, as
you pointed out.

Then it doesn't help you with the submit message, and doesn't
do the little details of cleaning up pure-copied files or
changing the username for preserveUser.

What you're doing makes sense, though, but maybe there's a
cleaner way to provide that functionality.

We could build the change then say "type p4 submit -c ... if it
looks good".  Still doesn't handle the little details.

We could spawn a shell to let them go inspect.

We could try to implement a "--continue" option, and give them
a chance to edit.

I've got an upcoming series that changes the interaction loop on
conflict, and makes it easier to do some interaction at each
patch, possibly before applying too.  Might make things easier.

		-- Pete

Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Pete Wyckoff]

pw@padd.com wrote on Thu, 05 Jul 2012 08:30 -0400:
> luke@diamand.org wrote on Thu, 05 Jul 2012 08:20 +0100:
> > On 04/07/12 14:34, Pete Wyckoff wrote:
> > >The code is unused.  Delete.
> > 
> > I've used that non-interactive code path in the past, in the very
> > early days of using it (setting interactive to false manually).
> > 
> > The nice thing about it is that if you're using git-p4 for the very
> > first time it lets you do the final submission to p4 by hand,
> > without having to trust the script to do the right thing. Once I
> > convinced myself that git-p4 was doing the right thing, I then
> > stopped using it.
> > 
> > Is it worth retaining, perhaps fixed so that it can be set on the
> > command line and documented? Or just discard?
> 
> My biggest complaint is that there's no way to enable the option.
> You have to edit the code to change self.interactive to False, as
> you pointed out.
> 
> Then it doesn't help you with the submit message, and doesn't
> do the little details of cleaning up pure-copied files or
> changing the username for preserveUser.
> 
> What you're doing makes sense, though, but maybe there's a
> cleaner way to provide that functionality.
> 
> We could build the change then say "type p4 submit -c ... if it
> looks good".  Still doesn't handle the little details.
> 
> We could spawn a shell to let them go inspect.
> 
> We could try to implement a "--continue" option, and give them
> a chance to edit.
> 
> I've got an upcoming series that changes the interaction loop on
> conflict, and makes it easier to do some interaction at each
> patch, possibly before applying too.  Might make things easier.

I did code up two new options to "git p4 submit":

    --dry-run : just show the commits that would be submitted

    --prepare-p4-only : open/add, apply patch, but do not submit

The latter prints a rather lengthy message about how to submit
or revert the changes.  It fills the role of self.interactive,
hopefully.

I'll send the patches out for review once the other in-flight
changes have settled.

		-- Pete

[PATCH 2/3] git p4 test: refactor marshal_dump

[Pete Wyckoff]

This function will be useful in future tests.  Move it to
the git-p4 test library.  Let it accept an optional argument
to pick a certain marshaled object out of the input stream.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/lib-git-p4.sh         | 14 ++++++++++++++
 t/t9800-git-p4-basic.sh |  5 -----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 31d75ae..080b2c1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -102,3 +102,17 @@ cleanup_git() {
 	rm -rf "$git" &&
 	mkdir "$git"
 }
+
+marshal_dump() {
+	what=$1 &&
+	line=${2:-1} &&
+	cat >"$TRASH_DIRECTORY/marshal-dump.py" <<-EOF &&
+	import marshal
+	import sys
+	for i in range($line):
+	    d = marshal.load(sys.stdin)
+	print d['$what']
+	EOF
+	"$PYTHON_PATH" "$TRASH_DIRECTORY/marshal-dump.py"
+}
+
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 07c2e15..b7ad716 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -155,11 +155,6 @@ test_expect_success 'clone bare' '
 	)
 '
 
-marshal_dump() {
-	what=$1
-	"$PYTHON_PATH" -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
-}
-
 # Sleep a bit so that the top-most p4 change did not happen "now".  Then
 # import the repo and make sure that the initial import has the same time
 # as the top-most change.
-- 
1.7.11.1.125.g4a65fea

[PATCH 3/3] git p4: notice Jobs lines in git commit messages

[Pete Wyckoff]

P4 has a feature called "jobs" that allows linking changes
to a bug tracking system or other tasks.  When submitting
code, a job name can be specified to mark that this change
is associated with a particular job.

Teach git-p4 to find an optional "Jobs:" line in git commit
messages and use them to make a Jobs section in the p4
change specifitation.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py                |  46 ++++++++++++--
 t/t9807-git-p4-submit.sh | 155 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 542c20a..fa44817 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -854,9 +854,34 @@ class P4Submit(Command, P4UserMap):
         if len(p4CmdList("opened ...")) > 0:
             die("You have files opened with perforce! Close them before starting the sync.")
 
-    # replaces everything between 'Description:' and the next P4 submit template field with the
-    # commit message
-    def prepareLogMessage(self, template, message):
+    def separate_jobs_from_description(self, message):
+        """Extract and return a possible Jobs field in the commit
+           message.  It goes into a separate section in the p4 change
+           specification.
+
+           A jobs line starts with "Jobs:" and looks like a new field
+           in a form.  Values are white-space separated on the same
+           line or on following lines that start with a tab.
+
+           This does not parse and extract the full git commit message
+           like a p4 form.  It just sees the Jobs: line as a marker
+           to pass everything from then on directly into the p4 form,
+           but outside the description section.
+           
+           Return a tuple (stripped log message, jobs string)."""
+
+        m = re.search(r'^Jobs:', message, re.MULTILINE)
+        if m is None:
+            return (message, None)
+
+        jobtext = message[m.start():]
+        stripped_message = message[:m.start()].rstrip()
+        return (stripped_message, jobtext)
+
+    def prepareLogMessage(self, template, message, jobs):
+        """Edits the template returned from "p4 change -o" to insert
+           the message in the Description field, and the jobs text in
+           the Jobs field."""
         result = ""
 
         inDescriptionSection = False
@@ -869,6 +894,9 @@ class P4Submit(Command, P4UserMap):
             if inDescriptionSection:
                 if line.startswith("Files:") or line.startswith("Jobs:"):
                     inDescriptionSection = False
+                    # insert Jobs section
+                    if jobs:
+                        result += jobs + "\n"
                 else:
                     continue
             else:
@@ -980,7 +1008,13 @@ class P4Submit(Command, P4UserMap):
         return 0
 
     def prepareSubmitTemplate(self):
-        # remove lines in the Files section that show changes to files outside the depot path we're committing into
+        """Run "p4 change -o" to grab a change specification template.
+           This does not use "p4 -G", as it is nice to keep the submission
+           template in original order, since a human might edit it.
+
+           Remove lines in the Files section that show changes to files
+           outside the depot path we're committing into."""
+
         template = ""
         inFilesSection = False
         for line in p4_read_pipe_lines(['change', '-o']):
@@ -1205,10 +1239,10 @@ class P4Submit(Command, P4UserMap):
 
         logMessage = extractLogMessageFromGitCommit(id)
         logMessage = logMessage.strip()
+        (logMessage, jobs) = self.separate_jobs_from_description(logMessage)
 
         template = self.prepareSubmitTemplate()
-
-        submitTemplate = self.prepareLogMessage(template, logMessage)
+        submitTemplate = self.prepareLogMessage(template, logMessage, jobs)
 
         if self.preserveUser:
            submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index f23b4c3..d6d588c 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -182,6 +182,161 @@ test_expect_success 'submit rename' '
 	)
 '
 
+#
+# Converting git commit message to p4 change description, including
+# parsing out the optional Jobs: line.
+#
+test_expect_success 'simple one-line description' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo desc2 >desc2 &&
+		git add desc2 &&
+		cat >msg <<-EOF &&
+		One-line description line for desc2.
+		EOF
+		git commit -F - <msg &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit &&
+		change=$(p4 -G changes -m 1 //depot/... | \
+		         marshal_dump change) &&
+		# marshal_dump always adds a newline
+		p4 -G describe $change | marshal_dump desc | sed \$d >pmsg &&
+		test_cmp msg pmsg
+	)
+'
+
+test_expect_success 'description with odd formatting' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo desc3 >desc3 &&
+		git add desc3 &&
+		(
+			printf "subject line\n\n\tExtra tab\nline.\n\n" &&
+			printf "Description:\n\tBogus description marker\n\n" &&
+			# git commit eats trailing newlines; only use one
+			printf "Files:\n\tBogus descs marker\n"
+		) >msg &&
+		git commit -F - <msg &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit &&
+		change=$(p4 -G changes -m 1 //depot/... | \
+		         marshal_dump change) &&
+		# marshal_dump always adds a newline
+		p4 -G describe $change | marshal_dump desc | sed \$d >pmsg &&
+		test_cmp msg pmsg
+	)
+'
+
+make_job() {
+	name="$1" &&
+	tab="$(printf \\t)" &&
+	p4 job -o | \
+	sed -e "/^Job:/s/.*/Job: $name/" \
+	    -e "/^Description/{ n; s/.*/$tab job text/; }" | \
+	p4 job -i
+}
+
+test_expect_success 'description with Jobs section at end' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo desc4 >desc4 &&
+		git add desc4 &&
+		echo 6060842 >jobname &&
+		(
+			printf "subject line\n\n\tExtra tab\nline.\n\n" &&
+			printf "Files:\n\tBogus files marker\n" &&
+			printf "Junk: 3164175\n" &&
+			printf "Jobs: $(cat jobname)\n"
+		) >msg &&
+		git commit -F - <msg &&
+		git config git-p4.skipSubmitEdit true &&
+		# build a job
+		make_job $(cat jobname) &&
+		git p4 submit &&
+		change=$(p4 -G changes -m 1 //depot/... | \
+		         marshal_dump change) &&
+		# marshal_dump always adds a newline
+		p4 -G describe $change | marshal_dump desc | sed \$d >pmsg &&
+		# make sure Jobs line and all following is gone
+		sed "/^Jobs:/,\$d" msg >jmsg &&
+		test_cmp jmsg pmsg &&
+		# make sure p4 knows about job
+		p4 -G describe $change | marshal_dump job0 >job0 &&
+		test_cmp jobname job0
+	)
+'
+
+test_expect_success 'description with Jobs and values on separate lines' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo desc5 >desc5 &&
+		git add desc5 &&
+		echo PROJ-6060842 >jobname1 &&
+		echo PROJ-6060847 >jobname2 &&
+		(
+			printf "subject line\n\n\tExtra tab\nline.\n\n" &&
+			printf "Files:\n\tBogus files marker\n" &&
+			printf "Junk: 3164175\n" &&
+			printf "Jobs:\n" &&
+			printf "\t$(cat jobname1)\n" &&
+			printf "\t$(cat jobname2)\n"
+		) >msg &&
+		git commit -F - <msg &&
+		git config git-p4.skipSubmitEdit true &&
+		# build two jobs
+		make_job $(cat jobname1) &&
+		make_job $(cat jobname2) &&
+		git p4 submit &&
+		change=$(p4 -G changes -m 1 //depot/... | \
+		         marshal_dump change) &&
+		# marshal_dump always adds a newline
+		p4 -G describe $change | marshal_dump desc | sed \$d >pmsg &&
+		# make sure Jobs line and all following is gone
+		sed "/^Jobs:/,\$d" msg >jmsg &&
+		test_cmp jmsg pmsg &&
+		# make sure p4 knows about the two jobs
+		p4 -G describe $change >change &&
+		(
+			marshal_dump job0 <change &&
+			marshal_dump job1 <change
+		) | sort >jobs &&
+		cat jobname1 jobname2 | sort >expected &&
+		test_cmp expected jobs
+	)
+'
+
+test_expect_success 'description with Jobs section and bogus following text' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo desc6 >desc6 &&
+		git add desc6 &&
+		echo 6060843 >jobname &&
+		(
+			printf "subject line\n\n\tExtra tab\nline.\n\n" &&
+			printf "Files:\n\tBogus files marker\n" &&
+			printf "Junk: 3164175\n" &&
+			printf "Jobs: $(cat jobname)\n" &&
+			printf "MoreJunk: 3711\n"
+		) >msg &&
+		git commit -F - <msg &&
+		git config git-p4.skipSubmitEdit true &&
+		# build a job
+		make_job $(cat jobname) &&
+		test_must_fail git p4 submit 2>err &&
+		test_i18ngrep "Unknown field name" err
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.11.1.125.g4a65fea