From adf159b44183537538e78c2e69997f9f605b92d4 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:43 +0100 Subject: [PATCH 01/22] git-p4: add blank lines between functions and class definitions In the PEP8 style guidelines, top-level functions and class definitions should be separated by two blank lines. Methods should be surrounded by a single blank line. This guideline is described here in the "Blank Lines" section: https://www.python.org/dev/peps/pep-0008/#blank-lines Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/git-p4.py b/git-p4.py index a9b1f90441..265182b088 100755 --- a/git-p4.py +++ b/git-p4.py @@ -59,6 +59,7 @@ p4_access_checked = False re_ko_keywords = re.compile(br'\$(Id|Header)(:[^$\n]+)?\$') re_k_keywords = re.compile(br'\$(Id|Header|Author|Date|DateTime|Change|File|Revision)(:[^$\n]+)?\$') + def format_size_human_readable(num): """ Returns a number of units (typically bytes) formatted as a human-readable string. @@ -71,6 +72,7 @@ def format_size_human_readable(num): return "{:3.1f} {}B".format(num, unit) return "{:.1f} YiB".format(num) + def p4_build_cmd(cmd): """Build a suitable p4 command line. @@ -118,6 +120,7 @@ def p4_build_cmd(cmd): return real_cmd + def git_dir(path): """ Return TRUE if the given path is a git directory (/path/to/dir/.git). This won't automatically add ".git" to a directory. @@ -128,6 +131,7 @@ def git_dir(path): else: return d + def chdir(path, is_client_path=False): """Do chdir to the given path, and set the PWD environment variable for use by P4. It does not look at getcwd() output. @@ -150,6 +154,7 @@ def chdir(path, is_client_path=False): path = os.getcwd() os.environ['PWD'] = path + def calcDiskFree(): """Return free space in bytes on the disk of the given dirname.""" if platform.system() == 'Windows': @@ -160,6 +165,7 @@ def calcDiskFree(): st = os.statvfs(os.getcwd()) return st.f_bavail * st.f_frsize + def die(msg): """ Terminate execution. Make sure that any running child processes have been wait()ed for before calling this. @@ -170,6 +176,7 @@ def die(msg): sys.stderr.write(msg + "\n") sys.exit(1) + def prompt(prompt_text): """ Prompt the user to choose one of the choices @@ -188,21 +195,25 @@ def prompt(prompt_text): if response in choices: return response + # We need different encoding/decoding strategies for text data being passed # around in pipes depending on python version if bytes is not str: # For python3, always encode and decode as appropriate def decode_text_stream(s): return s.decode() if isinstance(s, bytes) else s + def encode_text_stream(s): return s.encode() if isinstance(s, str) else s else: # For python2.7, pass read strings as-is, but also allow writing unicode def decode_text_stream(s): return s + def encode_text_stream(s): return s.encode('utf_8') if isinstance(s, unicode) else s + def decode_path(path): """Decode a given string (bytes or otherwise) using configured path encoding options """ @@ -218,6 +229,7 @@ def decode_path(path): print('Path with non-ASCII characters detected. Used {} to decode: {}'.format(encoding, path)) return path + def run_git_hook(cmd, param=[]): """Execute a hook if the hook exists.""" args = ['git', 'hook', 'run', '--ignore-missing', cmd] @@ -227,6 +239,7 @@ def run_git_hook(cmd, param=[]): args.append(p) return subprocess.call(args) == 0 + def write_pipe(c, stdin, *k, **kw): if verbose: sys.stderr.write('Writing pipe: {}\n'.format(' '.join(c))) @@ -240,12 +253,14 @@ def write_pipe(c, stdin, *k, **kw): return val + def p4_write_pipe(c, stdin, *k, **kw): real_cmd = p4_build_cmd(c) if bytes is not str and isinstance(stdin, str): stdin = encode_text_stream(stdin) return write_pipe(real_cmd, stdin, *k, **kw) + def read_pipe_full(c, *k, **kw): """ Read output from command. Returns a tuple of the return status, stdout text and stderr @@ -259,6 +274,7 @@ def read_pipe_full(c, *k, **kw): (out, err) = p.communicate() return (p.returncode, out, decode_text_stream(err)) + def read_pipe(c, ignore_error=False, raw=False, *k, **kw): """ Read output from command. Returns the output text on success. On failure, terminates execution, unless @@ -276,6 +292,7 @@ def read_pipe(c, ignore_error=False, raw=False, *k, **kw): out = decode_text_stream(out) return out + def read_pipe_text(c, *k, **kw): """ Read output from a command with trailing whitespace stripped. On error, returns None. @@ -286,10 +303,12 @@ def read_pipe_text(c, *k, **kw): else: return decode_text_stream(out).rstrip() + def p4_read_pipe(c, ignore_error=False, raw=False, *k, **kw): real_cmd = p4_build_cmd(c) return read_pipe(real_cmd, ignore_error, raw=raw, *k, **kw) + def read_pipe_lines(c, raw=False, *k, **kw): if verbose: sys.stderr.write('Reading pipe: {}\n'.format(' '.join(c))) @@ -303,11 +322,13 @@ def read_pipe_lines(c, raw=False, *k, **kw): die('Command failed: {}'.format(' '.join(c))) return lines + def p4_read_pipe_lines(c, *k, **kw): """Specifically invoke p4 on the command supplied. """ real_cmd = p4_build_cmd(c) return read_pipe_lines(real_cmd, *k, **kw) + def p4_has_command(cmd): """Ask p4 for help on this command. If it returns an error, the command does not exist in this version of p4.""" @@ -317,6 +338,7 @@ def p4_has_command(cmd): p.communicate() return p.returncode == 0 + def p4_has_move_command(): """See if the move command exists, that it supports -k, and that it has not been administratively disabled. The arguments @@ -337,6 +359,7 @@ def p4_has_move_command(): # assume it failed because @... was invalid changelist return True + def system(cmd, ignore_error=False, *k, **kw): if verbose: sys.stderr.write("executing {}\n".format( @@ -347,6 +370,7 @@ def system(cmd, ignore_error=False, *k, **kw): return retcode + def p4_system(cmd, *k, **kw): """Specifically invoke p4 as the system command. """ real_cmd = p4_build_cmd(cmd) @@ -354,9 +378,11 @@ def p4_system(cmd, *k, **kw): if retcode: raise subprocess.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 """ @@ -402,7 +428,10 @@ def p4_check_access(min_expiration=1): 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 hopefully is the interesting version bit. @@ -418,12 +447,15 @@ def p4_version_string(): _p4_version_string = a[-1].rstrip() return _p4_version_string + def p4_integrate(src, dest): p4_system(["integrate", "-Dt", wildcard_encode(src), wildcard_encode(dest)]) + def p4_sync(f, *options): p4_system(["sync"] + list(options) + [wildcard_encode(f)]) + def p4_add(f): # forcibly add file names with wildcards if wildcard_present(f): @@ -431,29 +463,37 @@ def p4_add(f): else: p4_system(["add", f]) + def p4_delete(f): p4_system(["delete", wildcard_encode(f)]) + def p4_edit(f, *options): p4_system(["edit"] + list(options) + [wildcard_encode(f)]) + def p4_revert(f): p4_system(["revert", wildcard_encode(f)]) + def p4_reopen(type, f): p4_system(["reopen", "-t", type, wildcard_encode(f)]) + def p4_reopen_in_change(changelist, files): cmd = ["reopen", "-c", str(changelist)] + files p4_system(cmd) + def p4_move(src, dest): p4_system(["move", "-k", wildcard_encode(src), wildcard_encode(dest)]) + def p4_last_change(): results = p4CmdList(["changes", "-m", "1"], skip_info=True) return int(results[0]['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 @@ -482,6 +522,7 @@ def p4_describe(change, shelved=False): return d + # # Canonicalize the p4 type and return a tuple of the # base type, plus any modifiers. See "p4 help filetypes" @@ -517,6 +558,7 @@ def split_p4_type(p4type): mods = s[1] return (base, mods) + # # return the raw p4 type of a file (text, text+ko, etc) # @@ -524,6 +566,7 @@ def p4_type(f): results = p4CmdList(["fstat", "-T", "headType", wildcard_encode(f)]) return results[0]['headType'] + # # Given a type base and modifier, return a regexp matching # the keywords that can be expanded in the file @@ -539,6 +582,7 @@ def p4_keywords_regexp_for_type(base, type_mods): else: return None + # # Given a file, return a regexp matching the possible # RCS keywords that will be expanded, or None for files @@ -551,6 +595,7 @@ def p4_keywords_regexp_for_file(file): (type_base, type_mods) = split_p4_type(p4_type(file)) return p4_keywords_regexp_for_type(type_base, type_mods) + def setP4ExecBit(file, mode): # Reopens an already open file and changes the execute bit to match # the execute bit setting in the passed in mode. @@ -566,6 +611,7 @@ def setP4ExecBit(file, mode): p4_reopen(p4Type, file) + def getP4OpenedType(file): # Returns the perforce file type for the given file. @@ -576,6 +622,7 @@ def getP4OpenedType(file): else: die("Could not determine file type for %s (result: '%s')" % (file, result)) + # Return the set of all p4 labels def getP4Labels(depotPaths): labels = set() @@ -588,6 +635,7 @@ def getP4Labels(depotPaths): return labels + # Return the set of all git tags def getGitTags(): gitTags = set() @@ -596,8 +644,10 @@ def getGitTags(): gitTags.add(tag) return gitTags + _diff_tree_pattern = None + def parseDiffTreeEntry(entry): """Parses a single diff tree entry into its component elements. @@ -635,41 +685,52 @@ def parseDiffTreeEntry(entry): } return None + def isModeExec(mode): # Returns True if the given git mode represents an executable file, # 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 + class P4CommandException(P4Exception): """ Something went wrong calling p4 which means we have to give up """ + def __init__(self, msg): self.msg = msg def __str__(self): return self.msg + 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, errors_as_exceptions=False, *k, **kw): @@ -746,6 +807,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False, return result + def p4Cmd(cmd, *k, **kw): list = p4CmdList(cmd, *k, **kw) result = {} @@ -753,6 +815,7 @@ def p4Cmd(cmd, *k, **kw): result.update(entry) return result; + def p4Where(depotPath): if not depotPath.endswith("/"): depotPath += "/" @@ -789,20 +852,25 @@ def p4Where(depotPath): clientPath = clientPath[:-3] return clientPath + def currentGitBranch(): return read_pipe_text(["git", "symbolic-ref", "--short", "-q", "HEAD"]) + def isValidGitDir(path): return git_dir(path) != None + def parseRevision(ref): return read_pipe(["git", "rev-parse", ref]).strip() + def branchExists(ref): rev = read_pipe(["git", "rev-parse", "-q", "--verify", ref], ignore_error=True) return len(rev) > 0 + def extractLogMessageFromGitCommit(commit): logMessage = "" @@ -817,6 +885,7 @@ def extractLogMessageFromGitCommit(commit): logMessage += log return logMessage + def extractSettingsGitLog(log): values = {} for line in log.split("\n"): @@ -842,19 +911,24 @@ def extractSettingsGitLog(log): values['depot-paths'] = paths.split(',') return values + def gitBranchExists(branch): proc = subprocess.Popen(["git", "rev-parse", 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): if key not in _gitConfig: cmd = [ "git", "config" ] @@ -865,6 +939,7 @@ def gitConfig(key, typeSpecifier=None): _gitConfig[key] = s.strip() return _gitConfig[key] + def gitConfigBool(key): """Return a bool, using git config --bool. It is True only if the variable is set to true, and False if set to false or not present @@ -874,6 +949,7 @@ def gitConfigBool(key): _gitConfig[key] = gitConfig(key, '--bool') == "true" return _gitConfig[key] + def gitConfigInt(key): if key not in _gitConfig: cmd = [ "git", "config", "--int", key ] @@ -885,6 +961,7 @@ def gitConfigInt(key): _gitConfig[key] = None return _gitConfig[key] + def gitConfigList(key): if key not in _gitConfig: s = read_pipe(["git", "config", "--get-all", key], ignore_error=True) @@ -893,6 +970,7 @@ def gitConfigList(key): _gitConfig[key] = [] return _gitConfig[key] + def p4BranchesInGit(branchesAreInRemotes=True): """Find all the branches whose names start with "p4/", looking in remotes or heads as specified by the argument. Return @@ -925,6 +1003,7 @@ def p4BranchesInGit(branchesAreInRemotes=True): return branches + def branch_exists(branch): """Make sure that the given ref name really exists.""" @@ -937,6 +1016,7 @@ def branch_exists(branch): # expect exactly one line of output: the branch name return out.rstrip() == branch + def findUpstreamBranchPoint(head = "HEAD"): branches = p4BranchesInGit() # map from depot-path to branch name @@ -964,6 +1044,7 @@ def findUpstreamBranchPoint(head = "HEAD"): return ["", settings] + def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent=True): if not silent: print("Creating/updating branch(es) in %s based on origin branch(es)" @@ -1011,6 +1092,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent if update: system(["git", "update-ref", remoteHead, originHead]) + def originP4BranchesExist(): return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master") @@ -1024,12 +1106,14 @@ def p4ParseNumericChangeRange(parts): return (changeStart, changeEnd) + def chooseBlockSize(blockSize): if blockSize: return blockSize else: return defaultBlockSize + def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): assert depotPaths @@ -1107,6 +1191,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): changes = sorted(changes) return changes + def p4PathStartsWith(path, prefix): # This method tries to remedy a potential mixed-case issue: # @@ -1119,6 +1204,7 @@ def p4PathStartsWith(path, prefix): return path.lower().startswith(prefix.lower()) return path.startswith(prefix) + def getClientSpec(): """Look at the p4 client spec, create a View() object that contains all the mappings, and return it.""" @@ -1149,6 +1235,7 @@ def getClientSpec(): return view + def getClientRoot(): """Grab the client directory.""" @@ -1162,6 +1249,7 @@ def getClientRoot(): return entry["Root"] + # # P4 wildcards are not allowed in filenames. P4 complains # if you simply add them, but you can force it with "-f", in @@ -1179,6 +1267,7 @@ def wildcard_decode(path): .replace("%25", "%") return path + def wildcard_encode(path): # do % first to avoid double-encoding the %s introduced here path = path.replace("%", "%25") \ @@ -1187,10 +1276,12 @@ def wildcard_encode(path): .replace("@", "%40") return path + def wildcard_present(path): m = re.search("[*#@%]", path) return m is not None + class LargeFileSystem(object): """Base class for large file system support.""" @@ -1272,6 +1363,7 @@ class LargeFileSystem(object): sys.stderr.write("%s moved to large file system (%s)\n" % (relPath, localLargeFile)) return (git_mode, contents) + class MockLFS(LargeFileSystem): """Mock large file system for testing.""" @@ -1295,6 +1387,7 @@ class MockLFS(LargeFileSystem): os.makedirs(remotePath) shutil.copyfile(localLargeFile, os.path.join(remotePath, os.path.basename(localLargeFile))) + class GitLFS(LargeFileSystem): """Git LFS as backend for the git-p4 large file system. See https://git-lfs.github.com/ for details.""" @@ -1383,6 +1476,7 @@ class GitLFS(LargeFileSystem): else: return LargeFileSystem.processContent(self, git_mode, relPath, contents) + class Command: delete_actions = ( "delete", "move/delete", "purge" ) add_actions = ( "add", "branch", "move/add" ) @@ -1398,6 +1492,7 @@ class Command: setattr(self, attr, value) return getattr(self, attr) + class P4UserMap: def __init__(self): self.userMapFromPerforceServer = False @@ -1468,6 +1563,7 @@ class P4UserMap: except IOError: self.getUserMapFromPerforceServer() + class P4Submit(Command, P4UserMap): conflict_behavior_choices = ("ask", "skip", "quit") @@ -2473,6 +2569,7 @@ class P4Submit(Command, P4UserMap): return True + class View(object): """Represent a p4 view ("p4 help views"), and map files in a repo according to the view.""" @@ -2580,11 +2677,13 @@ class View(object): die( "Error: %s is not found in client spec path" % depot_path ) return "" + def cloneExcludeCallback(option, opt_str, value, parser): # prepend "/" because the first "/" was consumed as part of the option itself. # ("-//depot/A/..." becomes "/depot/A/..." after option parsing) parser.values.cloneExclude += ["/" + re.sub(r"\.\.\.$", "", value)] + class P4Sync(Command, P4UserMap): def __init__(self): @@ -3942,6 +4041,7 @@ class P4Sync(Command, P4UserMap): return True + class P4Rebase(Command): def __init__(self): Command.__init__(self) @@ -3979,6 +4079,7 @@ class P4Rebase(Command): "HEAD", "--"]) return True + class P4Clone(P4Sync): def __init__(self): P4Sync.__init__(self) @@ -4057,6 +4158,7 @@ class P4Clone(P4Sync): return True + class P4Unshelve(Command): def __init__(self): Command.__init__(self) @@ -4172,6 +4274,7 @@ class P4Unshelve(Command): return True + class P4Branches(Command): def __init__(self): Command.__init__(self) @@ -4197,6 +4300,7 @@ class P4Branches(Command): print("%s <= %s (%s)" % (branch, ",".join(settings["depot-paths"]), settings["change"])) return True + class HelpFormatter(optparse.IndentedHelpFormatter): def __init__(self): optparse.IndentedHelpFormatter.__init__(self) @@ -4207,6 +4311,7 @@ class HelpFormatter(optparse.IndentedHelpFormatter): else: return "" + def printUsage(commands): print("usage: %s [options]" % sys.argv[0]) print("") @@ -4215,6 +4320,7 @@ def printUsage(commands): print("Try %s --help for command specific help." % sys.argv[0]) print("") + commands = { "submit" : P4Submit, "commit" : P4Submit, @@ -4225,6 +4331,7 @@ commands = { "unshelve" : P4Unshelve, } + def main(): if len(sys.argv[1:]) == 0: printUsage(commands.keys()) From 990547aa2b701ff8cbdf85219175bd015371f55c Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:44 +0100 Subject: [PATCH 02/22] git-p4: remove unneeded semicolons from statements Python allows the usage of compound statements where multiple statements are written on a single line separared by semicolons. It is also possible to add a semicolon after a single statement, however this is generally considered to be untidy, and is unnecessary. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/git-p4.py b/git-p4.py index 265182b088..aabf657a57 100755 --- a/git-p4.py +++ b/git-p4.py @@ -813,7 +813,7 @@ def p4Cmd(cmd, *k, **kw): result = {} for entry in list: result.update(entry) - return result; + return result def p4Where(depotPath): @@ -914,8 +914,8 @@ def extractSettingsGitLog(log): def gitBranchExists(branch): proc = subprocess.Popen(["git", "rev-parse", branch], - stderr=subprocess.PIPE, stdout=subprocess.PIPE); - return proc.wait() == 0; + stderr=subprocess.PIPE, stdout=subprocess.PIPE) + return proc.wait() == 0 def gitUpdateRef(ref, newvalue): @@ -3530,8 +3530,8 @@ class P4Sync(Command, P4UserMap): def importNewBranch(self, branch, maxChange): # make fast-import flush all changes to disk and update the refs using the checkpoint # command so that we can try to find the branch parent in the git history - self.gitStream.write("checkpoint\n\n"); - self.gitStream.flush(); + self.gitStream.write("checkpoint\n\n") + self.gitStream.flush() branchPrefix = self.depotPaths[0] + branch + "/" range = "@1,%s" % maxChange #print "prefix" + branchPrefix @@ -3607,12 +3607,12 @@ class P4Sync(Command, P4UserMap): fullBranch = self.projectName + branch if fullBranch not in self.p4BranchesInGit: if not self.silent: - print("\n Importing new branch %s" % fullBranch); + print("\n Importing new branch %s" % fullBranch) if self.importNewBranch(branch, change - 1): parent = "" self.p4BranchesInGit.append(fullBranch) if not self.silent: - print("\n Resuming with change %s" % change); + print("\n Resuming with change %s" % change) if self.verbose: print("parent determined through known branches: %s" % parent) @@ -3680,7 +3680,7 @@ class P4Sync(Command, P4UserMap): % info['data']) if info['data'].find("must refer to client") >= 0: sys.stderr.write("This particular p4 error is misleading.\n") - sys.stderr.write("Perhaps the depot path was misspelled.\n"); + sys.stderr.write("Perhaps the depot path was misspelled.\n") sys.stderr.write("Depot path: %s\n" % " ".join(self.depotPaths)) sys.exit(1) if 'p4ExitCode' in info: @@ -3789,7 +3789,7 @@ class P4Sync(Command, P4UserMap): self.importProcess = subprocess.Popen(["git", "fast-import"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE); + stderr=subprocess.PIPE) self.gitOutput = self.importProcess.stdout self.gitStream = self.importProcess.stdin self.gitError = self.importProcess.stderr @@ -3975,7 +3975,7 @@ class P4Sync(Command, P4UserMap): self.loadUserMapFromCache() self.labels = {} if self.detectLabels: - self.getLabels(); + self.getLabels() if self.detectBranches: ## FIXME - what's a P4 projectName ? @@ -4061,9 +4061,9 @@ class P4Rebase(Command): def rebase(self): if os.system("git update-index --refresh") != 0: - die("Some files in your working directory are modified and different than what is in your index. You can use git update-index to bring the index up to date or stash away all your changes with git stash."); + die("Some files in your working directory are modified and different than what is in your index. You can use git update-index to bring the index up to date or stash away all your changes with git stash.") if len(read_pipe(["git", "diff-index", "HEAD", "--"])) > 0: - die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash."); + die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.") [upstream, settings] = findUpstreamBranchPoint() if len(upstream) == 0: @@ -4362,7 +4362,7 @@ def main(): formatter = HelpFormatter()) try: - (cmd, args) = parser.parse_args(sys.argv[2:], cmd); + (cmd, args) = parser.parse_args(sys.argv[2:], cmd) except: parser.print_help() raise @@ -4378,7 +4378,7 @@ def main(): if os.path.exists(cmd.gitdir): cdup = read_pipe(["git", "rev-parse", "--show-cdup"]).strip() if len(cdup) > 0: - chdir(cdup); + chdir(cdup) if not isValidGitDir(cmd.gitdir): if isValidGitDir(cmd.gitdir + "/.git"): From 812ee74ea0d499000402fdfa3388cc69e72cdaf2 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:45 +0100 Subject: [PATCH 03/22] git-p4: indent with 4-spaces PEP8 recommends that all code should be indented in 4-space units. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#indentation Previously git-p4 had multiple cases where code was indented with a non-multiple of 4-spaces. This patch fixes each of these. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/git-p4.py b/git-p4.py index aabf657a57..a9be9ab13c 100755 --- a/git-p4.py +++ b/git-p4.py @@ -877,12 +877,12 @@ def extractLogMessageFromGitCommit(commit): ## fixme: title is first line of commit, not 1st paragraph. foundTitle = False for log in read_pipe_lines(["git", "cat-file", "commit", commit]): - if not foundTitle: - if len(log) == 1: - foundTitle = True - continue + if not foundTitle: + if len(log) == 1: + foundTitle = True + continue - logMessage += log + logMessage += log return logMessage @@ -1094,7 +1094,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent def originP4BranchesExist(): - return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master") + return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master") def p4ParseNumericChangeRange(parts): @@ -2116,7 +2116,7 @@ class P4Submit(Command, P4UserMap): submitTemplate = self.prepareLogMessage(template, logMessage, jobs) if self.preserveUser: - submitTemplate += "\n######## Actual user %s, modified after commit\n" % p4User + submitTemplate += "\n######## Actual user %s, modified after commit\n" % p4User if self.checkAuthorship and not self.p4UserIsMe(p4User): submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail @@ -2565,7 +2565,7 @@ class P4Submit(Command, P4UserMap): # exit with error unless everything applied perfectly if len(commits) != len(applied): - sys.exit(1) + sys.exit(1) return True @@ -3401,7 +3401,7 @@ class P4Sync(Command, P4UserMap): p = p[:-1] p = p[p.strip().rfind("/") + 1:] if not p.endswith("/"): - p += "/" + p += "/" return p def getBranchMapping(self): From 59ef3fc1044eb01b4691f0ca6ca0b1313f7bdc00 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:46 +0100 Subject: [PATCH 04/22] git-p4: improve consistency of docstring formatting This patch attempts to improve the consistency of the docstrings by making the following changes: - Rewraps all docstrings to a 79-character column limit. - Adds a full stop at the end of every docstring. - Removes any spaces after the opening triple-quotes of all docstrings. - Sets the hanging indent of multi-line docstrings to 3-spaces. - Ensures that the closing triple-quotes of multi-line docstrings are always on a new line indented by a 3-space indent. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 283 +++++++++++++++++++++++++++++------------------------- 1 file changed, 154 insertions(+), 129 deletions(-) diff --git a/git-p4.py b/git-p4.py index a9be9ab13c..10b51b17de 100755 --- a/git-p4.py +++ b/git-p4.py @@ -61,9 +61,9 @@ re_k_keywords = re.compile(br'\$(Id|Header|Author|Date|DateTime|Change|File|Revi def format_size_human_readable(num): - """ Returns a number of units (typically bytes) formatted as a human-readable - string. - """ + """Returns a number of units (typically bytes) formatted as a + human-readable string. + """ if num < 1024: return '{:d} B'.format(num) for unit in ["Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi"]: @@ -76,10 +76,10 @@ def format_size_human_readable(num): def p4_build_cmd(cmd): """Build a suitable p4 command line. - This consolidates building and returning a p4 command line into one - location. It means that hooking into the environment, or other configuration - can be done more easily. - """ + This consolidates building and returning a p4 command line into one + location. It means that hooking into the environment, or other + configuration can be done more easily. + """ real_cmd = ["p4"] user = gitConfig("git-p4.user") @@ -122,9 +122,9 @@ def p4_build_cmd(cmd): def git_dir(path): - """ Return TRUE if the given path is a git directory (/path/to/dir/.git). - This won't automatically add ".git" to a directory. - """ + """Return TRUE if the given path is a git directory (/path/to/dir/.git). + This won't automatically add ".git" to a directory. + """ d = read_pipe(["git", "--git-dir", path, "rev-parse", "--git-dir"], True).strip() if not d or len(d) == 0: return None @@ -133,20 +133,18 @@ def git_dir(path): def chdir(path, is_client_path=False): - """Do chdir to the given path, and set the PWD environment - variable for use by P4. It does not look at getcwd() output. - Since we're not using the shell, it is necessary to set the - PWD environment variable explicitly. + """Do chdir to the given path, and set the PWD environment variable for use + by P4. It does not look at getcwd() output. Since we're not using the + shell, it is necessary to set the PWD environment variable explicitly. - Normally, expand the path to force it to be absolute. This - addresses the use of relative path names inside P4 settings, - e.g. P4CONFIG=.p4config. P4 does not simply open the filename - as given; it looks for .p4config using PWD. + Normally, expand the path to force it to be absolute. This addresses + the use of relative path names inside P4 settings, e.g. + P4CONFIG=.p4config. P4 does not simply open the filename as given; it + looks for .p4config using PWD. - If is_client_path, the path was handed to us directly by p4, - and may be a symbolic link. Do not call os.getcwd() in this - case, because it will cause p4 to think that PWD is not inside - the client path. + If is_client_path, the path was handed to us directly by p4, and may be + a symbolic link. Do not call os.getcwd() in this case, because it will + cause p4 to think that PWD is not inside the client path. """ os.chdir(path) @@ -167,9 +165,9 @@ def calcDiskFree(): def die(msg): - """ Terminate execution. Make sure that any running child processes have been wait()ed for before - calling this. - """ + """Terminate execution. Make sure that any running child processes have + been wait()ed for before calling this. + """ if verbose: raise Exception(msg) else: @@ -178,11 +176,11 @@ def die(msg): def prompt(prompt_text): - """ Prompt the user to choose one of the choices + """Prompt the user to choose one of the choices. - Choices are identified in the prompt_text by square brackets around - a single letter option. - """ + Choices are identified in the prompt_text by square brackets around a + single letter option. + """ choices = set(m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text)) while True: sys.stderr.flush() @@ -215,8 +213,10 @@ else: def decode_path(path): - """Decode a given string (bytes or otherwise) using configured path encoding options - """ + """Decode a given string (bytes or otherwise) using configured path + encoding options. + """ + encoding = gitConfig('git-p4.pathEncoding') or 'utf_8' if bytes is not str: return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path @@ -262,10 +262,9 @@ def p4_write_pipe(c, stdin, *k, **kw): def read_pipe_full(c, *k, **kw): - """ Read output from command. Returns a tuple - of the return status, stdout text and stderr - text. - """ + """Read output from command. Returns a tuple of the return status, stdout + text and stderr text. + """ if verbose: sys.stderr.write('Reading pipe: {}\n'.format(' '.join(c))) @@ -276,12 +275,12 @@ def read_pipe_full(c, *k, **kw): def read_pipe(c, ignore_error=False, raw=False, *k, **kw): - """ Read output from command. Returns the output text on - success. On failure, terminates execution, unless - ignore_error is True, when it returns an empty string. + """Read output from command. Returns the output text on success. On + failure, terminates execution, unless ignore_error is True, when it + returns an empty string. - If raw is True, do not attempt to decode output text. - """ + If raw is True, do not attempt to decode output text. + """ (retcode, out, err) = read_pipe_full(c, *k, **kw) if retcode != 0: if ignore_error: @@ -294,9 +293,9 @@ def read_pipe(c, ignore_error=False, raw=False, *k, **kw): def read_pipe_text(c, *k, **kw): - """ Read output from a command with trailing whitespace stripped. - On error, returns None. - """ + """Read output from a command with trailing whitespace stripped. On error, + returns None. + """ (retcode, out, err) = read_pipe_full(c, *k, **kw) if retcode != 0: return None @@ -324,14 +323,15 @@ def read_pipe_lines(c, raw=False, *k, **kw): def p4_read_pipe_lines(c, *k, **kw): - """Specifically invoke p4 on the command supplied. """ + """Specifically invoke p4 on the command supplied.""" real_cmd = p4_build_cmd(c) return read_pipe_lines(real_cmd, *k, **kw) def p4_has_command(cmd): - """Ask p4 for help on this command. If it returns an error, the - command does not exist in this version of p4.""" + """Ask p4 for help on this command. If it returns an error, the command + does not exist in this version of p4. + """ real_cmd = p4_build_cmd(["help", cmd]) p = subprocess.Popen(real_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -340,10 +340,11 @@ def p4_has_command(cmd): def p4_has_move_command(): - """See if the move command exists, that it supports -k, and that - it has not been administratively disabled. The arguments - must be correct, but the filenames do not have to exist. Use - ones with wildcards so even if they exist, it will fail.""" + """See if the move command exists, that it supports -k, and that it has not + been administratively disabled. The arguments must be correct, but the + filenames do not have to exist. Use ones with wildcards so even if they + exist, it will fail. + """ if not p4_has_command("move"): return False @@ -372,7 +373,7 @@ def system(cmd, ignore_error=False, *k, **kw): def p4_system(cmd, *k, **kw): - """Specifically invoke p4 as the system command. """ + """Specifically invoke p4 as the system command.""" real_cmd = p4_build_cmd(cmd) retcode = subprocess.call(real_cmd, *k, **kw) if retcode: @@ -384,8 +385,8 @@ def die_bad_access(s): def p4_check_access(min_expiration=1): - """ Check if we can access Perforce - account still logged in - """ + """Check if we can access Perforce - account still logged in.""" + results = p4CmdList(["login", "-s"]) if len(results) == 0: @@ -433,14 +434,14 @@ _p4_version_string = None def p4_version_string(): - """Read the version string, showing just the last line, which - hopefully is the interesting version bit. + """Read the version string, showing just the last line, which hopefully is + the interesting version bit. $ p4 -V Perforce - The Fast Software Configuration Management System. Copyright 1995-2011 Perforce Software. All rights reserved. Rev. P4/NTX86/2011.1/393975 (2011/12/16). - """ + """ global _p4_version_string if not _p4_version_string: a = p4_read_pipe_lines(["-V"]) @@ -495,9 +496,11 @@ def p4_last_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.""" + """Make sure it returns a valid result by checking for the presence of + field "time". + + Return a dict of the results. + """ cmd = ["describe", "-s"] if shelved: @@ -651,21 +654,22 @@ _diff_tree_pattern = None def parseDiffTreeEntry(entry): """Parses a single diff tree entry into its component elements. - See git-diff-tree(1) manpage for details about the format of the diff - output. This method returns a dictionary with the following elements: + See git-diff-tree(1) manpage for details about the format of the diff + output. This method returns a dictionary with the following elements: - src_mode - The mode of the source file - dst_mode - The mode of the destination file - src_sha1 - The sha1 for the source file - dst_sha1 - The sha1 fr the destination file - status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc) - status_score - The score for the status (applicable for 'C' and 'R' - statuses). This is None if there is no score. - src - The path for the source file. - dst - The path for the destination file. This is only present for - copy or renames. If it is not present, this is None. + src_mode - The mode of the source file + dst_mode - The mode of the destination file + src_sha1 - The sha1 for the source file + dst_sha1 - The sha1 fr the destination file + status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc) + status_score - The score for the status (applicable for 'C' and 'R' + statuses). This is None if there is no score. + src - The path for the source file. + dst - The path for the destination file. This is only present for + copy or renames. If it is not present, this is None. - If the pattern is not matched, None is returned.""" + If the pattern is not matched, None is returned. + """ global _diff_tree_pattern if not _diff_tree_pattern: @@ -693,14 +697,16 @@ def isModeExec(mode): class P4Exception(Exception): - """ Base class for exceptions from the p4 client """ + """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 """ + """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) @@ -710,7 +716,7 @@ class P4ServerException(P4Exception): class P4RequestSizeException(P4ServerException): - """ One of the maxresults or maxscanrows errors """ + """One of the maxresults or maxscanrows errors.""" def __init__(self, exit_code, p4_result, limit): super(P4RequestSizeException, self).__init__(exit_code, p4_result) @@ -718,7 +724,7 @@ class P4RequestSizeException(P4ServerException): class P4CommandException(P4Exception): - """ Something went wrong calling p4 which means we have to give up """ + """Something went wrong calling p4 which means we have to give up.""" def __init__(self, msg): self.msg = msg @@ -943,7 +949,8 @@ def gitConfig(key, typeSpecifier=None): def gitConfigBool(key): """Return a bool, using git config --bool. It is True only if the variable is set to true, and False if set to false or not present - in the config.""" + in the config. + """ if key not in _gitConfig: _gitConfig[key] = gitConfig(key, '--bool') == "true" @@ -976,7 +983,8 @@ def p4BranchesInGit(branchesAreInRemotes=True): in remotes or heads as specified by the argument. Return a dictionary of { branch: revision } for each one found. The branch names are the short names, without any - "p4/" prefix.""" + "p4/" prefix. + """ branches = {} @@ -1207,7 +1215,8 @@ def p4PathStartsWith(path, prefix): def getClientSpec(): """Look at the p4 client spec, create a View() object that contains - all the mappings, and return it.""" + all the mappings, and return it. + """ specList = p4CmdList(["client", "-o"]) if len(specList) != 1: @@ -1290,13 +1299,15 @@ class LargeFileSystem(object): self.writeToGitStream = writeToGitStream def generatePointer(self, cloneDestination, contentFile): - """Return the content of a pointer file that is stored in Git instead of - the actual content.""" + """Return the content of a pointer file that is stored in Git instead + of the actual content. + """ assert False, "Method 'generatePointer' required in " + self.__class__.__name__ def pushFile(self, localLargeFile): """Push the actual content which is not stored in the Git repository to - a server.""" + a server. + """ assert False, "Method 'pushFile' required in " + self.__class__.__name__ def hasLargeFileExtension(self, relPath): @@ -1344,7 +1355,8 @@ class LargeFileSystem(object): def processContent(self, git_mode, relPath, contents): """Processes the content of git fast import. This method decides if a file is stored in the large file system and handles all necessary - steps.""" + steps. + """ if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath): contentTempFile = self.generateTempFile(contents) (pointer_git_mode, contents, localLargeFile) = self.generatePointer(contentTempFile) @@ -1369,7 +1381,8 @@ class MockLFS(LargeFileSystem): def generatePointer(self, contentFile): """The pointer content is the original content prefixed with "pointer-". - The local filename of the large file storage is derived from the file content. + The local filename of the large file storage is derived from the + file content. """ with open(contentFile, 'r') as f: content = next(f) @@ -1379,8 +1392,8 @@ class MockLFS(LargeFileSystem): return (gitMode, pointerContents, localLargeFile) def pushFile(self, localLargeFile): - """The remote filename of the large file storage is the same as the local - one but in a different directory. + """The remote filename of the large file storage is the same as the + local one but in a different directory. """ remotePath = os.path.join(os.path.dirname(localLargeFile), '..', 'remote') if not os.path.exists(remotePath): @@ -1390,7 +1403,8 @@ class MockLFS(LargeFileSystem): class GitLFS(LargeFileSystem): """Git LFS as backend for the git-p4 large file system. - See https://git-lfs.github.com/ for details.""" + See https://git-lfs.github.com/ for details. + """ def __init__(self, *args): LargeFileSystem.__init__(self, *args) @@ -1656,20 +1670,20 @@ class P4Submit(Command, P4UserMap): die("You have files opened with perforce! Close them before starting the sync.") def separate_jobs_from_description(self, message): - """Extract and return a possible Jobs field in the commit - message. It goes into a separate section in the p4 change - specification. + """Extract and return a possible Jobs field in the commit message. It + goes into a separate section in the p4 change specification. - A jobs line starts with "Jobs:" and looks like a new field - in a form. Values are white-space separated on the same - line or on following lines that start with a tab. + A jobs line starts with "Jobs:" and looks like a new field in a + form. Values are white-space separated on the same line or on + following lines that start with a tab. - This does not parse and extract the full git commit message - like a p4 form. It just sees the Jobs: line as a marker - to pass everything from then on directly into the p4 form, - but outside the description section. + This does not parse and extract the full git commit message like a + p4 form. It just sees the Jobs: line as a marker to pass everything + from then on directly into the p4 form, but outside the description + section. - Return a tuple (stripped log message, jobs string).""" + Return a tuple (stripped log message, jobs string). + """ m = re.search(r'^Jobs:', message, re.MULTILINE) if m is None: @@ -1680,9 +1694,10 @@ class P4Submit(Command, P4UserMap): return (stripped_message, jobtext) def prepareLogMessage(self, template, message, jobs): - """Edits the template returned from "p4 change -o" to insert - the message in the Description field, and the jobs text in - the Jobs field.""" + """Edits the template returned from "p4 change -o" to insert the + message in the Description field, and the jobs text in the Jobs + field. + """ result = "" inDescriptionSection = False @@ -1807,11 +1822,13 @@ class P4Submit(Command, P4UserMap): def prepareSubmitTemplate(self, changelist=None): """Run "p4 change -o" to grab a change specification template. + This does not use "p4 -G", as it is nice to keep the submission template in original order, since a human might edit it. Remove lines in the Files section that show changes to files - outside the depot path we're committing into.""" + outside the depot path we're committing into. + """ [upstream, settings] = findUpstreamBranchPoint() @@ -1874,8 +1891,10 @@ class P4Submit(Command, P4UserMap): return template def edit_template(self, template_file): - """Invoke the editor to let the user change the submission - message. Return true if okay to continue with the submit.""" + """Invoke the editor to let the user change the submission message. + + Return true if okay to continue with the submit. + """ # if configured to skip the editing part, just submit if gitConfigBool("git-p4.skipSubmitEdit"): @@ -2571,8 +2590,9 @@ class P4Submit(Command, P4UserMap): class View(object): - """Represent a p4 view ("p4 help views"), and map files in a - repo according to the view.""" + """Represent a p4 view ("p4 help views"), and map files in a repo according + to the view. + """ def __init__(self, client_name): self.mappings = [] @@ -2581,9 +2601,10 @@ class View(object): self.client_spec_path_cache = {} def append(self, view_line): - """Parse a view line, splitting it into depot and client - sides. Append to self.mappings, preserving order. This - is only needed for tag creation.""" + """Parse a view line, splitting it into depot and client sides. Append + to self.mappings, preserving order. This is only needed for tag + creation. + """ # Split the view line into exactly two words. P4 enforces # structure on these lines that simplifies this quite a bit. @@ -2632,7 +2653,7 @@ class View(object): return clientFile[len(self.client_prefix):] def update_client_spec_path_cache(self, files): - """ Caching file paths by "p4 where" batch query """ + """Caching file paths by "p4 where" batch query.""" # List depot file paths exclude that already cached fileArgs = [f['path'] for f in files if decode_path(f['path']) not in self.client_spec_path_cache] @@ -2664,9 +2685,11 @@ class View(object): self.client_spec_path_cache[depotFile] = b'' def map_in_client(self, depot_path): - """Return the relative location in the client where this - depot file should live. Returns "" if the file should - not be mapped in the client.""" + """Return the relative location in the client where this depot file + should live. + + Returns "" if the file should not be mapped in the client. + """ if gitConfigBool("core.ignorecase"): depot_path = depot_path.lower() @@ -2817,10 +2840,10 @@ class P4Sync(Command, P4UserMap): return jobs def stripRepoPath(self, path, prefixes): - """When streaming files, this is called to map a p4 depot path - to where it should go in git. The prefixes are either - self.depotPaths, or self.branchPrefixes in the case of - branch detection.""" + """When streaming files, this is called to map a p4 depot path to where + it should go in git. The prefixes are either self.depotPaths, or + self.branchPrefixes in the case of branch detection. + """ if self.useClientSpec: # branch detection moves files up a level (the branch name) @@ -2849,8 +2872,9 @@ class P4Sync(Command, P4UserMap): return path def splitFilesIntoBranches(self, commit): - """Look at each depotFile in the commit to figure out to what - branch it belongs.""" + """Look at each depotFile in the commit to figure out to what branch it + belongs. + """ if self.clientSpecDirs: files = self.extractFilesFromCommit(commit) @@ -3122,9 +3146,10 @@ class P4Sync(Command, P4UserMap): return "%s " % userid def streamTag(self, gitStream, labelName, labelDetails, commit, epoch): - """ Stream a p4 tag. - commit is either a git commit, or a fast-import mark, ":" - """ + """Stream a p4 tag. + + Commit is either a git commit, or a fast-import mark, ":". + """ if verbose: print("writing tag %s for commit %s" % (labelName, commit)) @@ -4175,8 +4200,7 @@ class P4Unshelve(Command): self.destbranch = "refs/remotes/p4-unshelved" def renameBranch(self, branch_name): - """ Rename the existing branch to branch_name.N - """ + """Rename the existing branch to branch_name.N .""" found = True for i in range(0,1000): @@ -4192,9 +4216,9 @@ class P4Unshelve(Command): sys.exit("gave up trying to rename existing branch {0}".format(sync.branch)) def findLastP4Revision(self, starting_point): - """ Look back from starting_point for the first commit created by git-p4 - to find the P4 commit we are based on, and the depot-paths. - """ + """Look back from starting_point for the first commit created by git-p4 + to find the P4 commit we are based on, and the depot-paths. + """ for parent in (range(65535)): log = extractLogMessageFromGitCommit("{0}~{1}".format(starting_point, parent)) @@ -4205,8 +4229,9 @@ class P4Unshelve(Command): sys.exit("could not find git-p4 commits in {0}".format(self.origin)) def createShelveParent(self, change, branch_name, sync, origin): - """ Create a commit matching the parent of the shelved changelist 'change' - """ + """Create a commit matching the parent of the shelved changelist + 'change'. + """ parent_description = p4_describe(change, shelved=True) parent_description['desc'] = 'parent for shelved changelist {}\n'.format(change) files = sync.extractFilesFromCommit(parent_description, shelved=False, shelved_cl=change) From 522e914f65da504f6b485dd8a89f49f7f176bbd0 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:47 +0100 Subject: [PATCH 05/22] git-p4: convert descriptive class and function comments into docstrings Previously, a small number of functions, methods and classes were documented using comments. This patch improves consistency by converting these into docstrings similar to those that already exist in the script. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 164 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 94 insertions(+), 70 deletions(-) diff --git a/git-p4.py b/git-p4.py index 10b51b17de..892b964eec 100755 --- a/git-p4.py +++ b/git-p4.py @@ -458,7 +458,7 @@ def p4_sync(f, *options): def p4_add(f): - # forcibly add file names with wildcards + """Forcibly add file names with wildcards.""" if wildcard_present(f): p4_system(["add", "-f", f]) else: @@ -526,12 +526,10 @@ def p4_describe(change, shelved=False): return d -# -# Canonicalize the p4 type and return a tuple of the -# base type, plus any modifiers. See "p4 help filetypes" -# for a list and explanation. -# def split_p4_type(p4type): + """Canonicalize the p4 type and return a tuple of the base type, plus any + modifiers. See "p4 help filetypes" for a list and explanation. + """ p4_filetypes_historical = { "ctempobj": "binary+Sw", @@ -562,19 +560,18 @@ def split_p4_type(p4type): return (base, mods) -# -# return the raw p4 type of a file (text, text+ko, etc) -# def p4_type(f): + """Return the raw p4 type of a file (text, text+ko, etc).""" + results = p4CmdList(["fstat", "-T", "headType", wildcard_encode(f)]) return results[0]['headType'] -# -# Given a type base and modifier, return a regexp matching -# the keywords that can be expanded in the file -# def p4_keywords_regexp_for_type(base, type_mods): + """Given a type base and modifier, return a regexp matching the keywords + that can be expanded in the file. + """ + if base in ("text", "unicode", "binary"): if "ko" in type_mods: return re_ko_keywords @@ -586,12 +583,11 @@ def p4_keywords_regexp_for_type(base, type_mods): return None -# -# Given a file, return a regexp matching the possible -# RCS keywords that will be expanded, or None for files -# with kw expansion turned off. -# def p4_keywords_regexp_for_file(file): + """Given a file, return a regexp matching the possible RCS keywords that + will be expanded, or None for files with kw expansion turned off. + """ + if not os.path.exists(file): return None else: @@ -600,8 +596,9 @@ def p4_keywords_regexp_for_file(file): def setP4ExecBit(file, mode): - # Reopens an already open file and changes the execute bit to match - # the execute bit setting in the passed in mode. + """Reopens an already open file and changes the execute bit to match the + execute bit setting in the passed in mode. + """ p4Type = "+x" @@ -616,7 +613,7 @@ def setP4ExecBit(file, mode): def getP4OpenedType(file): - # Returns the perforce file type for the given file. + """Returns the perforce file type for the given file.""" result = p4_read_pipe(["opened", wildcard_encode(file)]) match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result) @@ -626,8 +623,9 @@ def getP4OpenedType(file): die("Could not determine file type for %s (result: '%s')" % (file, result)) -# Return the set of all p4 labels def getP4Labels(depotPaths): + """Return the set of all p4 labels.""" + labels = set() if not isinstance(depotPaths, list): depotPaths = [depotPaths] @@ -639,8 +637,9 @@ def getP4Labels(depotPaths): return labels -# Return the set of all git tags def getGitTags(): + """Return the set of all git tags.""" + gitTags = set() for line in read_pipe_lines(["git", "tag"]): tag = line.strip() @@ -691,8 +690,9 @@ def parseDiffTreeEntry(entry): def isModeExec(mode): - # Returns True if the given git mode represents an executable file, - # otherwise False. + """Returns True if the given git mode represents an executable file, + otherwise False. + """ return mode[-3:] == "755" @@ -1201,13 +1201,14 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): def p4PathStartsWith(path, prefix): - # This method tries to remedy a potential mixed-case issue: - # - # If UserA adds //depot/DirA/file1 - # and UserB adds //depot/dira/file2 - # - # we may or may not have a problem. If you have core.ignorecase=true, - # we treat DirA and dira as the same directory + """This method tries to remedy a potential mixed-case issue: + + If UserA adds //depot/DirA/file1 + and UserB adds //depot/dira/file2 + + we may or may not have a problem. If you have core.ignorecase=true, + we treat DirA and dira as the same directory. + """ if gitConfigBool("core.ignorecase"): return path.lower().startswith(prefix.lower()) return path.startswith(prefix) @@ -1259,12 +1260,14 @@ def getClientRoot(): return entry["Root"] -# -# P4 wildcards are not allowed in filenames. P4 complains -# if you simply add them, but you can force it with "-f", in -# which case it translates them into %xx encoding internally. -# def wildcard_decode(path): + """Decode P4 wildcards into %xx encoding + + P4 wildcards are not allowed in filenames. P4 complains if you simply + add them, but you can force it with "-f", in which case it translates + them into %xx encoding internally. + """ + # Search for and fix just these four characters. Do % last so # that fixing it does not inadvertently create new %-escapes. # Cannot have * in a filename in windows; untested as to @@ -1278,6 +1281,8 @@ def wildcard_decode(path): def wildcard_encode(path): + """Encode %xx coded wildcards into P4 coding.""" + # do % first to avoid double-encoding the %s introduced here path = path.replace("%", "%25") \ .replace("*", "%2A") \ @@ -1524,7 +1529,7 @@ class P4UserMap: die("Could not find your p4 user id") def p4UserIsMe(self, p4User): - # return True if the given p4 user is actually me + """Return True if the given p4 user is actually me.""" me = self.p4UserId() if not p4User or p4User != me: return False @@ -1727,7 +1732,9 @@ class P4Submit(Command, P4UserMap): return result def patchRCSKeywords(self, file, regexp): - # Attempt to zap the RCS keywords in a p4 controlled file matching the given regex + """Attempt to zap the RCS keywords in a p4 controlled file matching the + given regex. + """ (handle, outFileName) = tempfile.mkstemp(dir='.') try: with os.fdopen(handle, "wb") as outFile, open(file, "rb") as inFile: @@ -1745,7 +1752,9 @@ class P4Submit(Command, P4UserMap): print("Patched up RCS keywords in %s" % file) def p4UserForCommit(self,id): - # Return the tuple (perforce user,git email) for a given git commit id + """Return the tuple (perforce user,git email) for a given git commit + id. + """ self.getUserMapFromPerforceServer() gitEmail = read_pipe(["git", "log", "--max-count=1", "--format=%ae", id]) @@ -1756,7 +1765,7 @@ class P4Submit(Command, P4UserMap): return (self.emails[gitEmail],gitEmail) def checkValidP4Users(self,commits): - # check if any git authors cannot be mapped to p4 users + """Check if any git authors cannot be mapped to p4 users.""" for id in commits: (user,email) = self.p4UserForCommit(id) if not user: @@ -1767,10 +1776,12 @@ class P4Submit(Command, P4UserMap): die("Error: %s\nSet git-p4.allowMissingP4Users to true to allow this." % msg) def lastP4Changelist(self): - # Get back the last changelist number submitted in this client spec. This - # then gets used to patch up the username in the change. If the same - # client spec is being used by multiple processes then this might go - # wrong. + """Get back the last changelist number submitted in this client spec. + + This then gets used to patch up the username in the change. If the + same client spec is being used by multiple processes then this might + go wrong. + """ results = p4CmdList(["client", "-o"]) # find the current client client = None for r in results: @@ -1786,7 +1797,7 @@ class P4Submit(Command, P4UserMap): die("Could not get changelist number for last submit - cannot patch up user details") def modifyChangelistUser(self, changelist, newUser): - # fixup the user field of a changelist after it has been submitted. + """Fixup the user field of a changelist after it has been submitted.""" changes = p4CmdList(["change", "-o", changelist]) if len(changes) != 1: die("Bad output from p4 change modifying %s to user %s" % @@ -1809,8 +1820,9 @@ class P4Submit(Command, P4UserMap): die("Could not modify user field of changelist %s to %s" % (changelist, newUser)) def canChangeChangelists(self): - # check to see if we have p4 admin or super-user permissions, either of - # which are required to modify changelists. + """Check to see if we have p4 admin or super-user permissions, either + of which are required to modify changelists. + """ results = p4CmdList(["protects", self.depotPath]) for r in results: if 'perm' in r: @@ -2262,9 +2274,11 @@ class P4Submit(Command, P4UserMap): os.remove(fileName) return submitted - # Export git tags as p4 labels. Create a p4 label and then tag - # with that. def exportGitTags(self, gitTags): + """Export git tags as p4 labels. Create a p4 label and then tag with + that. + """ + validLabelRegexp = gitConfig("git-p4.labelExportRegexp") if len(validLabelRegexp) == 0: validLabelRegexp = defaultLabelRegexp @@ -2787,8 +2801,8 @@ class P4Sync(Command, P4UserMap): 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): + """Force a checkpoint in fast-import and wait for it to finish.""" self.gitStream.write("checkpoint\n\n") self.gitStream.write("progress checkpoint\n\n") self.gitStream.flush() @@ -2934,10 +2948,12 @@ class P4Sync(Command, P4UserMap): print('Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, path)) return path - # output one file from the P4 stream - # - helper for streamP4Files - def streamOneP4File(self, file, contents): + """Output one file from the P4 stream. + + This is a helper for streamP4Files(). + """ + file_path = file['depotFile'] relPath = self.stripRepoPath(decode_path(file_path), self.branchPrefixes) @@ -3029,8 +3045,8 @@ class P4Sync(Command, P4UserMap): if self.largeFileSystem and self.largeFileSystem.isLargeFile(relPath): self.largeFileSystem.removeLargeFile(relPath) - # handle another chunk of streaming data def streamP4FilesCb(self, marshalled): + """Handle another chunk of streaming data.""" # catch p4 errors and complain err = None @@ -3094,8 +3110,9 @@ class P4Sync(Command, P4UserMap): self.stream_have_file_info = True - # Stream directly from "p4 files" into "git fast-import" def streamP4Files(self, files): + """Stream directly from "p4 files" into "git fast-import.""" + filesForCommit = [] filesToRead = [] filesToDelete = [] @@ -3199,15 +3216,18 @@ class P4Sync(Command, P4UserMap): return hasPrefix def findShadowedFiles(self, files, change): - # Perforce allows you commit files and directories with the same name, - # so you could have files //depot/foo and //depot/foo/bar both checked - # in. A p4 sync of a repository in this state fails. Deleting one of - # the files recovers the repository. - # - # Git will not allow the broken state to exist and only the most recent - # of the conflicting names is left in the repository. When one of the - # conflicting files is deleted we need to re-add the other one to make - # sure the git repository recovers in the same way as perforce. + """Perforce allows you commit files and directories with the same name, + so you could have files //depot/foo and //depot/foo/bar both checked + in. A p4 sync of a repository in this state fails. Deleting one of + the files recovers the repository. + + Git will not allow the broken state to exist and only the most + recent of the conflicting names is left in the repository. When one + of the conflicting files is deleted we need to re-add the other one + to make sure the git repository recovers in the same way as + perforce. + """ + deleted = [f for f in files if f['action'] in self.delete_actions] to_check = set() for f in deleted: @@ -3324,8 +3344,11 @@ class P4Sync(Command, P4UserMap): print("Tag %s does not match with change %s: file count is different." % (labelDetails["label"], change)) - # Build a dictionary of changelists and labels, for "detect-labels" option. def getLabels(self): + """Build a dictionary of changelists and labels, for "detect-labels" + option. + """ + self.labels = {} l = p4CmdList(["labels"] + ["%s..." % p for p in self.depotPaths]) @@ -3351,11 +3374,12 @@ class P4Sync(Command, P4UserMap): if self.verbose: print("Label changes: %s" % self.labels.keys()) - # Import p4 labels as git tags. A direct mapping does not - # exist, so assume that if all the files are at the same revision - # then we can use that, or it's something more complicated we should - # just ignore. def importP4Labels(self, stream, p4Labels): + """Import p4 labels as git tags. A direct mapping does not exist, so + assume that if all the files are at the same revision then we can + use that, or it's something more complicated we should just ignore. + """ + if verbose: print("import p4 labels: " + ' '.join(p4Labels)) From 9084961b2ab74010fa4efb88f72559b3ede5603a Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:48 +0100 Subject: [PATCH 06/22] git-p4: remove commented code Previously, the script contained commented code including Python 2 print statements. Presumably, these were used as a developer aid at some point in history. However, the commented code is generally undesirable, and this commented code serves no useful purpose. Therefore this patch removes it. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/git-p4.py b/git-p4.py index 892b964eec..53e99f81b7 100755 --- a/git-p4.py +++ b/git-p4.py @@ -3583,23 +3583,18 @@ class P4Sync(Command, P4UserMap): self.gitStream.flush() branchPrefix = self.depotPaths[0] + branch + "/" range = "@1,%s" % maxChange - #print "prefix" + branchPrefix changes = p4ChangesForPaths([branchPrefix], range, self.changes_block_size) if len(changes) <= 0: return False firstChange = changes[0] - #print "first change in branch: %s" % firstChange sourceBranch = self.knownBranches[branch] sourceDepotPath = self.depotPaths[0] + sourceBranch sourceRef = self.gitRefForBranch(sourceBranch) - #print "source " + sourceBranch branchParentChange = int(p4Cmd(["changes", "-m", "1", "%s...@1,%s" % (sourceDepotPath, firstChange)])["change"]) - #print "branch parent: %s" % branchParentChange gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange) if len(gitParent) > 0: self.initialParents[self.gitRefForBranch(branch)] = gitParent - #print "parent git commit: %s" % gitParent self.importChanges(changes) return True @@ -3742,8 +3737,6 @@ class P4Sync(Command, P4UserMap): newestRevision = change if info["action"] in self.delete_actions: - # don't increase the file cnt, otherwise details["depotFile123"] will have gaps! - #fileCnt = fileCnt + 1 continue for prop in ["depotFile", "rev", "action", "type" ]: From 794bb28d2a9853fa998c795120729277b17e047f Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:49 +0100 Subject: [PATCH 07/22] git-p4: sort and de-duplcate pylint disable list git-p4 contains configuration commands for pylint embedded in the header comment. Previously, these were combined onto single lines and not alphabetically sorted. This patch breaks each disable command onto a separate line to give cleaner diffs, removed duplicate entries, and sorts the list alphabetically. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/git-p4.py b/git-p4.py index 53e99f81b7..52c4cac683 100755 --- a/git-p4.py +++ b/git-p4.py @@ -7,13 +7,28 @@ # 2007 Trolltech ASA # License: MIT # -# pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except -# pylint: disable=no-self-use,wrong-import-position,consider-iterating-dictionary -# pylint: disable=wrong-import-order,unused-import,too-few-public-methods -# pylint: disable=too-many-lines,ungrouped-imports,fixme,too-many-locals -# pylint: disable=line-too-long,bad-whitespace,superfluous-parens -# pylint: disable=too-many-statements,too-many-instance-attributes -# pylint: disable=too-many-branches,too-many-nested-blocks +# pylint: disable=bad-whitespace +# pylint: disable=broad-except +# pylint: disable=consider-iterating-dictionary +# pylint: disable=disable +# pylint: disable=fixme +# pylint: disable=invalid-name +# pylint: disable=line-too-long +# pylint: disable=missing-docstring +# pylint: disable=no-self-use +# pylint: disable=superfluous-parens +# pylint: disable=too-few-public-methods +# pylint: disable=too-many-arguments +# pylint: disable=too-many-branches +# pylint: disable=too-many-instance-attributes +# pylint: disable=too-many-lines +# pylint: disable=too-many-locals +# pylint: disable=too-many-nested-blocks +# pylint: disable=too-many-statements +# pylint: disable=ungrouped-imports +# pylint: disable=unused-import +# pylint: disable=wrong-import-order +# pylint: disable=wrong-import-position # import sys if sys.version_info.major < 3 and sys.version_info.minor < 7: From 84af8b854481d686674622424977ecc5cdf0d587 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:50 +0100 Subject: [PATCH 08/22] git-p4: remove padding from lists, tuples and function arguments PEP8 discourages use of extraneous padding inside any parenthesis, brackets or braces in the "Pet Peeves" section: https://www.python.org/dev/peps/pep-0008/#pet-peeves This patch removes all cases of these. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/git-p4.py b/git-p4.py index 52c4cac683..f8d07cfd7f 100755 --- a/git-p4.py +++ b/git-p4.py @@ -952,10 +952,10 @@ _gitConfig = {} def gitConfig(key, typeSpecifier=None): if key not in _gitConfig: - cmd = [ "git", "config" ] + cmd = ["git", "config"] if typeSpecifier: - cmd += [ typeSpecifier ] - cmd += [ key ] + cmd += [typeSpecifier] + cmd += [key] s = read_pipe(cmd, ignore_error=True) _gitConfig[key] = s.strip() return _gitConfig[key] @@ -974,7 +974,7 @@ def gitConfigBool(key): def gitConfigInt(key): if key not in _gitConfig: - cmd = [ "git", "config", "--int", key ] + cmd = ["git", "config", "--int", key] s = read_pipe(cmd, ignore_error=True) v = s.strip() try: @@ -1030,7 +1030,7 @@ def p4BranchesInGit(branchesAreInRemotes=True): def branch_exists(branch): """Make sure that the given ref name really exists.""" - cmd = [ "git", "rev-parse", "--symbolic", "--verify", branch ] + cmd = ["git", "rev-parse", "--symbolic", "--verify", branch] p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, _ = p.communicate() out = decode_text_stream(out) @@ -1246,7 +1246,7 @@ def getClientSpec(): client_name = entry["Client"] # just the keys that start with "View" - view_keys = [ k for k in entry.keys() if k.startswith("View") ] + view_keys = [k for k in entry.keys() if k.startswith("View")] # hold this new View view = View(client_name) @@ -1512,8 +1512,8 @@ class GitLFS(LargeFileSystem): class Command: - delete_actions = ( "delete", "move/delete", "purge" ) - add_actions = ( "add", "branch", "move/add" ) + delete_actions = ("delete", "move/delete", "purge") + add_actions = ("add", "branch", "move/add") def __init__(self): self.usage = "usage: %prog [options]" @@ -2521,7 +2521,7 @@ class P4Submit(Command, P4UserMap): sys.exit(1) except Exception as e: print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed "\ - "with the error '{0}'".format(e.message) ) + "with the error '{0}'".format(e.message)) sys.exit(1) # @@ -2726,7 +2726,7 @@ class View(object): if depot_path in self.client_spec_path_cache: return self.client_spec_path_cache[depot_path] - die( "Error: %s is not found in client spec path" % depot_path ) + die("Error: %s is not found in client spec path" % depot_path) return "" @@ -3024,7 +3024,7 @@ class P4Sync(Command, P4UserMap): else: if p4_version_string().find('/NT') >= 0: text = text.replace(b'\r\n', b'\n') - contents = [ text ] + contents = [text] if type_base == "apple": # Apple filetype files will be streamed as a concatenation of @@ -3646,7 +3646,7 @@ class P4Sync(Command, P4UserMap): for branch in branches.keys(): ## HACK --hwn branchPrefix = self.depotPaths[0] + branch + "/" - self.branchPrefixes = [ branchPrefix ] + self.branchPrefixes = [branchPrefix] parent = "" @@ -3754,7 +3754,7 @@ class P4Sync(Command, P4UserMap): if info["action"] in self.delete_actions: continue - for prop in ["depotFile", "rev", "action", "type" ]: + for prop in ["depotFile", "rev", "action", "type"]: details["%s%s" % (prop, fileCnt)] = info[prop] fileCnt = fileCnt + 1 @@ -3908,7 +3908,7 @@ class P4Sync(Command, P4UserMap): if branch_arg_given: short = self.branch.split("/")[-1] if short in branches: - self.p4BranchesInGit = [ short ] + self.p4BranchesInGit = [short] else: self.p4BranchesInGit = branches.keys() @@ -4190,7 +4190,7 @@ class P4Clone(P4Sync): os.makedirs(self.cloneDestination) chdir(self.cloneDestination) - init_cmd = [ "git", "init" ] + init_cmd = ["git", "init"] if self.cloneBare: init_cmd.append("--bare") retcode = subprocess.call(init_cmd) @@ -4202,9 +4202,9 @@ class P4Clone(P4Sync): # create a master branch and check out a work tree if gitBranchExists(self.branch): - system([ "git", "branch", currentGitBranch(), self.branch ]) + system(["git", "branch", currentGitBranch(), self.branch]) if not self.cloneBare: - system([ "git", "checkout", "-f" ]) + system(["git", "checkout", "-f"]) else: print('Not checking out any branch, use ' \ '"git checkout -q -b master "') @@ -4335,7 +4335,7 @@ class P4Unshelve(Command): class P4Branches(Command): def __init__(self): Command.__init__(self) - self.options = [ ] + self.options = [] self.description = ("Shows the git branches that hold imports and their " + "corresponding perforce depot paths") self.verbose = False From 57fe2ce0e1a2cb7bd355b97230a9aa42be7da044 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:51 +0100 Subject: [PATCH 09/22] git-p4: remove spaces around default arguments PEP8 recommends that there should be no spaces around the = sign of default argument values of functions. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#other-recommendations Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/git-p4.py b/git-p4.py index f8d07cfd7f..0353bca289 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1040,7 +1040,7 @@ def branch_exists(branch): return out.rstrip() == branch -def findUpstreamBranchPoint(head = "HEAD"): +def findUpstreamBranchPoint(head="HEAD"): branches = p4BranchesInGit() # map from depot-path to branch name branchByDepotPath = {} @@ -1068,7 +1068,7 @@ def findUpstreamBranchPoint(head = "HEAD"): return ["", settings] -def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent=True): +def createOrUpdateBranchesFromOrigin(localRefPrefix="refs/remotes/p4/", silent=True): if not silent: print("Creating/updating branch(es) in %s based on origin branch(es)" % localRefPrefix) @@ -2838,7 +2838,7 @@ class P4Sync(Command, P4UserMap): return True return False - def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): + def extractFilesFromCommit(self, commit, shelved=False, shelved_cl=0): files = [] fnum = 0 while "depotFile%s" % fnum in commit: @@ -3269,7 +3269,7 @@ class P4Sync(Command, P4UserMap): 'rev': record['headRev'], 'type': record['headType']}) - def commit(self, details, files, branch, parent = "", allow_empty=False): + def commit(self, details, files, branch, parent="", allow_empty=False): epoch = details["time"] author = details["user"] jobs = self.extractJobsFromCommit(details) @@ -4415,8 +4415,8 @@ def main(): parser = optparse.OptionParser(cmd.usage.replace("%prog", "%prog " + cmdName), options, - description = cmd.description, - formatter = HelpFormatter()) + description=cmd.description, + formatter=HelpFormatter()) try: (cmd, args) = parser.parse_args(sys.argv[2:], cmd) From 0874bb016a06703d942092c23922c1e4e2bf5f4a Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:52 +0100 Subject: [PATCH 10/22] git-p4: removed brackets when assigning multiple return values In several places, git-p4 contains code of the form: (a, b) = foo() In each case, multiple values are returned through a tuple or a list and bound into multiple values. The brackets around the assigned variables are redundant and can be removed: a, b = foo() Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/git-p4.py b/git-p4.py index 0353bca289..0d444d2aa4 100755 --- a/git-p4.py +++ b/git-p4.py @@ -285,7 +285,7 @@ def read_pipe_full(c, *k, **kw): p = subprocess.Popen( c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, *k, **kw) - (out, err) = p.communicate() + out, err = p.communicate() return (p.returncode, out, decode_text_stream(err)) @@ -296,7 +296,7 @@ def read_pipe(c, ignore_error=False, raw=False, *k, **kw): If raw is True, do not attempt to decode output text. """ - (retcode, out, err) = read_pipe_full(c, *k, **kw) + retcode, out, err = read_pipe_full(c, *k, **kw) if retcode != 0: if ignore_error: out = "" @@ -311,7 +311,7 @@ def read_pipe_text(c, *k, **kw): """Read output from a command with trailing whitespace stripped. On error, returns None. """ - (retcode, out, err) = read_pipe_full(c, *k, **kw) + retcode, out, err = read_pipe_full(c, *k, **kw) if retcode != 0: return None else: @@ -365,7 +365,7 @@ def p4_has_move_command(): return False cmd = p4_build_cmd(["move", "-k", "@from", "@to"]) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - (out, err) = p.communicate() + out, err = p.communicate() err = decode_text_stream(err) # return code will be 1 in either case if err.find("Invalid option") >= 0: @@ -606,7 +606,7 @@ def p4_keywords_regexp_for_file(file): if not os.path.exists(file): return None else: - (type_base, type_mods) = split_p4_type(p4_type(file)) + type_base, type_mods = split_p4_type(p4_type(file)) return p4_keywords_regexp_for_type(type_base, type_mods) @@ -1154,7 +1154,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): parts = changeRange.split(',') assert len(parts) == 2 try: - (changeStart, changeEnd) = p4ParseNumericChangeRange(parts) + changeStart, changeEnd = p4ParseNumericChangeRange(parts) block_size = chooseBlockSize(requestedBlockSize) except ValueError: changeStart = parts[0][1:] @@ -1379,7 +1379,7 @@ class LargeFileSystem(object): """ if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath): contentTempFile = self.generateTempFile(contents) - (pointer_git_mode, contents, localLargeFile) = self.generatePointer(contentTempFile) + pointer_git_mode, contents, localLargeFile = self.generatePointer(contentTempFile) if pointer_git_mode: git_mode = pointer_git_mode if localLargeFile: @@ -1750,7 +1750,7 @@ class P4Submit(Command, P4UserMap): """Attempt to zap the RCS keywords in a p4 controlled file matching the given regex. """ - (handle, outFileName) = tempfile.mkstemp(dir='.') + handle, outFileName = tempfile.mkstemp(dir='.') try: with os.fdopen(handle, "wb") as outFile, open(file, "rb") as inFile: for line in inFile.readlines(): @@ -1782,7 +1782,7 @@ class P4Submit(Command, P4UserMap): def checkValidP4Users(self,commits): """Check if any git authors cannot be mapped to p4 users.""" for id in commits: - (user,email) = self.p4UserForCommit(id) + user, email = self.p4UserForCommit(id) if not user: msg = "Cannot find p4 user for email %s in commit %s." % (email, id) if gitConfigBool("git-p4.allowMissingP4Users"): @@ -1857,7 +1857,7 @@ class P4Submit(Command, P4UserMap): outside the depot path we're committing into. """ - [upstream, settings] = findUpstreamBranchPoint() + upstream, settings = findUpstreamBranchPoint() template = """\ # A Perforce Change Specification. @@ -1991,7 +1991,7 @@ class P4Submit(Command, P4UserMap): print("Applying", read_pipe(["git", "show", "-s", "--format=format:%h %s", id])) - (p4User, gitEmail) = self.p4UserForCommit(id) + p4User, gitEmail = self.p4UserForCommit(id) diff = read_pipe_lines( ["git", "diff-tree", "-r"] + self.diffOpts + ["{}^".format(id), id]) @@ -2156,7 +2156,7 @@ class P4Submit(Command, P4UserMap): # logMessage = extractLogMessageFromGitCommit(id) logMessage = logMessage.strip() - (logMessage, jobs) = self.separate_jobs_from_description(logMessage) + logMessage, jobs = self.separate_jobs_from_description(logMessage) template = self.prepareSubmitTemplate(update_shelve) submitTemplate = self.prepareLogMessage(template, logMessage, jobs) @@ -2174,7 +2174,7 @@ class P4Submit(Command, P4UserMap): submitTemplate += separatorLine submitTemplate += self.get_diff_description(editedFiles, filesToAdd, symlinks) - (handle, fileName) = tempfile.mkstemp() + handle, fileName = tempfile.mkstemp() tmpFile = os.fdopen(handle, "w+b") if self.isWindows: submitTemplate = submitTemplate.replace("\n", "\r\n") @@ -2381,7 +2381,7 @@ class P4Submit(Command, P4UserMap): if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","): die("%s is not in git-p4.allowSubmit" % self.master) - [upstream, settings] = findUpstreamBranchPoint() + upstream, settings = findUpstreamBranchPoint() self.depotPath = settings['depot-paths'][0] if len(self.origin) == 0: self.origin = upstream @@ -2981,7 +2981,7 @@ class P4Sync(Command, P4UserMap): file_path, relPath, format_size_human_readable(size))) sys.stdout.flush() - (type_base, type_mods) = split_p4_type(file["type"]) + type_base, type_mods = split_p4_type(file["type"]) git_mode = "100644" if "x" in type_mods: @@ -3046,7 +3046,7 @@ class P4Sync(Command, P4UserMap): contents = [regexp.sub(br'$\1$', c) for c in contents] if self.largeFileSystem: - (git_mode, contents) = self.largeFileSystem.processContent(git_mode, relPath, contents) + git_mode, contents = self.largeFileSystem.processContent(git_mode, relPath, contents) self.writeToGitStream(git_mode, relPath, contents) @@ -3514,7 +3514,7 @@ class P4Sync(Command, P4UserMap): configBranches = gitConfigList("git-p4.branchList") for branch in configBranches: if branch: - (source, destination) = branch.split(":") + source, destination = branch.split(":") self.knownBranches[destination] = source lostAndFoundBranches.discard(destination) @@ -4122,7 +4122,7 @@ class P4Rebase(Command): if len(read_pipe(["git", "diff-index", "HEAD", "--"])) > 0: die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.") - [upstream, settings] = findUpstreamBranchPoint() + upstream, settings = findUpstreamBranchPoint() if len(upstream) == 0: die("Cannot find upstream branchpoint for rebase") @@ -4419,7 +4419,7 @@ def main(): formatter=HelpFormatter()) try: - (cmd, args) = parser.parse_args(sys.argv[2:], cmd) + cmd, args = parser.parse_args(sys.argv[2:], cmd) except: parser.print_help() raise From 12a77f5b7e2d963434636472894d98caf095c0a1 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:53 +0100 Subject: [PATCH 11/22] git-p4: place a single space after every comma This patch improves consistency across git-p4 by ensuring all command separated arguments to function invocations, tuples and lists are separated by commas with a single space following. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/git-p4.py b/git-p4.py index 0d444d2aa4..c9081d79ac 100755 --- a/git-p4.py +++ b/git-p4.py @@ -99,7 +99,7 @@ def p4_build_cmd(cmd): user = gitConfig("git-p4.user") if len(user) > 0: - real_cmd += ["-u",user] + real_cmd += ["-u", user] password = gitConfig("git-p4.password") if len(password) > 0: @@ -1766,7 +1766,7 @@ class P4Submit(Command, P4UserMap): print("Patched up RCS keywords in %s" % file) - def p4UserForCommit(self,id): + def p4UserForCommit(self, id): """Return the tuple (perforce user,git email) for a given git commit id. """ @@ -1775,11 +1775,11 @@ class P4Submit(Command, P4UserMap): "--format=%ae", id]) gitEmail = gitEmail.strip() if gitEmail not in self.emails: - return (None,gitEmail) + return (None, gitEmail) else: - return (self.emails[gitEmail],gitEmail) + return (self.emails[gitEmail], gitEmail) - def checkValidP4Users(self,commits): + def checkValidP4Users(self, commits): """Check if any git authors cannot be mapped to p4 users.""" for id in commits: user, email = self.p4UserForCommit(id) @@ -3203,7 +3203,7 @@ class P4Sync(Command, P4UserMap): gitStream.write("tagger %s\n" % tagger) - print("labelDetails=",labelDetails) + print("labelDetails=", labelDetails) if 'Description' in labelDetails: description = labelDetails['Description'] else: @@ -3409,7 +3409,7 @@ class P4Sync(Command, P4UserMap): if not m.match(name): if verbose: - print("label %s does not match regexp %s" % (name,validLabelRegexp)) + print("label %s does not match regexp %s" % (name, validLabelRegexp)) continue if name in ignoredP4Labels: @@ -3730,7 +3730,7 @@ class P4Sync(Command, P4UserMap): newestRevision = 0 fileCnt = 0 - fileArgs = ["%s...%s" % (p,revision) for p in self.depotPaths] + fileArgs = ["%s...%s" % (p, revision) for p in self.depotPaths] for info in p4CmdList(["files"] + fileArgs): @@ -4235,7 +4235,7 @@ class P4Unshelve(Command): """Rename the existing branch to branch_name.N .""" found = True - for i in range(0,1000): + for i in range(0, 1000): backup_branch_name = "{0}.{1}".format(branch_name, i) if not gitBranchExists(backup_branch_name): gitUpdateRef(backup_branch_name, branch_name) # copy ref to backup From 843d847ff75f76bc9cba89bf2bf1162906f1f6be Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:54 +0100 Subject: [PATCH 12/22] git-p4: remove extraneous spaces before function arguments PEP8 recommends that there should be no spaces before function arguments in the in the "Pet Peeves" section: https://www.python.org/dev/peps/pep-0008/#pet-peeves Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/git-p4.py b/git-p4.py index c9081d79ac..862dc4ca00 100755 --- a/git-p4.py +++ b/git-p4.py @@ -911,16 +911,16 @@ def extractSettingsGitLog(log): values = {} for line in log.split("\n"): line = line.strip() - m = re.search (r"^ *\[git-p4: (.*)\]$", line) + m = re.search(r"^ *\[git-p4: (.*)\]$", line) if not m: continue - assignments = m.group(1).split (':') + assignments = m.group(1).split(':') for a in assignments: - vals = a.split ('=') + vals = a.split('=') key = vals[0].strip() - val = ('='.join (vals[1:])).strip() - if val.endswith ('\"') and val.startswith('"'): + val = ('='.join(vals[1:])).strip() + if val.endswith('\"') and val.startswith('"'): val = val[1:-1] values[key] = val @@ -2275,9 +2275,9 @@ class P4Submit(Command, P4UserMap): # Revert changes if we skip this patch if not submitted or self.shelve: if self.shelve: - print ("Reverting shelved files.") + print("Reverting shelved files.") else: - print ("Submission cancelled, undoing p4 changes.") + print("Submission cancelled, undoing p4 changes.") sys.stdout.flush() for f in editedFiles | filesToDelete: p4_revert(f) @@ -3948,7 +3948,7 @@ class P4Sync(Command, P4UserMap): i = i - 1 break - paths.append ("/".join(cur_list[:i + 1])) + paths.append("/".join(cur_list[:i + 1])) self.previousDepotPaths = paths @@ -3977,8 +3977,8 @@ class P4Sync(Command, P4UserMap): else: if self.depotPaths and self.depotPaths != args: print("previous import used depot path %s and now %s was specified. " - "This doesn't work!" % (' '.join (self.depotPaths), - ' '.join (args))) + "This doesn't work!" % (' '.join(self.depotPaths), + ' '.join(args))) sys.exit(1) self.depotPaths = sorted(args) @@ -4018,7 +4018,7 @@ class P4Sync(Command, P4UserMap): if len(self.changesFile) == 0: revision = "#head" - p = re.sub ("\.\.\.$", "", p) + p = re.sub("\.\.\.$", "", p) if not p.endswith("/"): p += "/" From 968e29e16bf6ea5565208bf5440f7bcee1be2891 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:55 +0100 Subject: [PATCH 13/22] git-p4: remove redundant backslash-continuations inside brackets PEP8 recommends that backslash line continuations should only be used for line-breaks outside parentheses. This recommendation is described in the "Maximum Line Length" section: https://www.python.org/dev/peps/pep-0008/#maximum-line-length Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/git-p4.py b/git-p4.py index 862dc4ca00..471a26744a 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2201,13 +2201,13 @@ class P4Submit(Command, P4UserMap): print(" " + self.clientPath) print("") print("To submit, use \"p4 submit\" to write a new description,") - print("or \"p4 submit -i <%s\" to use the one prepared by" \ + print("or \"p4 submit -i <%s\" to use the one prepared by" " \"git p4\"." % fileName) print("You can delete the file \"%s\" when finished." % fileName) if self.preserveUser and p4User and not self.p4UserIsMe(p4User): - print("To preserve change ownership by user %s, you must\n" \ - "do \"p4 change -f \" after submitting and\n" \ + print("To preserve change ownership by user %s, you must\n" + "do \"p4 change -f \" after submitting and\n" "edit the User field.") if pureRenameCopy: print("After submitting, renamed files must be re-synced.") @@ -2350,7 +2350,7 @@ class P4Submit(Command, P4UserMap): if self.dry_run: print("Would create p4 label %s for tag" % name) elif self.prepare_p4_only: - print("Not creating p4 label %s for tag due to option" \ + print("Not creating p4 label %s for tag due to option" " --prepare-p4-only" % name) else: p4_write_pipe(["label", "-i"], labelTemplate) @@ -2515,12 +2515,12 @@ class P4Submit(Command, P4UserMap): if not self.no_verify: try: if not run_git_hook("p4-pre-submit"): - print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nYou can skip " \ - "this pre-submission check by adding\nthe command line option '--no-verify', " \ + print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nYou can skip " + "this pre-submission check by adding\nthe command line option '--no-verify', " "however,\nthis will also skip the p4-changelist hook as well.") sys.exit(1) except Exception as e: - print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed "\ + print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed " "with the error '{0}'".format(e.message)) sys.exit(1) @@ -2543,7 +2543,7 @@ class P4Submit(Command, P4UserMap): applied.append(commit) if self.prepare_p4_only: if i < last: - print("Processing only the first commit due to option" \ + print("Processing only the first commit due to option" " --prepare-p4-only") break else: @@ -4206,7 +4206,7 @@ class P4Clone(P4Sync): if not self.cloneBare: system(["git", "checkout", "-f"]) else: - print('Not checking out any branch, use ' \ + print('Not checking out any branch, use ' '"git checkout -q -b master "') # auto-set this variable if invoked with --use-client-spec From 2bcf611088ffcc585b963efcc5c69fb6e819c1b3 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:56 +0100 Subject: [PATCH 14/22] git-p4: remove spaces between dictionary keys and colons PEP8 makes no specific recommendation about spaces preceding colons in dictionary declarations, but all the code examples contained with it declare dictionaries with a single space after the colon, and none before. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/git-p4.py b/git-p4.py index 471a26744a..167bc19775 100755 --- a/git-p4.py +++ b/git-p4.py @@ -4379,13 +4379,13 @@ def printUsage(commands): commands = { - "submit" : P4Submit, - "commit" : P4Submit, - "sync" : P4Sync, - "rebase" : P4Rebase, - "clone" : P4Clone, - "branches" : P4Branches, - "unshelve" : P4Unshelve, + "submit": P4Submit, + "commit": P4Submit, + "sync": P4Sync, + "rebase": P4Rebase, + "clone": P4Clone, + "branches": P4Branches, + "unshelve": P4Unshelve, } From c785e2029c478dce241f9e07e657ec372b633e6c Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:57 +0100 Subject: [PATCH 15/22] git-p4: ensure every comment has a single # PEP8 recommends that every comment should begin with a single '#' character. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#comments Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/git-p4.py b/git-p4.py index 167bc19775..88c2b5213e 100755 --- a/git-p4.py +++ b/git-p4.py @@ -895,7 +895,7 @@ def branchExists(ref): def extractLogMessageFromGitCommit(commit): logMessage = "" - ## fixme: title is first line of commit, not 1st paragraph. + # fixme: title is first line of commit, not 1st paragraph. foundTitle = False for log in read_pipe_lines(["git", "cat-file", "commit", commit]): if not foundTitle: @@ -3485,7 +3485,7 @@ class P4Sync(Command, P4UserMap): continue source = paths[0] destination = paths[1] - ## HACK + # HACK if p4PathStartsWith(source, self.depotPaths[0]) and p4PathStartsWith(destination, self.depotPaths[0]): source = source[len(self.depotPaths[0]):-4] destination = destination[len(self.depotPaths[0]):-4] @@ -3644,7 +3644,7 @@ class P4Sync(Command, P4UserMap): if self.detectBranches: branches = self.splitFilesIntoBranches(description) for branch in branches.keys(): - ## HACK --hwn + # HACK --hwn branchPrefix = self.depotPaths[0] + branch + "/" self.branchPrefixes = [branchPrefix] @@ -4035,7 +4035,7 @@ class P4Sync(Command, P4UserMap): self.getLabels() if self.detectBranches: - ## FIXME - what's a P4 projectName ? + # FIXME - what's a P4 projectName ? self.projectName = self.guessProjectName() if self.hasOrigin: @@ -4048,7 +4048,7 @@ class P4Sync(Command, P4UserMap): for b in self.p4BranchesInGit: if b != "master": - ## FIXME + # FIXME b = b[len(self.projectName):] self.createdBranches.add(b) @@ -4154,7 +4154,7 @@ class P4Clone(P4Sync): self.cloneBare = False def defaultDestination(self, args): - ## TODO: use common prefix of args? + # TODO: use common prefix of args? depotPath = args[0] depotDir = re.sub("(@[^@]*)$", "", depotPath) depotDir = re.sub("(#[^#]*)$", "", depotDir) From 6febb9f84307fab60af6873258006d98d97be87f Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:58 +0100 Subject: [PATCH 16/22] git-p4: ensure there is a single space around all operators PEP8 requires that binary operators such as assignment and comparison operators should always be surrounded by a pair of single spaces, and recommends that all other binary operators should typically be surround by single spaces. The recommendation is given here in the "Other Recommendations" section https://www.python.org/dev/peps/pep-0008/#other-recommendations Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/git-p4.py b/git-p4.py index 88c2b5213e..97c2f82ee8 100755 --- a/git-p4.py +++ b/git-p4.py @@ -67,7 +67,7 @@ verbose = False defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$' # The block size is reduced automatically if required -defaultBlockSize = 1<<20 +defaultBlockSize = 1 << 20 p4_access_checked = False @@ -201,7 +201,7 @@ def prompt(prompt_text): sys.stderr.flush() sys.stdout.write(prompt_text) sys.stdout.flush() - response=sys.stdin.readline().strip().lower() + response = sys.stdin.readline().strip().lower() if not response: continue response = response[0] @@ -2339,7 +2339,7 @@ class P4Submit(Command, P4UserMap): # Create the label - use the same view as the client spec we are using clientSpec = getClientSpec() - labelTemplate = "Label: %s\n" % name + labelTemplate = "Label: %s\n" % name labelTemplate += "Description:\n" for b in body: labelTemplate += "\t" + b + "\n" @@ -2842,7 +2842,7 @@ class P4Sync(Command, P4UserMap): files = [] fnum = 0 while "depotFile%s" % fnum in commit: - path = commit["depotFile%s" % fnum] + path = commit["depotFile%s" % fnum] found = self.isPathWanted(decode_path(path)) if not found: fnum = fnum + 1 @@ -3925,7 +3925,7 @@ class P4Sync(Command, P4UserMap): p4Change = 0 for branch in self.p4BranchesInGit: - logMsg = extractLogMessageFromGitCommit(self.refPrefix + branch) + logMsg = extractLogMessageFromGitCommit(self.refPrefix + branch) settings = extractSettingsGitLog(logMsg) From 7a3e83d0bd1d8fe004cd2f3dd4bc03bc8ca6fe9e Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:24:59 +0100 Subject: [PATCH 17/22] git-p4: normalize indentation of lines in conditionals PEP8 recommends that when wrapping the arguments of conditional statements, an extra level of indentation should be added to distinguish arguments from the body of the statement. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#indentation This patch either adds the indentation, or removes unnecessary wrapping. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/git-p4.py b/git-p4.py index 97c2f82ee8..a25adc8bae 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1085,8 +1085,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix="refs/remotes/p4/", silent=T originHead = line original = extractSettingsGitLog(extractLogMessageFromGitCommit(originHead)) - if ('depot-paths' not in original - or 'change' not in original): + if 'depot-paths' not in original or 'change' not in original: continue update = False @@ -2098,8 +2097,8 @@ class P4Submit(Command, P4UserMap): if regexp: # this file is a possibility...look for RCS keywords. for line in read_pipe_lines( - ["git", "diff", "%s^..%s" % (id, id), file], - raw=True): + ["git", "diff", "%s^..%s" % (id, id), file], + raw=True): if regexp.search(line): if verbose: print("got keyword match on %s in %s in %s" % (regex.pattern, line, file)) @@ -3112,9 +3111,9 @@ class P4Sync(Command, P4UserMap): self.stream_file[k] = marshalled[k] if (verbose and - 'streamContentSize' in self.stream_file and - 'fileSize' in self.stream_file and - 'depotFile' in self.stream_file): + 'streamContentSize' in self.stream_file and + 'fileSize' in self.stream_file and + 'depotFile' in self.stream_file): size = int(self.stream_file["fileSize"]) if size > 0: progress = 100*self.stream_file['streamContentSize']/size @@ -3930,8 +3929,7 @@ class P4Sync(Command, P4UserMap): settings = extractSettingsGitLog(logMsg) self.readOptions(settings) - if ('depot-paths' in settings - and 'change' in settings): + if 'depot-paths' in settings and 'change' in settings: change = int(settings['change']) + 1 p4Change = max(p4Change, change) From da0134f6534f407f8377ea97eb040ee6e49412e1 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:25:00 +0100 Subject: [PATCH 18/22] git-p4: compare to singletons with "is" and "is not" PEP8 recommends that comparisons with singletons such as None should be done with "is" and "is not", and never equality operators. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#programming-recommendations Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index a25adc8bae..2f2450f7d9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -857,7 +857,7 @@ def p4Where(depotPath): if data[:space] == depotPath: output = entry break - if output == None: + if output is None: return "" if output["code"] == "error": return "" @@ -879,7 +879,7 @@ def currentGitBranch(): def isValidGitDir(path): - return git_dir(path) != None + return git_dir(path) is not None def parseRevision(ref): @@ -4425,7 +4425,7 @@ def main(): global verbose verbose = cmd.verbose if cmd.needsGit: - if cmd.gitdir == None: + if cmd.gitdir is None: cmd.gitdir = os.path.abspath(".git") if not isValidGitDir(cmd.gitdir): # "rev-parse --git-dir" without arguments will try $PWD/.git From 77956b9de5266de6e597509a4d58e2f0bcafd09a Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:25:01 +0100 Subject: [PATCH 19/22] git-p4: only seperate code blocks by a single empty line PEP8 recommends that blank lines should be used sparingly to separate sections in the "Blank Lines" section: https://www.python.org/dev/peps/pep-0008/#blank-lines This patch replaces all double blank-line separations with a single blank line. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index 2f2450f7d9..c003b6246d 100755 --- a/git-p4.py +++ b/git-p4.py @@ -3521,7 +3521,6 @@ class P4Sync(Command, P4UserMap): if source not in self.knownBranches: lostAndFoundBranches.add(source) - for branch in lostAndFoundBranches: self.knownBranches[branch] = branch @@ -3745,7 +3744,6 @@ class P4Sync(Command, P4UserMap): sys.stderr.write("p4 exitcode: %s\n" % info['p4ExitCode']) sys.exit(1) - change = int(info["change"]) if change > newestRevision: newestRevision = change @@ -3773,7 +3771,6 @@ class P4Sync(Command, P4UserMap): print("IO error details: {}".format(err)) print(self.gitError.read()) - def importRevisions(self, args, branch_arg_given): changes = [] From 4768af208850ef3de74908c498acf9f8956cf545 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:25:02 +0100 Subject: [PATCH 20/22] git-p4: move inline comments to line above PEP8 recommends that all inline comments should be separated from code by two spaces, in the "Inline Comments" section: https://www.python.org/dev/peps/pep-0008/#inline-comments However, because all instances of these inline comments extended to an excessive line length, they have been moved onto a seprate line. Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index c003b6246d..524a7a5c99 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1979,7 +1979,9 @@ class P4Submit(Command, P4UserMap): for line in f.readlines(): newdiff += "+" + line except UnicodeDecodeError: - pass # Found non-text data and skip, since diff description should only include text + # Found non-text data and skip, since diff description + # should only include text + pass f.close() return (diff + newdiff).replace('\r\n', '\n') @@ -2975,7 +2977,8 @@ class P4Sync(Command, P4UserMap): if 'fileSize' in self.stream_file: size = int(self.stream_file['fileSize']) else: - size = 0 # deleted files don't get a fileSize apparently + # Deleted files don't get a fileSize apparently + size = 0 sys.stdout.write('\r%s --> %s (%s)\n' % ( file_path, relPath, format_size_human_readable(size))) sys.stdout.flush() @@ -4233,7 +4236,8 @@ class P4Unshelve(Command): for i in range(0, 1000): backup_branch_name = "{0}.{1}".format(branch_name, i) if not gitBranchExists(backup_branch_name): - gitUpdateRef(backup_branch_name, branch_name) # copy ref to backup + # Copy ref to backup + gitUpdateRef(backup_branch_name, branch_name) gitDeleteRef(branch_name) found = True print("renamed old unshelve branch to {0}".format(backup_branch_name)) From e8f8b3b2a34fadaa559b25ec734c78ab9ef1228f Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:25:03 +0100 Subject: [PATCH 21/22] git-p4: seperate multiple statements onto seperate lines PEP8 discourages the use of compound statements where there are multiple statements on a single line in the "Other Recommendations" section: https://www.python.org/dev/peps/pep-0008/#other-recommendations Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index 524a7a5c99..c3239cdabc 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1191,7 +1191,8 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): else: block_size = max(2, block_size // 2) - if verbose: print("block size error, retrying with block size {0}".format(block_size)) + 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)) @@ -1818,7 +1819,9 @@ class P4Submit(Command, P4UserMap): (changelist, newUser)) c = changes[0] - if c['User'] == newUser: return # nothing to do + if c['User'] == newUser: + # Nothing to do + return c['User'] = newUser # p4 does not understand format version 3 and above input = marshal.dumps(c, 2) From 4ff0108d9e21e274b4fefa840a84ff4953ec8e06 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 1 Apr 2022 15:25:04 +0100 Subject: [PATCH 22/22] git-p4: sort imports Signed-off-by: Joel Holdsworth Signed-off-by: Junio C Hamano --- git-p4.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/git-p4.py b/git-p4.py index c3239cdabc..ea1d09f69f 100755 --- a/git-p4.py +++ b/git-p4.py @@ -30,26 +30,28 @@ # pylint: disable=wrong-import-order # pylint: disable=wrong-import-position # + import sys if sys.version_info.major < 3 and sys.version_info.minor < 7: sys.stderr.write("git-p4: requires Python 2.7 or later.\n") sys.exit(1) -import os -import optparse + +import ctypes +import errno import functools +import glob import marshal -import subprocess -import tempfile -import time +import optparse +import os import platform import re import shutil import stat +import subprocess +import tempfile +import time import zipfile import zlib -import ctypes -import errno -import glob # On python2.7 where raw_input() and input() are both availble, # we want raw_input's semantics, but aliased to input for python3