[PATCH 0/1] git-p4: add unshelve command

[PATCH 0/1] git-p4: add unshelve command

[Luke Diamand]

This is an initial attempt at adding an unshelve command to
git-p4.

For those not familiar with it, p4 shelve creates a "pending"
changelist, which isn't committed into the central repo but is
nonetheless visible to other develoeprs. The "unshelve" command
takes one of these pending changelists and applies it to your repo.
It is used quite a lot for code review.

git-p4 learned about shelving changelists recently; this completes
the picture by letting you unshelve them as well.

This was inspired by the stackoverflow answer here:

    https://stackoverflow.com/questions/41841917/git-p4-how-to-fetch-a-changelist

The secret is to use the "p4 print file@=N" syntax to get the
contents of a shelved changelist, which has long perplexed me.

I haven't used this a great deal, so it may still have a few rough
edges.

In particular, it currently puts the unshelved commit into
    refs/remotes/p4/unshelved/<N>

where <N> is the changelist being unshelved. That might not be
the best way to do this.


Luke Diamand (1):
  git-p4: add unshelve command

 Documentation/git-p4.txt |  22 ++++++++
 git-p4.py                | 128 +++++++++++++++++++++++++++++++++++------------
 t/t9832-unshelve.sh      |  67 +++++++++++++++++++++++++
 3 files changed, 186 insertions(+), 31 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

-- 
2.15.1.272.gc310869385

[PATCH 1/1] git-p4: add unshelve command

[Luke Diamand]

This can be used to "unshelve" a shelved P4 commit into
a git commit.

For example:

  $ git p4 unshelve 12345

The resulting commit ends up in the branch:
   refs/remotes/p4/unshelved/12345

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 Documentation/git-p4.txt |  22 ++++++++
 git-p4.py                | 128 +++++++++++++++++++++++++++++++++++------------
 t/t9832-unshelve.sh      |  67 +++++++++++++++++++++++++
 3 files changed, 186 insertions(+), 31 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..910ae63a1c 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,21 @@ $ git p4 submit --shelve
 $ git p4 submit --update-shelve 1234 --update-shelve 2345
 ----
 
+
+Unshelve
+~~~~~~~~
+Unshelving will take a shelved P4 changelist, and produce the equivalent git commit
+in the branch refs/remotes/p4/unshelved/<changelist>.
+
+The git commit is created relative to the current p4/master branch, so if this
+branch is behind Perforce itself, it may include more changes than you expected.
+
+----
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+----
+
 OPTIONS
 -------
 
@@ -337,6 +352,13 @@ These options can be used to modify 'git p4 rebase' behavior.
 --import-labels::
 	Import p4 labels.
 
+Unshelve options
+~~~~~~~~~~~~~~~~
+
+--origin::
+    Sets the git refspec against which the shelved P4 changelist is compared.
+    Defaults to p4/master.
+
 DEPOT PATH SYNTAX
 -----------------
 The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..59bd4ff64f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
     results = p4CmdList(["changes", "-m", "1"], skip_info=True)
     return int(results[0]['change'])
 
-def p4_describe(change):
+def p4_describe(change, shelved=False):
     """Make sure it returns a valid result by checking for
        the presence of field "time".  Return a dict of the
        results."""
 
-    ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+    cmd = ["describe", "-s"]
+    if shelved:
+        cmd += ["-S"]
+    cmd += [str(change)]
+
+    ds = p4CmdList(cmd, skip_info=True)
     if len(ds) != 1:
         die("p4 describe -s %d did not return 1 result: %s" % (change, str(ds)))
 
@@ -2421,6 +2426,18 @@ class P4Sync(Command, P4UserMap):
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
 
+        self.depotPaths = []
+        self.changeRange = ""
+        self.previousDepotPaths = []
+        self.hasOrigin = False
+
+        # map from branch depot path to parent branch
+        self.knownBranches = {}
+        self.initialParents = {}
+
+        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
+        self.labels = {}
+
     # Force a checkpoint in fast-import and wait for it to finish
     def checkpoint(self):
         self.gitStream.write("checkpoint\n\n")
@@ -2429,7 +2446,7 @@ class P4Sync(Command, P4UserMap):
         if self.verbose:
             print "checkpoint finished: " + out
 
-    def extractFilesFromCommit(self, commit):
+    def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
         self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
                              for path in self.cloneExclude]
         files = []
@@ -2452,6 +2469,9 @@ class P4Sync(Command, P4UserMap):
             file["rev"] = commit["rev%s" % fnum]
             file["action"] = commit["action%s" % fnum]
             file["type"] = commit["type%s" % fnum]
+            if shelved:
+                file["shelved_cl"] = int(shelved_cl)
+
             files.append(file)
             fnum = fnum + 1
         return files
@@ -2743,7 +2763,16 @@ class P4Sync(Command, P4UserMap):
             def streamP4FilesCbSelf(entry):
                 self.streamP4FilesCb(entry)
 
