[PATCHv2 0/6] git-p4: some small fixes updated

[PATCHv2 0/6] git-p4: some small fixes updated

[Luke Diamand]

This is an updated version of the set of changes I posted recently,
following comments on the list:

disable automatic sync after git-p4 submit:
    https://marc.info/?l=git&m=152818734814838&w=2

better handling of being logged out by Perforce:
   https://marc.info/?l=git&m=152818893815326&w=2

adapt the block size automatically on git-p4 submit:
   https://marc.info/?l=git&m=152819004315688&w=2

- Spelling corrections (Eric)
- Improved comments (Eric)
- Exception class hierarchy fix (Merland)
- test simplification (Eric)


Luke Diamand (6):
  git-p4: disable-rebase: allow setting this via configuration
  git-p4: add option to disable syncing of p4/master with p4
  git-p4: better error reporting when p4 fails
  git-p4: raise exceptions from p4CmdList based on error from p4 server
  git-p4: narrow the scope of exceptions caught when parsing an int
  git-p4: auto-size the block

 Documentation/git-p4.txt |  13 +++-
 git-p4.py                | 161 +++++++++++++++++++++++++++++++++------
 t/t9818-git-p4-block.sh  |   8 ++
 t/t9833-errors.sh        |  78 +++++++++++++++++++
 4 files changed, 236 insertions(+), 24 deletions(-)
 create mode 100755 t/t9833-errors.sh

-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 1/6] git-p4: disable-rebase: allow setting this via configuration

[Luke Diamand]

This just lets you set the --disable-rebase option with the
git configuration options git-p4.disableRebase. If you're
using this option, you probably want to set it all the time
for a given repo.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 Documentation/git-p4.txt | 5 ++++-
 git-p4.py                | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index e8452528fc..3d83842e47 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -367,7 +367,7 @@ These options can be used to modify 'git p4 submit' behavior.
 
 --disable-rebase::
     Disable the automatic rebase after all commits have been successfully
-    submitted.
+    submitted. Can also be set with git-p4.disableRebase.
 
 Rebase options
 ~~~~~~~~~~~~~~
@@ -690,6 +690,9 @@ git-p4.conflict::
 	Specify submit behavior when a conflict with p4 is found, as per
 	--conflict.  The default behavior is 'ask'.
 
+git-p4.disableRebase::
+    Do not rebase the tree against p4/master following a submit.
+
 IMPLEMENTATION DETAILS
 ----------------------
 * Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index c4581d20a6..5ab9421af8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1379,7 +1379,7 @@ def __init__(self):
         self.shelve = False
         self.update_shelve = list()
         self.commit = ""
-        self.disable_rebase = False
+        self.disable_rebase = gitConfigBool("git-p4.disableRebase")
         self.prepare_p4_only = False
         self.conflict_behavior = None
         self.isWindows = (platform.system() == "Windows")
-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 2/6] git-p4: add option to disable syncing of p4/master with p4

[Luke Diamand]

Add an option to the git-p4 submit command to disable syncing
with Perforce.

This is useful for the case where a git-p4 mirror has been setup
on a server somewhere, running from (e.g.) cron, and developers
then clone from this. Having the local cloned copy also sync
from Perforce just isn't useful.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 Documentation/git-p4.txt |  8 ++++++++
 git-p4.py                | 31 ++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 3d83842e47..f0de3b891b 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -369,6 +369,11 @@ These options can be used to modify 'git p4 submit' behavior.
     Disable the automatic rebase after all commits have been successfully
     submitted. Can also be set with git-p4.disableRebase.
 
+--disable-p4sync::
+    Disable the automatic sync of p4/master from Perforce after commits have
+    been submitted. Implies --disable-rebase. Can also be set with
+    git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+
 Rebase options
 ~~~~~~~~~~~~~~
 These options can be used to modify 'git p4 rebase' behavior.
@@ -693,6 +698,9 @@ git-p4.conflict::
 git-p4.disableRebase::
     Do not rebase the tree against p4/master following a submit.
 
+git-p4.disableP4Sync::
+    Do not sync p4/master with Perforce following a submit. Implies git-p4.disableRebase.
+
 IMPLEMENTATION DETAILS
 ----------------------
 * Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index 5ab9421af8..b61f47cc61 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1368,7 +1368,9 @@ def __init__(self):
                                      help="submit only the specified commit(s), one commit or xxx..xxx"),
                 optparse.make_option("--disable-rebase", dest="disable_rebase", action="store_true",
                                      help="Disable rebase after submit is completed. Can be useful if you "
-                                     "work from a local git branch that is not master")
+                                     "work from a local git branch that is not master"),
+                optparse.make_option("--disable-p4sync", dest="disable_p4sync", action="store_true",
+                                     help="Skip Perforce sync of p4/master after submit or shelve"),
         ]
         self.description = "Submit changes from git to the perforce depot."
         self.usage += " [name of git branch to submit into perforce depot]"
