threads / patch / 57176

v3, 2 partsgit-p4: remove "debug" and "rollback" verbs

Subject: [PATCH v3 RESEND 0/2] git-p4: remove "debug" and "rollback" verbs

## tl;dr

4 messages between Jan 4, 2022 and Jan 4, 2022. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Joel Holdsworth· Jan 4, 2022, 12:34 UTC · lore

git-p4 contains a selection of verbs for various functions of the script. The "debug" and "rollback" verbs appear to have been added early in the development life of git-p4. They were once used as debugging tools, but are no longer being used either by developers or users, and are largely undocumented. Removing these verbs simplifies the script by removing dead code, and increases usability by reducing complexity.

This third version of the patch-set adds more detail to the commit messages.

Joel Holdsworth (2):
  git-p4: remove "debug" verb
  git-p4: remove "rollback" verb
 git-p4.py | 76 -------------------------------------------------------
 1 file changed, 76 deletions(-)
-- 
2.34.1
Joel Holdsworth· Jan 4, 2022, 12:34 UTC · re: Joel Holdsworth · lore

[PATCH v3 RESEND 1/2] git-p4: remove "debug" verb

The git-p4 "debug" verb is described as "A tool to debug the output of p4 -G".

The verb is not documented in any detail, but implements a function which executes an arbitrary p4 command with the -G flag, which causes perforce to format all output as marshalled Python dictionary objects.

The verb was implemented early in the history of git-p4, and may once have served a useful purpose to the authors in the early stages of development. However, the "debug" verb is no longer being used by the current developers (and users) of git-p4, and whatever purpose the verb previously offered is easily replaced by invoking p4 directly.

This patch therefore removes the verb from git-p4.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
 git-p4.py | 16 ----------------
 1 file changed, 16 deletions(-)
Show changes to git-p4.py +0 −16
diff --git a/git-p4.py b/git-p4.py
index 2b4500226a..b7ed8e41ff 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1532,21 +1532,6 @@ def loadUserMapFromCache(self):
         except IOError:
             self.getUserMapFromPerforceServer()
 
-class P4Debug(Command):
-    def __init__(self):
-        Command.__init__(self)
-        self.options = []
-        self.description = "A tool to debug the output of p4 -G."
-        self.needsGit = False
-
-    def run(self, args):
-        j = 0
-        for output in p4CmdList(args):
-            print('Element: %d' % j)
-            j += 1
-            print(output)
-        return True
-
 class P4RollBack(Command):
     def __init__(self):
         Command.__init__(self)