-            fileArgs = ['%s#%s' % (f['path'], f['rev']) for f in filesToRead]
+            fileArgs = []
+            for f in filesToRead:
+                if 'shelved_cl' in f:
+                    # Handle shelved CLs using the "p4 print file@=N" syntax to print
+                    # the contents
+                    fileArg = '%s@=%d' % (f['path'], f['shelved_cl'])
+                else:
+                    fileArg = '%s#%s' % (f['path'], f['rev'])
+
+                fileArgs.append(fileArg)
 
             p4CmdList(["-x", "-", "print"],
                       stdin=fileArgs,
@@ -3162,10 +3191,10 @@ class P4Sync(Command, P4UserMap):
         else:
             return None
 
-    def importChanges(self, changes):
+    def importChanges(self, changes, shelved=False):
         cnt = 1
         for change in changes:
-            description = p4_describe(change)
+            description = p4_describe(change, shelved)
             self.updateOptionDict(description)
 
             if not self.silent:
@@ -3235,7 +3264,7 @@ class P4Sync(Command, P4UserMap):
                                 print "Parent of %s not found. Committing into head of %s" % (branch, parent)
                             self.commit(description, filesForCommit, branch, parent)
                 else:
-                    files = self.extractFilesFromCommit(description)
+                    files = self.extractFilesFromCommit(description, shelved, change)
                     self.commit(description, files, self.branch,
                                 self.initialParent)
                     # only needed once, to connect to the previous commit
@@ -3300,17 +3329,23 @@ class P4Sync(Command, P4UserMap):
             print "IO error with git fast-import. Is your git version recent enough?"
             print self.gitError.read()
 
+    def openStreams(self):
+        self.importProcess = subprocess.Popen(["git", "fast-import"],
+                                              stdin=subprocess.PIPE,
+                                              stdout=subprocess.PIPE,
+                                              stderr=subprocess.PIPE);
+        self.gitOutput = self.importProcess.stdout
+        self.gitStream = self.importProcess.stdin
+        self.gitError = self.importProcess.stderr
 
-    def run(self, args):
-        self.depotPaths = []
-        self.changeRange = ""
-        self.previousDepotPaths = []
-        self.hasOrigin = False
-
-        # map from branch depot path to parent branch
-        self.knownBranches = {}
-        self.initialParents = {}
+    def closeStreams(self):
+        self.gitStream.close()
+        if self.importProcess.wait() != 0:
+            die("fast-import failed: %s" % self.gitError.read())
+        self.gitOutput.close()
+        self.gitError.close()
 
+    def run(self, args):
         if self.importIntoRemotes:
             self.refPrefix = "refs/remotes/p4/"
         else:
@@ -3497,15 +3532,7 @@ class P4Sync(Command, P4UserMap):
                     b = b[len(self.projectName):]
                 self.createdBranches.add(b)
 
-        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
-
-        self.importProcess = subprocess.Popen(["git", "fast-import"],
-                                              stdin=subprocess.PIPE,
-                                              stdout=subprocess.PIPE,
-                                              stderr=subprocess.PIPE);
-        self.gitOutput = self.importProcess.stdout
-        self.gitStream = self.importProcess.stdin
-        self.gitError = self.importProcess.stderr
+        self.openStreams()
 
         if revision:
             self.importHeadRevision(revision)
@@ -3585,11 +3612,7 @@ class P4Sync(Command, P4UserMap):
             missingP4Labels = p4Labels - gitTags
             self.importP4Labels(self.gitStream, missingP4Labels)
 
-        self.gitStream.close()
-        if self.importProcess.wait() != 0:
-            die("fast-import failed: %s" % self.gitError.read())
-        self.gitOutput.close()
-        self.gitError.close()
+        self.closeStreams()
 
         # Cleanup temporary branches created during import
         if self.tempBranches != []:
@@ -3721,6 +3744,48 @@ class P4Clone(P4Sync):
 
         return True
 
+class P4Unshelve(Command):
+    def __init__(self):
+        Command.__init__(self)
+        self.options = []
+        self.description = "Unshelve a P4 changelist into a git commit"
+        self.usage = "usage: %prog [options] changelist"
+        self.options += [
+                optparse.make_option("--no-commit", dest="noCommit",
+                    action='store_true', default=False,
+                    help="do not commit, just update the files"),
+                optparse.make_option("--origin", dest="origin"),
+        ]
+        self.verbose = False
+        self.noCommit = False
+        self.origin = "p4/master"
+        self.destbranch = "refs/remotes/p4/unshelved/%s"
+
+    def run(self, args):
+        if len(args) != 1:
+            return False
+
+        if not gitBranchExists(self.origin):
+            sys.exit("origin branch %s does not exist" % self.origin)
+
+        sync = P4Sync()
+        changes = args
+        sync.initialParent = self.origin
+        sync.branch = self.destbranch % changes[0]
+        sync.verbose = self.verbose
+
+        log = extractLogMessageFromGitCommit(self.origin)
+        settings = extractSettingsGitLog(log)
+        sync.depotPaths = settings['depot-paths']
+        sync.branchPrefixes = sync.depotPaths
+
+        sync.openStreams()
+        sync.loadUserMapFromCache()
+        sync.importChanges(changes, shelved=True)
+        sync.closeStreams()
+
+        return True
+
 class P4Branches(Command):
     def __init__(self):
         Command.__init__(self)
@@ -3775,7 +3840,8 @@ commands = {
     "rebase" : P4Rebase,
     "clone" : P4Clone,
     "rollback" : P4RollBack,
-    "branches" : P4Branches
+    "branches" : P4Branches,
+    "unshelve" : P4Unshelve,
 }
 
 
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
new file mode 100755
index 0000000000..868297507a
--- /dev/null
+++ b/t/t9832-unshelve.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='git p4 unshelve'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'init depot' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "change 1"
+		: > file_to_delete &&
+		p4 add file_to_delete &&
+		p4 submit -d "file to delete"
+	)
+'
+
+test_expect_success 'initial clone' '
+	git p4 clone --dest="$git" //depot/@all
+'
+
+test_expect_success 'create shelved changelist' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$cli" &&
+		p4 edit file1 &&
+		echo "a change" >>file1 &&
+		echo "new file" > file2 &&
+		p4 add file2 &&
+		p4 delete file_to_delete &&
+		p4 opened &&
+		p4 shelve -i <<EOF
+Change: new
+Description:
+	Test commit
+
+	Further description
+Files:
+	//depot/file1
+	//depot/file2
+	//depot/file_to_delete
+EOF
+
+	) &&
+	(
+		cd "$git" &&
+		change=$(p4 changes -s shelved -m1 | cut -d " " -f 2) &&
+		git p4 unshelve $change &&
+		git show refs/remotes/p4/unshelved/$change | grep -q "Further description" &&
+		git cherry-pick refs/remotes/p4/unshelved/$change &&
+		test_path_is_file file2 &&
+		test_cmp file1 "$cli"/file1 &&
+		test_cmp file2 "$cli"/file2 &&
+		test_path_is_missing file_to_delete
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
2.15.1.272.gc310869385

Re: [PATCH 1/1] git-p4: add unshelve command

[Miguel Torroja]

Hi Luke,

I really like the idea of creating a branch based on a shelved CL (We
particularly use shelves all the time), I tested your change and I
have some comments.

 - I have some concerns about having the same "[git-p4...change =
.....]" as if it were a real submitted CL.
    One use case I foresee of the new implementation could be to
cherry-pick that change on another branch (or current branch) prior to
a git p4 submit.

 - I see that the new p4/unshelve... branch is based on the tip of
p4/master by default. what if we set the default to the current HEAD?

 - Shelved CLs can be updated and you might have to run "git p4
unshelve" a second time to get the latest updates. if we call it a
second time it fails as it's trying to update p4/unshelve... rather
than discarding previous one and creating a new one.


Thanks,