@@ -1380,6 +1382,7 @@ def __init__(self):
         self.update_shelve = list()
         self.commit = ""
         self.disable_rebase = gitConfigBool("git-p4.disableRebase")
+        self.disable_p4sync = gitConfigBool("git-p4.disableP4Sync")
         self.prepare_p4_only = False
         self.conflict_behavior = None
         self.isWindows = (platform.system() == "Windows")
@@ -2240,11 +2243,14 @@ def run(self, args):
             sync = P4Sync()
             if self.branch:
                 sync.branch = self.branch
-            sync.run([])
+            if self.disable_p4sync:
+                sync.sync_origin_only()
+            else:
+                sync.run([])
 
-            if self.disable_rebase is False:
-                rebase = P4Rebase()
-                rebase.rebase()
+                if not self.disable_rebase:
+                    rebase = P4Rebase()
+                    rebase.rebase()
 
         else:
             if len(applied) == 0:
@@ -3324,6 +3330,14 @@ def importChanges(self, changes, shelved=False, origin_revision=0):
                 print self.gitError.read()
                 sys.exit(1)
 
+    def sync_origin_only(self):
+        if self.syncWithOrigin:
+            self.hasOrigin = originP4BranchesExist()
+            if self.hasOrigin:
+                if not self.silent:
+                    print 'Syncing with origin first, using "git fetch origin"'
+                system("git fetch origin")
+
     def importHeadRevision(self, revision):
         print "Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch)
 
@@ -3402,12 +3416,7 @@ def run(self, args):
         else:
             self.refPrefix = "refs/heads/p4/"
 
-        if self.syncWithOrigin:
-            self.hasOrigin = originP4BranchesExist()
-            if self.hasOrigin:
-                if not self.silent:
-                    print 'Syncing with origin first, using "git fetch origin"'
-                system("git fetch origin")
+        self.sync_origin_only()
 
         branch_arg_given = bool(self.branch)
         if len(self.branch) == 0:
-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 3/6] git-p4: better error reporting when p4 fails

[Luke Diamand]

Currently when p4 fails to run, git-p4 just crashes with an obscure
error message.

For example, if the P4 ticket has expired, you get:

  Error: Cannot locate perforce checkout of <path> in client view

This change checks whether git-p4 can talk to the Perforce server when
the first P4 operation is attempted, and tries to print a meaningful
error message if it fails.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py         | 55 +++++++++++++++++++++++++++++++++
 t/t9833-errors.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+)
 create mode 100755 t/t9833-errors.sh

diff --git a/git-p4.py b/git-p4.py
index b61f47cc61..3de12a4a0a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -50,6 +50,8 @@ def __str__(self):
 # Grab changes in blocks of this many revisions, unless otherwise requested
 defaultBlockSize = 512
 
