{"thread":{"id":"57176","subject":"[PATCH v3 RESEND 0/2] git-p4: remove \"debug\" and \"rollback\" verbs","startedAt":"2022-01-04T12:34:58Z","lastAt":"2022-01-04T21:54:15Z","messageCount":4,"participants":["Joel Holdsworth","Andrew Oakley"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"445394","messageId":"20220104123431.1710-1-jholdsworth@nvidia.com","threadId":"57176","inReplyTo":null,"subject":"[PATCH v3 RESEND 0/2] git-p4: remove \"debug\" and \"rollback\" verbs","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2022-01-04T12:34:29Z","receivedAt":"2022-01-04T12:34:58Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"git-p4 contains a selection of verbs for various functions of the\nscript. The \"debug\" and \"rollback\" verbs appear to have been added early\nin the development life of git-p4. They were once used as debugging\ntools, but are no longer being used either by developers or users, and\nare largely undocumented. Removing these verbs simplifies the script by\nremoving dead code, and increases usability by reducing complexity.\n\nThis third version of the patch-set adds more detail to the commit\nmessages.\n\nJoel Holdsworth (2):\n  git-p4: remove \"debug\" verb\n  git-p4: remove \"rollback\" verb\n\n git-p4.py | 76 -------------------------------------------------------\n 1 file changed, 76 deletions(-)\n\n-- \n2.34.1\n\n"},{"id":"445395","messageId":"20220104123431.1710-2-jholdsworth@nvidia.com","threadId":"57176","inReplyTo":"20220104123431.1710-1-jholdsworth@nvidia.com","subject":"[PATCH v3 RESEND 1/2] git-p4: remove \"debug\" verb","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2022-01-04T12:34:30Z","receivedAt":"2022-01-04T12:35:00Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The git-p4 \"debug\" verb is described as \"A tool to debug the output of\np4 -G\".\n\nThe verb is not documented in any detail, but implements a function\nwhich executes an arbitrary p4 command with the -G flag, which causes\nperforce to format all output as marshalled Python dictionary objects.\n\nThe verb was implemented early in the history of git-p4, and may once\nhave served a useful purpose to the authors in the early stages of\ndevelopment. However, the \"debug\" verb is no longer being used by the\ncurrent developers (and users) of git-p4, and whatever purpose the verb\npreviously offered is easily replaced by invoking p4 directly.\n\nThis patch therefore removes the verb from git-p4.\n\nSigned-off-by: Joel Holdsworth <jholdsworth@nvidia.com>\n---\n git-p4.py | 16 ----------------\n 1 file changed, 16 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 2b4500226a..b7ed8e41ff 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1532,21 +1532,6 @@ def loadUserMapFromCache(self):\n         except IOError:\n             self.getUserMapFromPerforceServer()\n \n-class P4Debug(Command):\n-    def __init__(self):\n-        Command.__init__(self)\n-        self.options = []\n-        self.description = \"A tool to debug the output of p4 -G.\"\n-        self.needsGit = False\n-\n-    def run(self, args):\n-        j = 0\n-        for output in p4CmdList(args):\n-            print('Element: %d' % j)\n-            j += 1\n-            print(output)\n-        return True\n-\n class P4RollBack(Command):\n     def __init__(self):\n         Command.__init__(self)\n@@ -4363,7 +4348,6 @@ def printUsage(commands):\n     print(\"\")\n \n commands = {\n-    \"debug\" : P4Debug,\n     \"submit\" : P4Submit,\n     \"commit\" : P4Submit,\n     \"sync\" : P4Sync,\n-- \n2.34.1\n\n"},{"id":"445396","messageId":"20220104123431.1710-3-jholdsworth@nvidia.com","threadId":"57176","inReplyTo":"20220104123431.1710-1-jholdsworth@nvidia.com","subject":"[PATCH v3 RESEND 2/2] git-p4: remove \"rollback\" verb","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2022-01-04T12:34:31Z","receivedAt":"2022-01-04T12:35:03Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The \"rollback\" verb implements a simple algorithm which takes the set of\nremote perforce tracker branches, or optionally, the complete collection\nof local branches in a git repository, and deletes commits from these\nbranches until there are no commits left with a perforce change number\ngreater than than a user-specified change number. If the base of a git\nbranch has a newer change number than the user-specified maximum, then\nthe branch is deleted.\n\nIn future, there might be an argument for the addition of some kind of\n\"reset this branch back to a given perforce change number\" verb for\ngit-p4. However, in its current form it is unlikely to be useful to\nusers for the following reasons:\n\n  * The verb is completely undocumented. The only description provided\n    contains the following text: \"A tool to debug the multi-branch\n    import. Don't use :)\".\n\n  * The verb has a very narrow purpose in that it applies the rollback\n    operation to fixed sets of branches - either all remote p4 branches,\n    or all local branches. There is no way for users to specify branches\n    with more granularity, for example, allowing users to specify a\n    single branch or a set of branches. The utility of the current\n    implementation is therefore a niche within a niche.\n\nGiven these shortcomings, this patch removes the verb from git-p4.\n\nSigned-off-by: Joel Holdsworth <jholdsworth@nvidia.com>\n---\n git-p4.py | 60 -------------------------------------------------------\n 1 file changed, 60 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b7ed8e41ff..a7cb321f75 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1532,65 +1532,6 @@ def loadUserMapFromCache(self):\n         except IOError:\n             self.getUserMapFromPerforceServer()\n \n-class P4RollBack(Command):\n-    def __init__(self):\n-        Command.__init__(self)\n-        self.options = [\n-            optparse.make_option(\"--local\", dest=\"rollbackLocalBranches\", action=\"store_true\")\n-        ]\n-        self.description = \"A tool to debug the multi-branch import. Don't use :)\"\n-        self.rollbackLocalBranches = False\n-\n-    def run(self, args):\n-        if len(args) != 1:\n-            return False\n-        maxChange = int(args[0])\n-\n-        if \"p4ExitCode\" in p4Cmd(\"changes -m 1\"):\n-            die(\"Problems executing p4\");\n-\n-        if self.rollbackLocalBranches:\n-            refPrefix = \"refs/heads/\"\n-            lines = read_pipe_lines(\"git rev-parse --symbolic --branches\")\n-        else:\n-            refPrefix = \"refs/remotes/\"\n-            lines = read_pipe_lines(\"git rev-parse --symbolic --remotes\")\n-\n-        for line in lines:\n-            if self.rollbackLocalBranches or (line.startswith(\"p4/\") and line != \"p4/HEAD\\n\"):\n-                line = line.strip()\n-                ref = refPrefix + line\n-                log = extractLogMessageFromGitCommit(ref)\n-                settings = extractSettingsGitLog(log)\n-\n-                depotPaths = settings['depot-paths']\n-                change = settings['change']\n-\n-                changed = False\n-\n-                if len(p4Cmd(\"changes -m 1 \"  + ' '.join (['%s...@%s' % (p, maxChange)\n-                                                           for p in depotPaths]))) == 0:\n-                    print(\"Branch %s did not exist at change %s, deleting.\" % (ref, maxChange))\n-                    system(\"git update-ref -d %s `git rev-parse %s`\" % (ref, ref))\n-                    continue\n-\n-                while change and int(change) > maxChange:\n-                    changed = True\n-                    if self.verbose:\n-                        print(\"%s is at %s ; rewinding towards %s\" % (ref, change, maxChange))\n-                    system(\"git update-ref %s \\\"%s^\\\"\" % (ref, ref))\n-                    log = extractLogMessageFromGitCommit(ref)\n-                    settings =  extractSettingsGitLog(log)\n-\n-\n-                    depotPaths = settings['depot-paths']\n-                    change = settings['change']\n-\n-                if changed:\n-                    print(\"%s rewound to %s\" % (ref, change))\n-\n-        return True\n-\n class P4Submit(Command, P4UserMap):\n \n     conflict_behavior_choices = (\"ask\", \"skip\", \"quit\")\n@@ -4353,7 +4294,6 @@ def printUsage(commands):\n     \"sync\" : P4Sync,\n     \"rebase\" : P4Rebase,\n     \"clone\" : P4Clone,\n-    \"rollback\" : P4RollBack,\n     \"branches\" : P4Branches,\n     \"unshelve\" : P4Unshelve,\n }\n-- \n2.34.1\n\n"},{"id":"445480","messageId":"20220104215406.27298d62@ado-tr.dyn.home.arpa","threadId":"57176","inReplyTo":"20220104123431.1710-1-jholdsworth@nvidia.com","subject":"Re: [PATCH v3 RESEND 0/2] git-p4: remove \"debug\" and \"rollback\" verbs","fromName":"Andrew Oakley","fromEmail":"andrew@adoakley.name","sentAt":"2022-01-04T21:54:06Z","receivedAt":"2022-01-04T21:54:15Z","isPatch":true,"sender":{"key":"andrew@adoakley.name","avatar":"https://avatars.githubusercontent.com/u/1107440?v=4"},"body":"On Tue,  4 Jan 2022 12:34:29 +0000\nJoel Holdsworth <jholdsworth@nvidia.com> wrote:\n\n> git-p4 contains a selection of verbs for various functions of the\n> script. The \"debug\" and \"rollback\" verbs appear to have been added\n> early in the development life of git-p4. They were once used as\n> debugging tools, but are no longer being used either by developers or\n> users, and are largely undocumented. Removing these verbs simplifies\n> the script by removing dead code, and increases usability by reducing\n> complexity.\n\nI agree, these commands look rather usesless and I've never used them.\n\nThe patches to remove them look correct to me.\n\nThanks\n"}]}