On Thu, Feb 22, 2018 at 10:50 AM, Luke Diamand <luke@diamand.org> wrote:
> This can be used to "unshelve" a shelved P4 commit into
> a git commit.
>
> For example:
>
>   $ git p4 unshelve 12345
>
> The resulting commit ends up in the branch:
>    refs/remotes/p4/unshelved/12345
>
> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---
>  Documentation/git-p4.txt |  22 ++++++++
>  git-p4.py                | 128 +++++++++++++++++++++++++++++++++++------------
>  t/t9832-unshelve.sh      |  67 +++++++++++++++++++++++++
>  3 files changed, 186 insertions(+), 31 deletions(-)
>  create mode 100755 t/t9832-unshelve.sh
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9f..910ae63a1c 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -164,6 +164,21 @@ $ git p4 submit --shelve
>  $ git p4 submit --update-shelve 1234 --update-shelve 2345
>  ----
>
> +
> +Unshelve
> +~~~~~~~~
> +Unshelving will take a shelved P4 changelist, and produce the equivalent git commit
> +in the branch refs/remotes/p4/unshelved/<changelist>.
> +
> +The git commit is created relative to the current p4/master branch, so if this
> +branch is behind Perforce itself, it may include more changes than you expected.
> +
> +----
> +$ git p4 sync
> +$ git p4 unshelve 12345
> +$ git show refs/remotes/p4/unshelved/12345
> +----
> +
>  OPTIONS
>  -------
>
> @@ -337,6 +352,13 @@ These options can be used to modify 'git p4 rebase' behavior.
>  --import-labels::
>         Import p4 labels.
>
> +Unshelve options
> +~~~~~~~~~~~~~~~~
> +
> +--origin::
> +    Sets the git refspec against which the shelved P4 changelist is compared.
> +    Defaults to p4/master.
> +
>  DEPOT PATH SYNTAX
>  -----------------
>  The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc69..59bd4ff64f 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -316,12 +316,17 @@ def p4_last_change():
>      results = p4CmdList(["changes", "-m", "1"], skip_info=True)
>      return int(results[0]['change'])
>
> -def p4_describe(change):
> +def p4_describe(change, shelved=False):
>      """Make sure it returns a valid result by checking for
>         the presence of field "time".  Return a dict of the
>         results."""
>
> -    ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
> +    cmd = ["describe", "-s"]
> +    if shelved:
> +        cmd += ["-S"]
> +    cmd += [str(change)]
> +
> +    ds = p4CmdList(cmd, skip_info=True)
>      if len(ds) != 1:
>          die("p4 describe -s %d did not return 1 result: %s" % (change, str(ds)))
>
> @@ -2421,6 +2426,18 @@ class P4Sync(Command, P4UserMap):
>          if gitConfig("git-p4.syncFromOrigin") == "false":
>              self.syncWithOrigin = False
>
> +        self.depotPaths = []
> +        self.changeRange = ""
> +        self.previousDepotPaths = []
> +        self.hasOrigin = False
> +
> +        # map from branch depot path to parent branch
> +        self.knownBranches = {}
> +        self.initialParents = {}
> +
> +        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
> +        self.labels = {}
> +
>      # Force a checkpoint in fast-import and wait for it to finish
>      def checkpoint(self):
>          self.gitStream.write("checkpoint\n\n")
> @@ -2429,7 +2446,7 @@ class P4Sync(Command, P4UserMap):
>          if self.verbose:
>              print "checkpoint finished: " + out
>
> -    def extractFilesFromCommit(self, commit):
> +    def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
>          self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
>                               for path in self.cloneExclude]
>          files = []
> @@ -2452,6 +2469,9 @@ class P4Sync(Command, P4UserMap):
>              file["rev"] = commit["rev%s" % fnum]
>              file["action"] = commit["action%s" % fnum]
>              file["type"] = commit["type%s" % fnum]
> +            if shelved:
> +                file["shelved_cl"] = int(shelved_cl)
> +
>              files.append(file)
>              fnum = fnum + 1
>          return files
> @@ -2743,7 +2763,16 @@ class P4Sync(Command, P4UserMap):
>              def streamP4FilesCbSelf(entry):
>                  self.streamP4FilesCb(entry)
>
> -            fileArgs = ['%s#%s' % (f['path'], f['rev']) for f in filesToRead]
> +            fileArgs = []
> +            for f in filesToRead:
> +                if 'shelved_cl' in f:
> +                    # Handle shelved CLs using the "p4 print file@=N" syntax to print
> +                    # the contents
> +                    fileArg = '%s@=%d' % (f['path'], f['shelved_cl'])
> +                else:
> +                    fileArg = '%s#%s' % (f['path'], f['rev'])
> +
> +                fileArgs.append(fileArg)
>
>              p4CmdList(["-x", "-", "print"],
>                        stdin=fileArgs,
> @@ -3162,10 +3191,10 @@ class P4Sync(Command, P4UserMap):
>          else:
>              return None
>
> -    def importChanges(self, changes):
> +    def importChanges(self, changes, shelved=False):
>          cnt = 1
>          for change in changes:
> -            description = p4_describe(change)
> +            description = p4_describe(change, shelved)
>              self.updateOptionDict(description)
>
>              if not self.silent:
> @@ -3235,7 +3264,7 @@ class P4Sync(Command, P4UserMap):
>                                  print "Parent of %s not found. Committing into head of %s" % (branch, parent)
>                              self.commit(description, filesForCommit, branch, parent)
>                  else:
> -                    files = self.extractFilesFromCommit(description)
> +                    files = self.extractFilesFromCommit(description, shelved, change)
>                      self.commit(description, files, self.branch,
>                                  self.initialParent)
>                      # only needed once, to connect to the previous commit
> @@ -3300,17 +3329,23 @@ class P4Sync(Command, P4UserMap):
>              print "IO error with git fast-import. Is your git version recent enough?"
>              print self.gitError.read()
>
> +    def openStreams(self):
> +        self.importProcess = subprocess.Popen(["git", "fast-import"],
> +                                              stdin=subprocess.PIPE,
> +                                              stdout=subprocess.PIPE,
> +                                              stderr=subprocess.PIPE);
> +        self.gitOutput = self.importProcess.stdout
> +        self.gitStream = self.importProcess.stdin
> +        self.gitError = self.importProcess.stderr
>
> -    def run(self, args):
> -        self.depotPaths = []
> -        self.changeRange = ""
> -        self.previousDepotPaths = []
> -        self.hasOrigin = False
> -
> -        # map from branch depot path to parent branch
> -        self.knownBranches = {}
> -        self.initialParents = {}
> +    def closeStreams(self):
> +        self.gitStream.close()
> +        if self.importProcess.wait() != 0:
> +            die("fast-import failed: %s" % self.gitError.read())
> +        self.gitOutput.close()
> +        self.gitError.close()
>
> +    def run(self, args):
>          if self.importIntoRemotes:
>              self.refPrefix = "refs/remotes/p4/"
>          else:
> @@ -3497,15 +3532,7 @@ class P4Sync(Command, P4UserMap):
>                      b = b[len(self.projectName):]
>                  self.createdBranches.add(b)
>
> -        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
> -
> -        self.importProcess = subprocess.Popen(["git", "fast-import"],
> -                                              stdin=subprocess.PIPE,
> -                                              stdout=subprocess.PIPE,
> -                                              stderr=subprocess.PIPE);
> -        self.gitOutput = self.importProcess.stdout
> -        self.gitStream = self.importProcess.stdin
> -        self.gitError = self.importProcess.stderr
> +        self.openStreams()
>
>          if revision:
>              self.importHeadRevision(revision)
> @@ -3585,11 +3612,7 @@ class P4Sync(Command, P4UserMap):
>              missingP4Labels = p4Labels - gitTags
>              self.importP4Labels(self.gitStream, missingP4Labels)
>
> -        self.gitStream.close()
> -        if self.importProcess.wait() != 0:
> -            die("fast-import failed: %s" % self.gitError.read())
> -        self.gitOutput.close()
> -        self.gitError.close()
> +        self.closeStreams()
>
>          # Cleanup temporary branches created during import
>          if self.tempBranches != []:
> @@ -3721,6 +3744,48 @@ class P4Clone(P4Sync):
>
>          return True
>
> +class P4Unshelve(Command):
> +    def __init__(self):
> +        Command.__init__(self)
> +        self.options = []
> +        self.description = "Unshelve a P4 changelist into a git commit"
> +        self.usage = "usage: %prog [options] changelist"
> +        self.options += [
> +                optparse.make_option("--no-commit", dest="noCommit",
> +                    action='store_true', default=False,
> +                    help="do not commit, just update the files"),
> +                optparse.make_option("--origin", dest="origin"),
> +        ]
> +        self.verbose = False
> +        self.noCommit = False
> +        self.origin = "p4/master"
As I mention previously, this could be an issue when having several p4 branches,
Does it make sense to set the default value of self.origin to HEAD? e.g.

> +        self.destbranch = "refs/remotes/p4/unshelved/%s"
> +
> +    def run(self, args):
> +        if len(args) != 1:
> +            return False
> +
> +        if not gitBranchExists(self.origin):
> +            sys.exit("origin branch %s does not exist" % self.origin)
> +
> +        sync = P4Sync()
> +        changes = args
> +        sync.initialParent = self.origin
> +        sync.branch = self.destbranch % changes[0]
I know this is kind of minor, but  could we use the format method instead?.
I think is more clear what was the intention. e.g

self.destbranch = "refs/remotes/p4/unshelved/{0}"
...
...
sync.branch = self.destbranch.format(changes[0])

> +        sync.verbose = self.verbose
> +
> +        log = extractLogMessageFromGitCommit(self.origin)
> +        settings = extractSettingsGitLog(log)
> +        sync.depotPaths = settings['depot-paths']
> +        sync.branchPrefixes = sync.depotPaths
> +
> +        sync.openStreams()
> +        sync.loadUserMapFromCache()
> +        sync.importChanges(changes, shelved=True)
> +        sync.closeStreams()
> +
> +        return True
> +
>  class P4Branches(Command):
>      def __init__(self):
>          Command.__init__(self)
> @@ -3775,7 +3840,8 @@ commands = {
>      "rebase" : P4Rebase,
>      "clone" : P4Clone,
>      "rollback" : P4RollBack,
> -    "branches" : P4Branches
> +    "branches" : P4Branches,
> +    "unshelve" : P4Unshelve,
>  }
>
>
> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
> new file mode 100755
> index 0000000000..868297507a
> --- /dev/null
> +++ b/t/t9832-unshelve.sh
> @@ -0,0 +1,67 @@
> +#!/bin/sh
> +
> +test_description='git p4 unshelve'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> +       start_p4d
> +'
> +
> +test_expect_success 'init depot' '
> +       (
> +               cd "$cli" &&
> +               echo file1 >file1 &&
> +               p4 add file1 &&
> +               p4 submit -d "change 1"
> +               : > file_to_delete &&
> +               p4 add file_to_delete &&
> +               p4 submit -d "file to delete"
> +       )
> +'
> +
> +test_expect_success 'initial clone' '
> +       git p4 clone --dest="$git" //depot/@all
> +'
> +
> +test_expect_success 'create shelved changelist' '
> +       test_when_finished cleanup_git &&
> +       (
> +               cd "$cli" &&
> +               p4 edit file1 &&
> +               echo "a change" >>file1 &&
> +               echo "new file" > file2 &&
> +               p4 add file2 &&
> +               p4 delete file_to_delete &&
> +               p4 opened &&
> +               p4 shelve -i <<EOF
> +Change: new
> +Description:
> +       Test commit
> +
> +       Further description
> +Files:
> +       //depot/file1
> +       //depot/file2
> +       //depot/file_to_delete
> +EOF
> +
> +       ) &&
> +       (
> +               cd "$git" &&
> +               change=$(p4 changes -s shelved -m1 | cut -d " " -f 2) &&
> +               git p4 unshelve $change &&
> +               git show refs/remotes/p4/unshelved/$change | grep -q "Further description" &&
> +               git cherry-pick refs/remotes/p4/unshelved/$change &&
> +               test_path_is_file file2 &&
> +               test_cmp file1 "$cli"/file1 &&
> +               test_cmp file2 "$cli"/file2 &&
> +               test_path_is_missing file_to_delete
> +       )
> +'
> +
> +test_expect_success 'kill p4d' '
> +       kill_p4d
> +'
> +
> +test_done
> --
> 2.15.1.272.gc310869385
>

Re: [PATCH 1/1] git-p4: add unshelve command

[Luke Diamand]

On 22 February 2018 at 21:39, Miguel Torroja <miguel.torroja@gmail.com> wrote:
> Hi Luke,
>
> I really like the idea of creating a branch based on a shelved CL (We
> particularly use shelves all the time), I tested your change and I
> have some comments.
>
>  - I have some concerns about having the same "[git-p4...change =
> .....]" as if it were a real submitted CL.
>     One use case I foresee of the new implementation could be to
> cherry-pick that change on another branch (or current branch) prior to
> a git p4 submit.