+p4_access_checked = False
+
 def p4_build_cmd(cmd):
     """Build a suitable p4 command line.
 
@@ -91,6 +93,13 @@ def p4_build_cmd(cmd):
         real_cmd = ' '.join(real_cmd) + ' ' + cmd
     else:
         real_cmd += cmd
+
+    # now check that we can actually talk to the server
+    global p4_access_checked
+    if not p4_access_checked:
+        p4_access_checked = True    # suppress access checks in p4_check_access itself
+        p4_check_access()
+
     return real_cmd
 
 def git_dir(path):
@@ -264,6 +273,52 @@ def p4_system(cmd):
     if retcode:
         raise CalledProcessError(retcode, real_cmd)
 
+def die_bad_access(s):
+    die("failure accessing depot: {0}".format(s.rstrip()))
+
+def p4_check_access(min_expiration=1):
+    """ Check if we can access Perforce - account still logged in
+    """
+    results = p4CmdList(["login", "-s"])
+
+    if len(results) == 0:
+        # should never get here: always get either some results, or a p4ExitCode
+        assert("could not parse response from perforce")
+
+    result = results[0]
+
+    if 'p4ExitCode' in result:
+        # p4 returned non-zero status, e.g. P4PORT invalid, or p4 not in path
+        die_bad_access("could not run p4")
+
+    code = result.get("code")
+    if not code:
+        # we get here if we couldn't connect and there was nothing to unmarshal
+        die_bad_access("could not connect")
+
+    elif code == "stat":
+        expiry = result.get("TicketExpiration")
+        if expiry:
+            expiry = int(expiry)
+            if expiry > min_expiration:
+                # ok to carry on
+                return
+            else:
+                die_bad_access("perforce ticket expires in {0} seconds".format(expiry))
+
+        else:
+            # account without a timeout - all ok
+            return
+
+    elif code == "error":
+        data = result.get("data")
+        if data:
+            die_bad_access("p4 error: {0}".format(data))
+        else:
+            die_bad_access("unknown error")
+    else:
+        die_bad_access("unknown error code {0}".format(code))
+
 _p4_version_string = None
 def p4_version_string():
     """Read the version string, showing just the last line, which
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
new file mode 100755
index 0000000000..9ba892de7a
--- /dev/null
+++ b/t/t9833-errors.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='git p4 errors'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'add p4 files' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "file1"
+	)
+'
+
+# after this test, the default user requires a password
+test_expect_success 'error handling' '
+	git p4 clone --dest="$git" //depot@all &&
+	(
+		cd "$git" &&
+		P4PORT=: test_must_fail git p4 submit 2>errmsg
+	) &&
+	p4 passwd -P newpassword &&
+	(
+		P4PASSWD=badpassword test_must_fail git p4 clone //depot/foo 2>errmsg &&
+		grep -q "failure accessing depot.*P4PASSWD" errmsg
+	)
+'
+
+test_expect_success 'ticket logged out' '
+	P4TICKETS="$cli/tickets" &&
+	echo "newpassword" | p4 login &&
+	(
+		cd "$git" &&
+		test_commit "ticket-auth-check" &&
+		p4 logout &&
+		test_must_fail git p4 submit 2>errmsg &&
+		grep -q "failure accessing depot" errmsg
+	)
+'
+
+test_expect_success 'create group with short ticket expiry' '
+	P4TICKETS="$cli/tickets" &&
+	echo "newpassword" | p4 login &&
+	p4_add_user short_expiry_user &&
+	p4 -u short_expiry_user passwd -P password &&
+	p4 group -i <<-EOF &&
+	Group: testgroup
+	Timeout: 3
+	Users: short_expiry_user
+	EOF
+
+	p4 users | grep short_expiry_user
+'
+
+test_expect_success 'git operation with expired ticket' '
+	P4TICKETS="$cli/tickets" &&
+	P4USER=short_expiry_user &&
+	echo "password" | p4 login &&
+	(
+		cd "$git" &&
+		git p4 sync &&
+		sleep 5 &&
+		test_must_fail git p4 sync 2>errmsg &&
+		grep "failure accessing depot" errmsg
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+
+test_done
-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 4/6] git-p4: raise exceptions from p4CmdList based on error from p4 server

[Luke Diamand]

This change lays some groundwork for better handling of rowcount errors
from the server, where it fails to send us results because we requested
too many.

It adds an option to p4CmdList() to return errors as a Python exception.

The exceptions are derived from P4Exception (something went wrong),
P4ServerException (the server sent us an error code) and
P4RequestSizeException (we requested too many rows/results from the
server database).

This makes the code that handles the errors a bit easier.

The default behavior is unchanged; the new code is enabled with a flag.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 44 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 3de12a4a0a..f4b5deeb83 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -566,10 +566,30 @@ def isModeExec(mode):
     # otherwise False.
     return mode[-3:] == "755"
 
+class P4Exception(Exception):
+    """ Base class for exceptions from the p4 client """
+    def __init__(self, exit_code):
+        self.p4ExitCode = exit_code
+
+class P4ServerException(P4Exception):
+    """ Base class for exceptions where we get some kind of marshalled up result from the server """
+    def __init__(self, exit_code, p4_result):
+        super(P4ServerException, self).__init__(exit_code)
+        self.p4_result = p4_result
+        self.code = p4_result[0]['code']
+        self.data = p4_result[0]['data']
+
+class P4RequestSizeException(P4ServerException):
+    """ One of the maxresults or maxscanrows errors """
+    def __init__(self, exit_code, p4_result, limit):
+        super(P4RequestSizeException, self).__init__(exit_code, p4_result)
+        self.limit = limit
+
 def isModeExecChanged(src_mode, dst_mode):
     return isModeExec(src_mode) != isModeExec(dst_mode)
 
-def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):
+def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
+        errors_as_exceptions=False):
 
     if isinstance(cmd,basestring):
         cmd = "-G " + cmd
@@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):
         pass
     exitCode = p4.wait()
     if exitCode != 0:
-        entry = {}
-        entry["p4ExitCode"] = exitCode
-        result.append(entry)
+        if errors_as_exceptions:
+            if len(result) > 0:
+                data = result[0].get('data')
+                if data:
+                    m = re.search('Too many rows scanned \(over (\d+)\)', data)
+                    if not m:
+                        m = re.search('Request too large \(over (\d+)\)', data)
+
+                    if m:
+                        limit = int(m.group(1))
+                        raise P4RequestSizeException(exitCode, result, limit)
+
+                raise P4ServerException(exitCode, result)
+            else:
+                raise P4Exception(exitCode)
+        else:
+            entry = {}
+            entry["p4ExitCode"] = exitCode
+            result.append(entry)
 
     return result
 
-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 5/6] git-p4: narrow the scope of exceptions caught when parsing an int

[Luke Diamand]

The current code traps all exceptions around some code which parses an
integer, and then talks to Perforce.

That can result in errors from Perforce being ignored. Change the code
to only catch the integer conversion exceptions.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index f4b5deeb83..5f59feb5bb 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -959,7 +959,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
         try:
             (changeStart, changeEnd) = p4ParseNumericChangeRange(parts)
             block_size = chooseBlockSize(requestedBlockSize)
-        except:
+        except ValueError:
             changeStart = parts[0][1:]
             changeEnd = parts[1]
             if requestedBlockSize:
-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv2 6/6] git-p4: auto-size the block

[Luke Diamand]

git-p4 originally would fetch changes in one query. On large repos this
could fail because of the limits that Perforce imposes on the number of
items returned and the number of queries in the database.

To fix this, git-p4 learned to query changes in blocks of 512 changes,
However, this can be very slow - if you have a few million changes,
with each chunk taking about a second, it can be an hour or so.

Although it's possible to tune this value manually with the
"--changes-block-size" option, it's far from obvious to ordinary users
that this is what needs doing.

This change alters the block size dynamically by looking for the
specific error messages returned from the Perforce server, and reducing
the block size if the error is seen, either to the limit reported by the
server, or to half the current block size.

That means we can start out with a very large block size, and then let
it automatically drop down to a value that works without error, while
still failing correctly if some other error occurs.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py               | 27 +++++++++++++++++++++------
 t/t9818-git-p4-block.sh |  8 ++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 5f59feb5bb..0354d4df5c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -47,8 +47,8 @@ def __str__(self):
 # Only labels/tags matching this will be imported/exported
 defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
 
-# Grab changes in blocks of this many revisions, unless otherwise requested
-defaultBlockSize = 512
+# The block size is reduced automatically if required
+defaultBlockSize = 1<<20
 
 p4_access_checked = False
 
@@ -969,7 +969,8 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
     changes = set()
 
     # Retrieve changes a block at a time, to prevent running
-    # into a MaxResults/MaxScanRows error from the server.
+    # into a MaxResults/MaxScanRows error from the server. If
+    # we _do_ hit one of those errors, turn down the block size
 
     while True:
         cmd = ['changes']
@@ -983,10 +984,24 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
         for p in depotPaths:
             cmd += ["%s...@%s" % (p, revisionRange)]
 
+        # fetch the changes
+        try:
+            result = p4CmdList(cmd, errors_as_exceptions=True)
+        except P4RequestSizeException as e:
+            if not block_size:
+                block_size = e.limit
+            elif block_size > e.limit:
+                block_size = e.limit
+            else:
+                block_size = max(2, block_size // 2)
+
+            if verbose: print("block size error, retrying with block size {0}".format(block_size))
+            continue
+        except P4Exception as e:
+            die('Error retrieving changes description ({0})'.format(e.p4ExitCode))
+
         # Insert changes in chronological order
-        for entry in reversed(p4CmdList(cmd)):
-            if entry.has_key('p4ExitCode'):
-                die('Error retrieving changes descriptions ({})'.format(entry['p4ExitCode']))
+        for entry in reversed(result):
             if not entry.has_key('change'):
                 continue
             changes.add(int(entry['change']))
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 8840a183ac..ce7cb22ad3 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -129,6 +129,7 @@ test_expect_success 'Create a repo with multiple depot paths' '
 '
 
 test_expect_success 'Clone repo with multiple depot paths' '
+	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
 		git p4 clone --changes-block-size=4 //depot/pathA@all //depot/pathB@all \
@@ -138,6 +139,13 @@ test_expect_success 'Clone repo with multiple depot paths' '
 	)
 '
 
+test_expect_success 'Clone repo with self-sizing block size' '
+	test_when_finished cleanup_git &&
+	git p4 clone --changes-block-size=1000000 //depot@all --destination="$git" &&
+	git -C "$git" log --oneline >log &&
+	test_line_count \> 10 log
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
2.17.0.392.gdeb1a6e9b7

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Junio C Hamano]

Luke Diamand <luke@diamand.org> writes:

> This is an updated version of the set of changes I posted recently,
> following comments on the list:
>
> disable automatic sync after git-p4 submit:
>     https://marc.info/?l=git&m=152818734814838&w=2
>
> better handling of being logged out by Perforce:
>    https://marc.info/?l=git&m=152818893815326&w=2
>
> adapt the block size automatically on git-p4 submit:
>    https://marc.info/?l=git&m=152819004315688&w=2
>
> - Spelling corrections (Eric)
> - Improved comments (Eric)
> - Exception class hierarchy fix (Merland)
> - test simplification (Eric)
>

That reminds me of one thing.  

This 6-patch series depends on the rm/p4-submit-with-commit-option
that came without and still waiting for a sign-off by the original
author.  Also I do not think the original patch reached the public
list, so I'm attaching the patch to make sure people know which
patch I am talking about.

Romain, can we get your sign-off on the patch you sent earlier?

Thanks.

-- >8 --
From: Romain Merland <merlorom@yahoo.fr>
Date: Wed, 9 May 2018 17:32:12 +0200
Subject: [PATCH] git-p4: add options --commit and --disable-rebase

On a daily work with multiple local git branches, the usual way to
submit only a specified commit was to cherry-pick the commit on
master then run git-p4 submit.  It can be very annoying to switch
between local branches and master, only to submit one commit.  The
proposed new way is to select directly the commit you want to
submit.

Add option --commit to command 'git-p4 submit' in order to submit
only specified commit(s) in p4.

On a daily work developping software with big compilation time, one
may not want to rebase on his local git tree, in order to avoid long
recompilation.

Add option --disable-rebase to command 'git-p4 submit' in order to
disable rebase after submission.

Reviewed-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-p4.txt | 14 ++++++++++++++
 git-p4.py                | 29 +++++++++++++++++++++++------
 t/t9807-git-p4-submit.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..88d109debb 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -149,6 +149,12 @@ To specify a branch other than the current one, use:
 $ git p4 submit topicbranch
 ------------
 
+To specify a single commit or a range of commits, use:
+------------
+$ git p4 submit --commit <sha1>
+$ git p4 submit --commit <sha1..sha1>
+------------
+
 The upstream reference is generally 'refs/remotes/p4/master', but can
 be overridden using the `--origin=` command-line option.
 
@@ -330,6 +336,14 @@ These options can be used to modify 'git p4 submit' behavior.
 	p4/master.  See the "Sync options" section above for more
 	information.
 
+--commit <sha1>|<sha1..sha1>::
+    Submit only the specified commit or range of commits, instead of the full
+    list of changes that are in the current Git branch.
+
+--disable-rebase::
+    Disable the automatic rebase after all commits have been successfully
+    submitted.
+
 Rebase options
 ~~~~~~~~~~~~~~
 These options can be used to modify 'git p4 rebase' behavior.
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..f4a6f3b4c3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1352,7 +1352,12 @@ def __init__(self):
                 optparse.make_option("--update-shelve", dest="update_shelve", action="append", type="int",
                                      metavar="CHANGELIST",
                                      help="update an existing shelved changelist, implies --shelve, "
-                                           "repeat in-order for multiple shelved changelists")
+                                           "repeat in-order for multiple shelved changelists"),
+                optparse.make_option("--commit", dest="commit", metavar="COMMIT",
+                                     help="submit only the specified commit(s), one commit or xxx..xxx"),
+                optparse.make_option("--disable-rebase", dest="disable_rebase", action="store_true",
+                                     help="Disable rebase after submit is completed. Can be useful if you "
+                                     "work from a local git branch that is not master")
         ]
         self.description = "Submit changes from git to the perforce depot."
         self.usage += " [name of git branch to submit into perforce depot]"
@@ -1362,6 +1367,8 @@ def __init__(self):
         self.dry_run = False
         self.shelve = False
         self.update_shelve = list()
+        self.commit = ""
+        self.disable_rebase = False
         self.prepare_p4_only = False
         self.conflict_behavior = None
         self.isWindows = (platform.system() == "Windows")
@@ -2103,9 +2110,18 @@ def run(self, args):
         else:
             commitish = 'HEAD'
 
-        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
-            commits.append(line.strip())
-        commits.reverse()
+        if self.commit != "":
+            if self.commit.find("..") != -1:
+                limits_ish = self.commit.split("..")
+                for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (limits_ish[0], limits_ish[1])]):
+                    commits.append(line.strip())
+                commits.reverse()
+            else:
+                commits.append(self.commit)
+        else:
+            for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
+                commits.append(line.strip())
+            commits.reverse()
 
         if self.preserveUser or gitConfigBool("git-p4.skipUserNameCheck"):
             self.checkAuthorship = False
