Subject: Re: [PATCH] git-p4: use raw string literals for regular expressions

From: Junio C Hamano <[email-masked]>
Date: Sat, 20 Jan 2024 10:47:10 -0800

"James Touton via GitGitGadget" <[email-masked]> writes:

> From: James Touton <[email-masked]>
>
> Fixes several Python diagnostics about invalid escape sequences.

Thanks for noticing, but we want a bit more backstory explained here
in the proposed commit log message, outlining:

1. With what version of Python the deprecation warning started.

This will help us judge the urgency of the fix.  If I am reading the
docs.python.org/$version/howto/regex.html right, we do not see this

    In addition, special escape sequences that are valid in regular
    expressions, but not valid as Python string literals, now result
    in a DeprecationWarning and will eventually become a
    SyntaxError, which means the sequences will be invalid if raw
    string notation or escaping the backslashes isn’t used.

in Python 3.5's document, but Python 3.6's document starts talking
about the warning.  Python 3.6 was released at the end of 2016 so it
is 7 years old---users have lived with the warning for this many
years, so if the above reasoning is correct, this is not all that
urgent to require a maintenance release.

2. How well the new construct, used by the code after applying this
   patch, is supported by older version of Python.

This will assure us that the change will not be robbing from users
of older versions of Python to pay users of newer versions of
Python.  Again, if I am reading the documentation right, feeding r''
raw strings to regexp engine was supported even by Python 2.7, which
is what git-p4.py already requires, so we should be OK.

But we want the developers who propose a change to explain why it is
a good idea, and why it is a safe change to make, in their proposed
commit log message, instead of forcing the reviewers to do that for
them.

For other syntactic and linguistic hints on writing a proposed log
message, please check Documentation/SubmittingPatches document.

Thanks, again.

> Signed-off-by: James Touton <[email-masked]>
> ---
>     git-p4: use raw string literals for regular expressions
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1639%2FBekenn%2Fp4-raw-strings-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1639/Bekenn/p4-raw-strings-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1639
>
>  git-p4.py | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 0eb3bb4c47d..156597adb59 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -689,8 +689,8 @@ def setP4ExecBit(file, mode):
>  
>      if not isModeExec(mode):
>          p4Type = getP4OpenedType(file)
> -        p4Type = re.sub('^([cku]?)x(.*)', '\\1\\2', p4Type)
> -        p4Type = re.sub('(.*?\+.*?)x(.*?)', '\\1\\2', p4Type)
> +        p4Type = re.sub(r'^([cku]?)x(.*)', r'\1\2', p4Type)
> +        p4Type = re.sub(r'(.*?\+.*?)x(.*?)', r'\1\2', p4Type)
>          if p4Type[-1] == "+":
>              p4Type = p4Type[0:-1]
>  
> @@ -701,7 +701,7 @@ def getP4OpenedType(file):
>      """Returns the perforce file type for the given file."""
>  
>      result = p4_read_pipe(["opened", wildcard_encode(file)])
> -    match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result)
> +    match = re.match(r".*\((.+)\)( \*exclusive\*)?\r?$", result)
>      if match:
>          return match.group(1)
>      else:
> @@ -757,7 +757,7 @@ def parseDiffTreeEntry(entry):
>  
>      global _diff_tree_pattern
>      if not _diff_tree_pattern:
> -        _diff_tree_pattern = re.compile(':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
> +        _diff_tree_pattern = re.compile(r':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
>  
>      match = _diff_tree_pattern.match(entry)
>      if match:
> @@ -918,9 +918,9 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
>              if len(result) > 0:
>                  data = result[0].get('data')
>                  if data:
> -                    m = re.search('Too many rows scanned \(over (\d+)\)', data)
> +                    m = re.search(r'Too many rows scanned \(over (\d+)\)', data)
>                      if not m:
> -                        m = re.search('Request too large \(over (\d+)\)', data)
> +                        m = re.search(r'Request too large \(over (\d+)\)', data)
>  
>                      if m:
>                          limit = int(m.group(1))
> @@ -1452,7 +1452,7 @@ def wildcard_encode(path):
>  
>  
>  def wildcard_present(path):
> -    m = re.search("[*#@%]", path)
> +    m = re.search(r"[*#@%]", path)
>      return m is not None
>  
>  
> @@ -3048,7 +3048,7 @@ def stripRepoPath(self, path, prefixes):
>              # Preserve everything in relative path name except leading
>              # //depot/; just look at first prefix as they all should
>              # be in the same depot.
> -            depot = re.sub("^(//[^/]+/).*", r'\1', prefixes[0])
> +            depot = re.sub(r"^(//[^/]+/).*", r'\1', prefixes[0])
>              if p4PathStartsWith(path, depot):
>                  path = path[len(depot):]
>  
> @@ -3603,7 +3603,7 @@ def importP4Labels(self, stream, p4Labels):
>                      commitFound = True
>                  else:
>                      gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
> -                        "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
> +                        "--reverse", r":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
>                      if len(gitCommit) == 0:
>                          print("importing label %s: could not find git commit for changelist %d" % (name, changelist))
>                      else:
> @@ -4182,7 +4182,7 @@ def run(self, args):
>                  if len(self.changesFile) == 0:
>                      revision = "#head"
>  
> -            p = re.sub("\.\.\.$", "", p)
> +            p = re.sub(r"\.\.\.$", "", p)
>              if not p.endswith("/"):
>                  p += "/"
>  
> @@ -4291,7 +4291,7 @@ def rebase(self):
>              die("Cannot find upstream branchpoint for rebase")
>  
>          # the branchpoint may be p4/foo~3, so strip off the parent
> -        upstream = re.sub("~[0-9]+$", "", upstream)
> +        upstream = re.sub(r"~[0-9]+$", "", upstream)
>  
>          print("Rebasing the current branch onto %s" % upstream)
>          oldHead = read_pipe(["git", "rev-parse", "HEAD"]).strip()
> @@ -4320,8 +4320,8 @@ def __init__(self):
>      def defaultDestination(self, args):
>          # TODO: use common prefix of args?
>          depotPath = args[0]
> -        depotDir = re.sub("(@[^@]*)$", "", depotPath)
> -        depotDir = re.sub("(#[^#]*)$", "", depotDir)
> +        depotDir = re.sub(r"(@[^@]*)$", "", depotPath)
> +        depotDir = re.sub(r"(#[^#]*)$", "", depotDir)
>          depotDir = re.sub(r"\.\.\.$", "", depotDir)
>          depotDir = re.sub(r"/$", "", depotDir)
>          return os.path.split(depotDir)[1]
>
> base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d