OK, I think we could just not add that in the case of an unshelved commit.

>
>  - I see that the new p4/unshelve... branch is based on the tip of
> p4/master by default. what if we set the default to the current HEAD?

There's a "--origin" option you can use to set it to whatever you want.

I started out with HEAD as the default, but then found that to get a
sensible diff you have to both sync and rebase, which can be quite
annoying.

In my case, in my early testing, I ended up with a git commit which
included both the creation of a file, and a subsequent change, even
though I had only unshelved the subsequent change. That was because
HEAD didn't include the file creation change (but p4/master _did_).

>
>  - Shelved CLs can be updated and you might have to run "git p4
> unshelve" a second time to get the latest updates. if we call it a
> second time it fails as it's trying to update p4/unshelve... rather
> than discarding previous one and creating a new one.

OK, that should also be fixable.

>
>
> Thanks,

Thanks for the feedback, very useful! I'll reroll.
Luke

Re: [PATCH 1/1] git-p4: add unshelve command

[Luke Diamand]

On 22 February 2018 at 22:28, Luke Diamand <luke@diamand.org> wrote:
> On 22 February 2018 at 21:39, Miguel Torroja <miguel.torroja@gmail.com> wrote:
>> Hi Luke,
>>
>> I really like the idea of creating a branch based on a shelved CL (We
>> particularly use shelves all the time), I tested your change and I
>> have some comments.
>>
>>  - I have some concerns about having the same "[git-p4...change =
>> .....]" as if it were a real submitted CL.
>>     One use case I foresee of the new implementation could be to
>> cherry-pick that change on another branch (or current branch) prior to
>> a git p4 submit.
>
> OK, I think we could just not add that in the case of an unshelved commit.
>
>>
>>  - I see that the new p4/unshelve... branch is based on the tip of
>> p4/master by default. what if we set the default to the current HEAD?
>
> There's a "--origin" option you can use to set it to whatever you want.
>
> I started out with HEAD as the default, but then found that to get a
> sensible diff you have to both sync and rebase, which can be quite
> annoying.
>
> In my case, in my early testing, I ended up with a git commit which
> included both the creation of a file, and a subsequent change, even
> though I had only unshelved the subsequent change. That was because
> HEAD didn't include the file creation change (but p4/master _did_).