@@ -2215,8 +2231,9 @@ def run(self, args):
                 sync.branch = self.branch
             sync.run([])
 
-            rebase = P4Rebase()
-            rebase.rebase()
+            if self.disable_rebase is False:
+                rebase = P4Rebase()
+                rebase.rebase()
 
         else:
             if len(applied) == 0:
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 71cae2874d..2325599ee6 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -155,6 +155,46 @@ test_expect_success 'allow submit from branch with same revision but different n
 	)
 '
 
+# make two commits, but tell it to apply only one
+
+test_expect_success 'submit --commit one' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_commit "file9" &&
+		test_commit "file10" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --commit HEAD
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing "file9.t" &&
+		test_path_is_file "file10.t"
+	)
+'
+
+# make three commits, but tell it to apply only range
+
+test_expect_success 'submit --commit range' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_commit "file11" &&
+		test_commit "file12" &&
+		test_commit "file13" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit --commit HEAD~2..HEAD
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing "file11.t" &&
+		test_path_is_file "file12.t" &&
+		test_path_is_file "file13.t"
+	)
+'
+
 #
 # Basic submit tests, the five handled cases
 #
-- 
2.18.0-rc1-1-g6f333ff2fb

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Luke Diamand]

On 12 June 2018 at 18:10, Junio C Hamano <gitster@pobox.com> wrote:
> Luke Diamand <luke@diamand.org> writes:
>
>> This is an updated version of the set of changes I posted recently,
>> following comments on the list:
>>
>> disable automatic sync after git-p4 submit:
>>     https://marc.info/?l=git&m=152818734814838&w=2
>>
>> better handling of being logged out by Perforce:
>>    https://marc.info/?l=git&m=152818893815326&w=2
>>
>> adapt the block size automatically on git-p4 submit:
>>    https://marc.info/?l=git&m=152819004315688&w=2
>>
>> - Spelling corrections (Eric)
>> - Improved comments (Eric)
>> - Exception class hierarchy fix (Merland)
>> - test simplification (Eric)
>>
>
> That reminds me of one thing.
>
> This 6-patch series depends on the rm/p4-submit-with-commit-option
> that came without and still waiting for a sign-off by the original
> author.  Also I do not think the original patch reached the public
> list, so I'm attaching the patch to make sure people know which
> patch I am talking about.
>
> Romain, can we get your sign-off on the patch you sent earlier?

Wasn't it already sent in this message:

https://marc.info/?l=git&m=152783923418317&w=2

Luke