@@ -4363,7 +4348,6 @@ def printUsage(commands):
     print("")
 
 commands = {
-    "debug" : P4Debug,
     "submit" : P4Submit,
     "commit" : P4Submit,
     "sync" : P4Sync,
-- 
2.34.1
Joel Holdsworth· Jan 4, 2022, 12:34 UTC · re: Joel Holdsworth · lore

[PATCH v3 RESEND 2/2] git-p4: remove "rollback" verb

The "rollback" verb implements a simple algorithm which takes the set of remote perforce tracker branches, or optionally, the complete collection of local branches in a git repository, and deletes commits from these branches until there are no commits left with a perforce change number greater than than a user-specified change number. If the base of a git branch has a newer change number than the user-specified maximum, then the branch is deleted.

In future, there might be an argument for the addition of some kind of "reset this branch back to a given perforce change number" verb for git-p4. However, in its current form it is unlikely to be useful to users for the following reasons:

  * The verb is completely undocumented. The only description provided
    contains the following text: "A tool to debug the multi-branch
    import. Don't use :)".
  * The verb has a very narrow purpose in that it applies the rollback
    operation to fixed sets of branches - either all remote p4 branches,
    or all local branches. There is no way for users to specify branches
    with more granularity, for example, allowing users to specify a
    single branch or a set of branches. The utility of the current
    implementation is therefore a niche within a niche.
Given these shortcomings, this patch removes the verb from git-p4.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
 git-p4.py | 60 -------------------------------------------------------
 1 file changed, 60 deletions(-)
Show changes to git-p4.py +0 −60
diff --git a/git-p4.py b/git-p4.py
index b7ed8e41ff..a7cb321f75 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1532,65 +1532,6 @@ def loadUserMapFromCache(self):
         except IOError:
             self.getUserMapFromPerforceServer()
 
-class P4RollBack(Command):
-    def __init__(self):
-        Command.__init__(self)
-        self.options = [
-            optparse.make_option("--local", dest="rollbackLocalBranches", action="store_true")
-        ]
-        self.description = "A tool to debug the multi-branch import. Don't use :)"
-        self.rollbackLocalBranches = False
-
-    def run(self, args):
-        if len(args) != 1:
-            return False
-        maxChange = int(args[0])
-
-        if "p4ExitCode" in p4Cmd("changes -m 1"):
-            die("Problems executing p4");
-
-        if self.rollbackLocalBranches:
-            refPrefix = "refs/heads/"
-            lines = read_pipe_lines("git rev-parse --symbolic --branches")
-        else:
-            refPrefix = "refs/remotes/"
-            lines = read_pipe_lines("git rev-parse --symbolic --remotes")
-
-        for line in lines:
-            if self.rollbackLocalBranches or (line.startswith("p4/") and line != "p4/HEAD\n"):
-                line = line.strip()
-                ref = refPrefix + line
-                log = extractLogMessageFromGitCommit(ref)
-                settings = extractSettingsGitLog(log)
-
-                depotPaths = settings['depot-paths']
-                change = settings['change']
-
-                changed = False
-
-                if len(p4Cmd("changes -m 1 "  + ' '.join (['%s...@%s' % (p, maxChange)
-                                                           for p in depotPaths]))) == 0:
-                    print("Branch %s did not exist at change %s, deleting." % (ref, maxChange))
-                    system("git update-ref -d %s `git rev-parse %s`" % (ref, ref))
-                    continue
-
-                while change and int(change) > maxChange:
-                    changed = True
-                    if self.verbose:
-                        print("%s is at %s ; rewinding towards %s" % (ref, change, maxChange))
-                    system("git update-ref %s \"%s^\"" % (ref, ref))
-                    log = extractLogMessageFromGitCommit(ref)
-                    settings =  extractSettingsGitLog(log)
-
-
-                    depotPaths = settings['depot-paths']
-                    change = settings['change']
-
-                if changed:
-                    print("%s rewound to %s" % (ref, change))
-
-        return True
-
 class P4Submit(Command, P4UserMap):
 
     conflict_behavior_choices = ("ask", "skip", "quit")
@@ -4353,7 +4294,6 @@ def printUsage(commands):
     "sync" : P4Sync,
     "rebase" : P4Rebase,
     "clone" : P4Clone,
-    "rollback" : P4RollBack,
     "branches" : P4Branches,
     "unshelve" : P4Unshelve,
 }
-- 
2.34.1
Andrew Oakley· Jan 4, 2022, 21:54 UTC · re: Joel Holdsworth · lore

Re: [PATCH v3 RESEND 0/2] git-p4: remove "debug" and "rollback" verbs

On Tue, 4 Jan 2022 12:34:29 +0000 Joel Holdsworth <jholdsworth@nvidia.com> wrote:

Show 7 quoted lines
> git-p4 contains a selection of verbs for various functions of the
> script. The "debug" and "rollback" verbs appear to have been added
> early in the development life of git-p4. They were once used as
> debugging tools, but are no longer being used either by developers or
> users, and are largely undocumented. Removing these verbs simplifies
> the script by removing dead code, and increases usability by reducing
> complexity.
I agree, these commands look rather usesless and I've never used them.
The patches to remove them look correct to me.
Thanks

← back to recent threads