Discussing this with some of my colleagues, and playing around with
it, it seems that what it really needs to do is to figure out the
parent commit of the shelved changelist, and use that as the basis for
the diff.

Unfortunately, Perforce doesn't have any concept of a "parent commit".
One option that would be possible to implement though is to look at
the shelved changelist, and foreach file, find the original revision
number ("//depot/foo.c#97"). Then "p4 changes //depot/foo.c" would
give you the changelist number for that file. Find the most recent P4
changelist, find the git commit corresponding to that, and do the diff
against that.

It's pretty clunky, and I'm quite glad I didn't try to do that in the
initial revision, as I would surely have given up!

To do it properly of course you need to handle the case where the
shelved changelist author had some files at one changelist, and others
at another. But I think that's just far too complicated to deal with.

Luke

Re: [PATCH 1/1] git-p4: add unshelve command

[Miguel Torroja]

On Fri, Feb 23, 2018 at 6:22 PM, Luke Diamand <luke@diamand.org> wrote:
> On 22 February 2018 at 22:28, Luke Diamand <luke@diamand.org> wrote:
>> On 22 February 2018 at 21:39, Miguel Torroja <miguel.torroja@gmail.com> wrote:
>>> Hi Luke,
>>>
>>> I really like the idea of creating a branch based on a shelved CL (We
>>> particularly use shelves all the time), I tested your change and I
>>> have some comments.
>>>
>>>  - I have some concerns about having the same "[git-p4...change =
>>> .....]" as if it were a real submitted CL.
>>>     One use case I foresee of the new implementation could be to
>>> cherry-pick that change on another branch (or current branch) prior to
>>> a git p4 submit.
>>
>> OK, I think we could just not add that in the case of an unshelved commit.
>>
>>>
>>>  - I see that the new p4/unshelve... branch is based on the tip of
>>> p4/master by default. what if we set the default to the current HEAD?
>>
>> There's a "--origin" option you can use to set it to whatever you want.
>>
>> I started out with HEAD as the default, but then found that to get a
>> sensible diff you have to both sync and rebase, which can be quite
>> annoying.
>>
>> In my case, in my early testing, I ended up with a git commit which
>> included both the creation of a file, and a subsequent change, even
>> though I had only unshelved the subsequent change. That was because
>> HEAD didn't include the file creation change (but p4/master _did_).
>
> Discussing this with some of my colleagues, and playing around with
> it, it seems that what it really needs to do is to figure out the
> parent commit of the shelved changelist, and use that as the basis for
> the diff.
>
> Unfortunately, Perforce doesn't have any concept of a "parent commit".
> One option that would be possible to implement though is to look at
> the shelved changelist, and foreach file, find the original revision
> number ("//depot/foo.c#97"). Then "p4 changes //depot/foo.c" would
> give you the changelist number for that file. Find the most recent P4
> changelist, find the git commit corresponding to that, and do the diff
> against that.
>
> It's pretty clunky, and I'm quite glad I didn't try to do that in the
> initial revision, as I would surely have given up!
>
> To do it properly of course you need to handle the case where the
> shelved changelist author had some files at one changelist, and others
> at another. But I think that's just far too complicated to deal with.
>
> Luke

The behavior of "p4 unshelve" and your "git p4 unshelve" approach I
think are equivalent
as p4 just copies the whole contents of the shelved file locally,
regardless of the previous revision synced.
In other words, you get the same result with "git p4 unshelve" than
with "p4 unshelve" now.

What about creating a new command in a future update to apply just the
change to your local tree?
One approach we took in the past was to create a diff patch and then
apply it to the working tree.
The command "p4 describe -S -du 12345" will output a patch but it has
two problems:
 * the header for each file is not standard, it has to be parsed and
converted, (it starts with "==== //depot/..." and it needs to be
converted to "--- a/...")
 * The new files are not output as a patch
let's say we call the command "git p4 cherry-pick"

Miguel

[PATCH 1/1] git-p4: add unshelve command

[Luke Diamand]

This can be used to "unshelve" a shelved P4 commit into
a git commit.

For example:

  $ git p4 unshelve 12345

The resulting commit ends up in the branch:
   refs/remotes/p4/unshelved/12345

If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.

Caveat:

The unshelving is done against the current "p4/master" branch;
git-p4 uses "p4 print" to get the file contents at the requested
revision, and then fast-import creates a commit relative to p4/master.

Ideally what you would want is for fast-import to create the
commit based on the Perforce "revision" prior to the shelved commit,
but Perforce doesn't have such a concept - to do this, git-p4
would need to figure out the revisions of the individual files
before the shelved changelist, and then construct a temporary
git branch which matched this.

It's possible to do this, but doing so makes this change a lot more
complicated.

This limitation means that if you unshelve a change where some
of the changed files were not based on p4/master, you will get
an amalgam of the change you wanted, and these other changes.

The reference branch can be changed manually with the "--origin"
option.