>
> Thanks.
>
> -- >8 --
> From: Romain Merland <merlorom@yahoo.fr>
> Date: Wed, 9 May 2018 17:32:12 +0200
> Subject: [PATCH] git-p4: add options --commit and --disable-rebase
>
> On a daily work with multiple local git branches, the usual way to
> submit only a specified commit was to cherry-pick the commit on
> master then run git-p4 submit.  It can be very annoying to switch
> between local branches and master, only to submit one commit.  The
> proposed new way is to select directly the commit you want to
> submit.
>
> Add option --commit to command 'git-p4 submit' in order to submit
> only specified commit(s) in p4.
>
> On a daily work developping software with big compilation time, one
> may not want to rebase on his local git tree, in order to avoid long
> recompilation.
>
> Add option --disable-rebase to command 'git-p4 submit' in order to
> disable rebase after submission.
>
> Reviewed-by: Luke Diamand <luke@diamand.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-p4.txt | 14 ++++++++++++++
>  git-p4.py                | 29 +++++++++++++++++++++++------
>  t/t9807-git-p4-submit.sh | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9f..88d109debb 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -149,6 +149,12 @@ To specify a branch other than the current one, use:
>  $ git p4 submit topicbranch
>  ------------
>
> +To specify a single commit or a range of commits, use:
> +------------
> +$ git p4 submit --commit <sha1>
> +$ git p4 submit --commit <sha1..sha1>
> +------------
> +
>  The upstream reference is generally 'refs/remotes/p4/master', but can
>  be overridden using the `--origin=` command-line option.
>
> @@ -330,6 +336,14 @@ These options can be used to modify 'git p4 submit' behavior.
>         p4/master.  See the "Sync options" section above for more
>         information.
>
> +--commit <sha1>|<sha1..sha1>::
> +    Submit only the specified commit or range of commits, instead of the full
> +    list of changes that are in the current Git branch.
> +
> +--disable-rebase::
> +    Disable the automatic rebase after all commits have been successfully
> +    submitted.
> +
>  Rebase options
>  ~~~~~~~~~~~~~~
>  These options can be used to modify 'git p4 rebase' behavior.
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc69..f4a6f3b4c3 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1352,7 +1352,12 @@ def __init__(self):
>                  optparse.make_option("--update-shelve", dest="update_shelve", action="append", type="int",
>                                       metavar="CHANGELIST",
>                                       help="update an existing shelved changelist, implies --shelve, "
> -                                           "repeat in-order for multiple shelved changelists")
> +                                           "repeat in-order for multiple shelved changelists"),
> +                optparse.make_option("--commit", dest="commit", metavar="COMMIT",
> +                                     help="submit only the specified commit(s), one commit or xxx..xxx"),
> +                optparse.make_option("--disable-rebase", dest="disable_rebase", action="store_true",
> +                                     help="Disable rebase after submit is completed. Can be useful if you "
> +                                     "work from a local git branch that is not master")
>          ]
>          self.description = "Submit changes from git to the perforce depot."
>          self.usage += " [name of git branch to submit into perforce depot]"
> @@ -1362,6 +1367,8 @@ def __init__(self):
>          self.dry_run = False
>          self.shelve = False
>          self.update_shelve = list()
> +        self.commit = ""
> +        self.disable_rebase = False
>          self.prepare_p4_only = False
>          self.conflict_behavior = None
>          self.isWindows = (platform.system() == "Windows")
> @@ -2103,9 +2110,18 @@ def run(self, args):
>          else:
>              commitish = 'HEAD'
>
> -        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
> -            commits.append(line.strip())
> -        commits.reverse()
> +        if self.commit != "":
> +            if self.commit.find("..") != -1:
> +                limits_ish = self.commit.split("..")
> +                for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (limits_ish[0], limits_ish[1])]):
> +                    commits.append(line.strip())
> +                commits.reverse()
> +            else:
> +                commits.append(self.commit)
> +        else:
> +            for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
> +                commits.append(line.strip())
> +            commits.reverse()
>
>          if self.preserveUser or gitConfigBool("git-p4.skipUserNameCheck"):
>              self.checkAuthorship = False
> @@ -2215,8 +2231,9 @@ def run(self, args):
>                  sync.branch = self.branch
>              sync.run([])
>
> -            rebase = P4Rebase()
> -            rebase.rebase()
> +            if self.disable_rebase is False:
> +                rebase = P4Rebase()
> +                rebase.rebase()
>
>          else:
>              if len(applied) == 0:
> diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
> index 71cae2874d..2325599ee6 100755
> --- a/t/t9807-git-p4-submit.sh
> +++ b/t/t9807-git-p4-submit.sh
> @@ -155,6 +155,46 @@ test_expect_success 'allow submit from branch with same revision but different n
>         )
>  '
>
> +# make two commits, but tell it to apply only one
> +
> +test_expect_success 'submit --commit one' '
> +       test_when_finished cleanup_git &&
> +       git p4 clone --dest="$git" //depot &&
> +       (
> +               cd "$git" &&
> +               test_commit "file9" &&
> +               test_commit "file10" &&
> +               git config git-p4.skipSubmitEdit true &&
> +               git p4 submit --commit HEAD
> +       ) &&
> +       (
> +               cd "$cli" &&
> +               test_path_is_missing "file9.t" &&
> +               test_path_is_file "file10.t"
> +       )
> +'
> +
> +# make three commits, but tell it to apply only range
> +
> +test_expect_success 'submit --commit range' '
> +       test_when_finished cleanup_git &&
> +       git p4 clone --dest="$git" //depot &&
> +       (
> +               cd "$git" &&
> +               test_commit "file11" &&
> +               test_commit "file12" &&
> +               test_commit "file13" &&
> +               git config git-p4.skipSubmitEdit true &&
> +               git p4 submit --commit HEAD~2..HEAD
> +       ) &&
> +       (
> +               cd "$cli" &&
> +               test_path_is_missing "file11.t" &&
> +               test_path_is_file "file12.t" &&
> +               test_path_is_file "file13.t"
> +       )
> +'
> +
>  #
>  # Basic submit tests, the five handled cases
>  #
> --
> 2.18.0-rc1-1-g6f333ff2fb
>

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Junio C Hamano]

Luke Diamand <luke@diamand.org> writes:

> On 12 June 2018 at 18:10, Junio C Hamano <gitster@pobox.com> wrote:
>> Luke Diamand <luke@diamand.org> writes:
>>
>>> This is an updated version of the set of changes I posted recently,
>>> following comments on the list:
>>>
>>> disable automatic sync after git-p4 submit:
>>>     https://marc.info/?l=git&m=152818734814838&w=2
>>>
>>> better handling of being logged out by Perforce:
>>>    https://marc.info/?l=git&m=152818893815326&w=2
>>>
>>> adapt the block size automatically on git-p4 submit:
>>>    https://marc.info/?l=git&m=152819004315688&w=2
>>>
>>> - Spelling corrections (Eric)
>>> - Improved comments (Eric)
>>> - Exception class hierarchy fix (Merland)
>>> - test simplification (Eric)
>>>
>>
>> That reminds me of one thing.
>>
>> This 6-patch series depends on the rm/p4-submit-with-commit-option
>> that came without and still waiting for a sign-off by the original
>> author.  Also I do not think the original patch reached the public
>> list, so I'm attaching the patch to make sure people know which
>> patch I am talking about.
>>
>> Romain, can we get your sign-off on the patch you sent earlier?
>
> Wasn't it already sent in this message:
>
> https://marc.info/?l=git&m=152783923418317&w=2
>
> Luke


Thanks for a pointer.  Will replace and requeue.

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Luke Diamand]

On 12 June 2018 at 22:35, Junio C Hamano <gitster@pobox.com> wrote:
> Luke Diamand <luke@diamand.org> writes:
>
>> On 12 June 2018 at 18:10, Junio C Hamano <gitster@pobox.com> wrote:
>>> Luke Diamand <luke@diamand.org> writes:
>>>
>>>> This is an updated version of the set of changes I posted recently,
>>>> following comments on the list:
>>>>
>>>> disable automatic sync after git-p4 submit:
>>>>     https://marc.info/?l=git&m=152818734814838&w=2
>>>>
>>>> better handling of being logged out by Perforce:
>>>>    https://marc.info/?l=git&m=152818893815326&w=2
>>>>
>>>> adapt the block size automatically on git-p4 submit:
>>>>    https://marc.info/?l=git&m=152819004315688&w=2
>>>>
>>>> - Spelling corrections (Eric)
>>>> - Improved comments (Eric)
>>>> - Exception class hierarchy fix (Merland)
>>>> - test simplification (Eric)
>>>>
>>>
>>> That reminds me of one thing.
>>>
>>> This 6-patch series depends on the rm/p4-submit-with-commit-option
>>> that came without and still waiting for a sign-off by the original
>>> author.  Also I do not think the original patch reached the public
>>> list, so I'm attaching the patch to make sure people know which
>>> patch I am talking about.
>>>
>>> Romain, can we get your sign-off on the patch you sent earlier?
>>
>> Wasn't it already sent in this message:
>>
>> https://marc.info/?l=git&m=152783923418317&w=2
>>
>> Luke
>
>
> Thanks for a pointer.  Will replace and requeue.

Thanks. While on the subject of git-p4, I thought I should mention
that I've been looking at getting git-p4 to work with Python3.

I've got some low risk easy (mostly automated) changes which get it to
the point where it compiles. After that I have to figure out how to
fix the byte-vs-string unicode problem that Python3 brings. Having
been playing around with it, I think it should be possible to make the
same git-p4.py script work with 2.7, 3.6 and probably 2.6, although
I'm still some way from making this last bit work.

Luke

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Eric Sunshine]

On Tue, Jun 12, 2018 at 5:49 PM, Luke Diamand <luke@diamand.org> wrote:
> Thanks. While on the subject of git-p4, I thought I should mention
> that I've been looking at getting git-p4 to work with Python3.
>
> I've got some low risk easy (mostly automated) changes which get it to
> the point where it compiles. After that I have to figure out how to
> fix the byte-vs-string unicode problem that Python3 brings. [...]

See how reposurgeon handles the problem[1]. It defines polystr() and
polybytes() functions which coerce strings and byte sequences as
needed. It's not pretty, but it works.

[1]: https://gitlab.com/esr/reposurgeon/blob/master/reposurgeon#L100

Re: [PATCHv2 0/6] git-p4: some small fixes updated

[Luke Diamand]

On 12 June 2018 at 22:53, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Jun 12, 2018 at 5:49 PM, Luke Diamand <luke@diamand.org> wrote:
>> Thanks. While on the subject of git-p4, I thought I should mention
>> that I've been looking at getting git-p4 to work with Python3.
>>
>> I've got some low risk easy (mostly automated) changes which get it to
>> the point where it compiles. After that I have to figure out how to
>> fix the byte-vs-string unicode problem that Python3 brings. [...]
>
> See how reposurgeon handles the problem[1]. It defines polystr() and
> polybytes() functions which coerce strings and byte sequences as
> needed. It's not pretty, but it works.
>
> [1]: https://gitlab.com/esr/reposurgeon/blob/master/reposurgeon#L100

Thanks, that's got some useful tips!