The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 Documentation/git-p4.txt |  26 ++++++
 git-p4.py                | 171 ++++++++++++++++++++++++++++++---------
 t/t9832-unshelve.sh      |  99 +++++++++++++++++++++++
 3 files changed, 260 insertions(+), 36 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..2d768eec10 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,25 @@ $ git p4 submit --shelve
 $ git p4 submit --update-shelve 1234 --update-shelve 2345
 ----
 
+
+Unshelve
+~~~~~~~~
+Unshelving will take a shelved P4 changelist, and produce the equivalent git commit
+in the branch refs/remotes/p4/unshelved/<changelist>.
+
+The git commit is created relative to the current p4/master, so if this
+is behind Perforce itself, it may include more changes than you expected. You can
+change the reference branch with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one will
+be renamed.
+
+----
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+----
+
 OPTIONS
 -------
 
@@ -337,6 +356,13 @@ These options can be used to modify 'git p4 rebase' behavior.
 --import-labels::
 	Import p4 labels.
 
+Unshelve options
+~~~~~~~~~~~~~~~~
+
+--origin::
+    Sets the git refspec against which the shelved P4 changelist is compared.
+    Defaults to p4/master.
+
 DEPOT PATH SYNTAX
 -----------------
 The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..dcf6dc9f4f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
     results = p4CmdList(["changes", "-m", "1"], skip_info=True)
     return int(results[0]['change'])
 
-def p4_describe(change):
+def p4_describe(change, shelved=False):
     """Make sure it returns a valid result by checking for
        the presence of field "time".  Return a dict of the
        results."""
 
-    ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+    cmd = ["describe", "-s"]
+    if shelved:
+        cmd += ["-S"]
+    cmd += [str(change)]
+
+    ds = p4CmdList(cmd, skip_info=True)
     if len(ds) != 1:
         die("p4 describe -s %d did not return 1 result: %s" % (change, str(ds)))
 
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
                             stderr=subprocess.PIPE, stdout=subprocess.PIPE);
     return proc.wait() == 0;
 
+def gitUpdateRef(ref, newvalue):
+    subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+    subprocess.check_call(["git", "update-ref", "-d", ref])
+
 _gitConfig = {}
 
 def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
         self.tempBranches = []
         self.tempBranchLocation = "refs/git-p4-tmp"
         self.largeFileSystem = None
+        self.suppress_meta_comment = False
 
         if gitConfig('git-p4.largeFileSystem'):
             largeFileSystemConstructor = globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
 
+        self.depotPaths = []
+        self.changeRange = ""
+        self.previousDepotPaths = []
+        self.hasOrigin = False
+
+        # map from branch depot path to parent branch
+        self.knownBranches = {}
+        self.initialParents = {}
+
+        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
+        self.labels = {}
+
     # Force a checkpoint in fast-import and wait for it to finish
     def checkpoint(self):
         self.gitStream.write("checkpoint\n\n")
@@ -2429,7 +2453,7 @@ class P4Sync(Command, P4UserMap):
         if self.verbose:
             print "checkpoint finished: " + out
 
-    def extractFilesFromCommit(self, commit):
+    def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
         self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
                              for path in self.cloneExclude]
         files = []
@@ -2452,6 +2476,9 @@ class P4Sync(Command, P4UserMap):
             file["rev"] = commit["rev%s" % fnum]
             file["action"] = commit["action%s" % fnum]
             file["type"] = commit["type%s" % fnum]
+            if shelved:
+                file["shelved_cl"] = int(shelved_cl)
+
             files.append(file)
             fnum = fnum + 1
         return files
@@ -2743,7 +2770,16 @@ class P4Sync(Command, P4UserMap):
             def streamP4FilesCbSelf(entry):
                 self.streamP4FilesCb(entry)
 
-            fileArgs = ['%s#%s' % (f['path'], f['rev']) for f in filesToRead]
+            fileArgs = []
+            for f in filesToRead:
+                if 'shelved_cl' in f:
+                    # Handle shelved CLs using the "p4 print file@=N" syntax to print
+                    # the contents
+                    fileArg = '%s@=%d' % (f['path'], f['shelved_cl'])
+                else:
+                    fileArg = '%s#%s' % (f['path'], f['rev'])
+
+                fileArgs.append(fileArg)
 
             p4CmdList(["-x", "-", "print"],
                       stdin=fileArgs,
@@ -2844,11 +2880,15 @@ class P4Sync(Command, P4UserMap):
         self.gitStream.write(details["desc"])
         if len(jobs) > 0:
             self.gitStream.write("\nJobs: %s" % (' '.join(jobs)))
-        self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" %
-                             (','.join(self.branchPrefixes), details["change"]))
-        if len(details['options']) > 0:
-            self.gitStream.write(": options = %s" % details['options'])
-        self.gitStream.write("]\nEOT\n\n")
+
+        if not self.suppress_meta_comment:
+            self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" %
+                                (','.join(self.branchPrefixes), details["change"]))
+            if len(details['options']) > 0:
+                self.gitStream.write(": options = %s" % details['options'])
+            self.gitStream.write("]\n")
+
+        self.gitStream.write("EOT\n\n")
 
         if len(parent) > 0:
             if self.verbose:
@@ -3162,10 +3202,10 @@ class P4Sync(Command, P4UserMap):
         else:
             return None
 
-    def importChanges(self, changes):
+    def importChanges(self, changes, shelved=False):
         cnt = 1
         for change in changes:
-            description = p4_describe(change)
+            description = p4_describe(change, shelved)
             self.updateOptionDict(description)
 
             if not self.silent:
@@ -3235,7 +3275,7 @@ class P4Sync(Command, P4UserMap):
                                 print "Parent of %s not found. Committing into head of %s" % (branch, parent)
                             self.commit(description, filesForCommit, branch, parent)
                 else:
-                    files = self.extractFilesFromCommit(description)
+                    files = self.extractFilesFromCommit(description, shelved, change)
                     self.commit(description, files, self.branch,
                                 self.initialParent)
                     # only needed once, to connect to the previous commit
@@ -3300,17 +3340,23 @@ class P4Sync(Command, P4UserMap):
             print "IO error with git fast-import. Is your git version recent enough?"
             print self.gitError.read()
 
+    def openStreams(self):
+        self.importProcess = subprocess.Popen(["git", "fast-import"],
+                                              stdin=subprocess.PIPE,
+                                              stdout=subprocess.PIPE,
+                                              stderr=subprocess.PIPE);
+        self.gitOutput = self.importProcess.stdout
+        self.gitStream = self.importProcess.stdin
+        self.gitError = self.importProcess.stderr
 
-    def run(self, args):
-        self.depotPaths = []
-        self.changeRange = ""
-        self.previousDepotPaths = []
-        self.hasOrigin = False
-
-        # map from branch depot path to parent branch
-        self.knownBranches = {}
-        self.initialParents = {}
+    def closeStreams(self):
+        self.gitStream.close()
+        if self.importProcess.wait() != 0:
+            die("fast-import failed: %s" % self.gitError.read())
+        self.gitOutput.close()
+        self.gitError.close()
 
+    def run(self, args):
         if self.importIntoRemotes:
             self.refPrefix = "refs/remotes/p4/"
         else:
@@ -3497,15 +3543,7 @@ class P4Sync(Command, P4UserMap):
                     b = b[len(self.projectName):]
                 self.createdBranches.add(b)
 
-        self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
-
-        self.importProcess = subprocess.Popen(["git", "fast-import"],
-                                              stdin=subprocess.PIPE,
-                                              stdout=subprocess.PIPE,
-                                              stderr=subprocess.PIPE);
-        self.gitOutput = self.importProcess.stdout
-        self.gitStream = self.importProcess.stdin
-        self.gitError = self.importProcess.stderr
+        self.openStreams()
 
         if revision:
             self.importHeadRevision(revision)
@@ -3585,11 +3623,7 @@ class P4Sync(Command, P4UserMap):
             missingP4Labels = p4Labels - gitTags
             self.importP4Labels(self.gitStream, missingP4Labels)
 
-        self.gitStream.close()
-        if self.importProcess.wait() != 0:
-            die("fast-import failed: %s" % self.gitError.read())
-        self.gitOutput.close()
-        self.gitError.close()
+        self.closeStreams()
 
         # Cleanup temporary branches created during import
         if self.tempBranches != []:
@@ -3721,6 +3755,70 @@ class P4Clone(P4Sync):
 
         return True
 
+class P4Unshelve(Command):
+    def __init__(self):
+        Command.__init__(self)
+        self.options = []
+        self.description = "Unshelve a P4 changelist into a git commit"
+        self.usage = "usage: %prog [options] changelist"
+        self.options += [
+                optparse.make_option("--origin", dest="origin"),
+        ]
+        self.verbose = False
+        self.noCommit = False
+        self.origin = "HEAD"
+        self.destbranch = "refs/remotes/p4/unshelved"
+
+    def run(self, args):
+        if len(args) != 1:
+            return False
+
+        if not gitBranchExists(self.origin):
+            sys.exit("origin branch {} does not exist".format(self.origin))
+
+        sync = P4Sync()
+        changes = args
+        sync.initialParent = self.origin
+
+        # use the first change in the list to construct the branch to unshelve into
+        change = changes[0]
+
+        # if it already exists, rename it
+        branch_name = "{}/{}".format(self.destbranch, change)
+        if gitBranchExists(branch_name):
+            found = True
+            for i in range(0,1000):
+                backup_branch_name = "{}.{}".format(branch_name, i)
+                if not gitBranchExists(backup_branch_name):
+                    gitUpdateRef(backup_branch_name, branch_name) # copy ref to backup
+                    gitDeleteRef(branch_name)
+                    found = True
+                    print("renamed old unshelve branch to {}".format(backup_branch_name))
+                    break
+
+            if not found:
+                sys.exit("gave up trying to rename existing branch {}".format(sync.branch))
+        sync.branch = branch_name
+
+        sync.verbose = self.verbose
+        sync.suppress_meta_comment = True
+
+        print("getting log message from {}".format(self.origin))
+        log = extractLogMessageFromGitCommit("refs/remotes/p4/master")
+        settings = extractSettingsGitLog(log)
+        sync.depotPaths = settings['depot-paths']
+        sync.branchPrefixes = sync.depotPaths
+
+        sync.openStreams()
+        sync.loadUserMapFromCache()
+        sync.silent = True
+        sync.importChanges(changes, shelved=True)
+        sync.closeStreams()
+
+        print("unshelved CL{} into {}".format(change, branch_name))
+
+        return True
+
 class P4Branches(Command):
     def __init__(self):
         Command.__init__(self)
@@ -3775,7 +3873,8 @@ commands = {
     "rebase" : P4Rebase,
     "clone" : P4Clone,
     "rollback" : P4RollBack,
-    "branches" : P4Branches
+    "branches" : P4Branches,
+    "unshelve" : P4Unshelve,
 }
 
 
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
new file mode 100755
index 0000000000..332d30ece9
--- /dev/null
+++ b/t/t9832-unshelve.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+last_shelved_change() {
+	p4 changes -s shelved -m1 | cut -d " " -f 2
+}
+
+test_description='git p4 unshelve'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'init depot' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "change 1"
+		: >file_to_delete &&
+		p4 add file_to_delete &&
+		p4 submit -d "file to delete"
+	)
+'
+
+test_expect_success 'initial clone' '
+	git p4 clone --dest="$git" //depot/@all
+'
+
+test_expect_success 'create shelved changelist' '
+	(
+		cd "$cli" &&
+		p4 edit file1 &&
+		echo "a change" >>file1 &&
+		echo "new file" >file2 &&
+		p4 add file2 &&
+		p4 delete file_to_delete &&
+		p4 opened &&
+		p4 shelve -i <<EOF
+Change: new
+Description:
+	Test commit
+
+	Further description
+Files:
+	//depot/file1
+	//depot/file2
+	//depot/file_to_delete
+EOF
+
+	) &&
+	(
+		cd "$git" &&
+		change=$(last_shelved_change) &&
+		git p4 unshelve $change &&
+		git show refs/remotes/p4/unshelved/$change | grep -q "Further description" &&
+		git cherry-pick refs/remotes/p4/unshelved/$change &&
+		test_path_is_file file2 &&
+		test_cmp file1 "$cli"/file1 &&
+		test_cmp file2 "$cli"/file2 &&
+		test_path_is_missing file_to_delete
+	)
+'
+
+test_expect_success 'update shelved changelist and re-unshelve' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$cli" &&
+		change=$(last_shelved_change) &&
+		echo "file3" >file3 &&
+		p4 add -c $change file3 &&
+		p4 shelve -i -r <<EOF &&
+Change: $change
+Description:
+	Test commit
+
+	Further description
+Files:
+	//depot/file1
+	//depot/file2
+	//depot/file3
+	//depot/file_to_delete
+EOF
+		p4 describe $change
+	) &&
+	(
+		cd "$git" &&
+		change=$(last_shelved_change) &&
+		git p4 unshelve $change &&
+		git diff refs/remotes/p4/unshelved/$change.0 refs/remotes/p4/unshelved/$change | grep -q file3
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
2.17.0.392.gdeb1a6e9b7

Re: [PATCH 1/1] git-p4: add unshelve command

[Luke Diamand]

On 13 May 2018 at 14:52, Merland Romain <merlorom@yahoo.fr> wrote:
> Hello Luke,
>
> Very interseting
> This is indeed an option we are waiting since the introduction of option --shelve for git p4 submit
> What I like most in your approach is the preservation of link to p4/master inducing small changes of git-p4 existing functions already heavily tested.
> Also I like the dedicated branch you create, it is cleaner and then we can cherry-pick in other git branches.

Thanks, I'd be interested to know how you get on with it!

> We made some basic tries on our side, just adding an option --unshelve to P4Submit (for simplicity, but I like much more your P4Unshelve class)
> and trying to use the diff of p4 to generate a patch when possible, then apply it in the current git branch on top of HEAD.
> Here it is, hope it can help a bit.
> Note it also uses p4 print -o for binary files.

I did try something like this earlier this year (if you look in the
archives you'll find it) but I found that it was getting quite
complicated trying to construct a sensible looking patch file from the
output of p4 describe. Better to let git's existing tools do that, as
they're going to be better than any half-baked attempt I might manage
in Python!

Thanks!
Luke



>
> diff --git a/git-p4.py b/git-p4.py
> index f4a6f3b4c..b466b46e1 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1353,6 +1353,8 @@ class P4Submit(Command, P4UserMap):
>                                       metavar="CHANGELIST",
>                                       help="update an existing shelved changelist, implies --shelve, "
>                                             "repeat in-order for multiple shelved changelists"),
> +                optparse.make_option("--unshelve", dest="unshelve",
> +                                     help="unshelve speficied ChangeList into current BRANCH."),
>                  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",
> @@ -1367,6 +1369,7 @@ class P4Submit(Command, P4UserMap):
>          self.dry_run = False
>          self.shelve = False
>          self.update_shelve = list()
> +        self.unshelve = ""
>          self.commit = ""
>          self.disable_rebase = False
>          self.prepare_p4_only = False
> @@ -2083,6 +2086,66 @@ class P4Submit(Command, P4UserMap):
>          if self.clientPath == "":
>              die("Error: Cannot locate perforce checkout of %s in client view" % self.depotPath)
>
> +        # special case of unshelving
> +        # todo: put this code in a class like P4Sync or P4Rebase
> +        if self.unshelve != "":
> +            git_dir = os.getcwd() + '/'
> +            print "Importing shelved CL %s into current git branch %s" % (self.unshelve, self.master)
> +            description = p4_describe(self.unshelve)
> +
> +            # get changed files
> +            files = p4CmdList(['files', "@=%s" % self.unshelve])
> +            editedFiles = []
> +            filesToAdd = []
> +            filesToDelete = []
> +            binaryFiles = []
> +            something_to_commit = False
> +            for f in files:
> +                if not f["depotFile"].startswith(self.depotPath):
> +                    print "WARNING: file %s not in this p4 depot - skipping" % f["depotFile"]
> +                    continue
> +
> +                elif f["action"] == 'delete':
> +                    filesToDelete.append(f)
> +                    something_to_commit = True
> +                elif f["action"] == 'add':
> +                    filesToAdd.append(f)
> +                    something_to_commit = True
> +                elif f["type"] == 'binary':
> +                    binaryFiles.append(f)
> +                    something_to_commit = True
> +                elif f["action"] == 'edit':
> +                    editedFiles.append(f)
> +                    something_to_commit = True
> +
> +                f["clientFile"] = f["depotFile"].replace(self.depotPath,self.clientPath)
> +                f["gitFile"] = f["depotFile"].replace(self.depotPath,git_dir)
> +
> +            if not something_to_commit:
> +                print "Nothing to commit. Exiting"
> +                return True
> +
> +            # get the diff and copy to diff directory
> +            for f in editedFiles:
> +                p4diff = p4_read_pipe(['diff2', '-du', f["depotFile"]+'#'+f["rev"], f["depotFile"]+'@='+self.unshelve])
> +                p4diff = "\n".join(p4diff.split("\n")[1:])
> +                p4diff = '--- '+f["gitFile"]+'\n' + '+++ '+f["gitFile"]+'\n' + p4diff
> +                write_pipe(['patch', '-d/', '-p0'], p4diff)
> +                write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> +            for f in filesToAdd:
> +                p4_write_pipe(['print', '-o', f["gitFile"], f["depotFile"]+'@='+self.unshelve], "")
> +                write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> +            for f in filesToDelete:
> +                os.remove(f["gitFile"])
> +                write_pipe(['git', 'rm', f["gitFile"]], "")
> +            for f in binaryFiles:
> +                p4_write_pipe(['print', '-o', f["gitFile"], f["depotFile"]+'@='+self.unshelve], "")
> +                write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> +
> +            # finalize: commit in git
> +            write_pipe(['git', 'commit', '-m', description["desc"]], "")
> +            return True
> +
>          print "Perforce checkout for depot path %s located at %s" % (self.depotPath, self.clientPath)
>          self.oldWorkingDirectory = os.getcwd()
>
> Romain
>
>
>
> Le samedi 12 mai 2018 à 23:24:48 UTC+2, Luke Diamand <luke@diamand.org> a écrit :
>
>
>
>
>
> This can be used to "unshelve" a shelved P4 commit into
> a git commit.
>
> For example:
>
>   $ git p4 unshelve 12345
>
> The resulting commit ends up in the branch:
>   refs/remotes/p4/unshelved/12345
>
> If that branch already exists, it is renamed - for example
> the above branch would be saved as p4/unshelved/12345.1.
>
> Caveat:
>
> The unshelving is done against the current "p4/master" branch;
> git-p4 uses "p4 print" to get the file contents at the requested
> revision, and then fast-import creates a commit relative to p4/master.
>
> Ideally what you would want is for fast-import to create the
> commit based on the Perforce "revision" prior to the shelved commit,
> but Perforce doesn't have such a concept - to do this, git-p4
> would need to figure out the revisions of the individual files
> before the shelved changelist, and then construct a temporary
> git branch which matched this.
>
> It's possible to do this, but doing so makes this change a lot more
> complicated.
>
> This limitation means that if you unshelve a change where some
> of the changed files were not based on p4/master, you will get
> an amalgam of the change you wanted, and these other changes.
>
> The reference branch can be changed manually with the "--origin"
> option.
>
> The change adds a new Unshelve command class. This just runs the
> existing P4Sync code tweaked to handle a shelved changelist.
>
> Signed-off-by: Luke Diamand <luke@diamand.org>