project: Check if dotgit exists w/out symlink check

os.path.exists returns false on a broken symlink. This is not what repo
needs when checking if a project is setup properly.

For example, if src/foo/.git can't be resolved, repo tries to create
symlink and that results in FileExistsError.

Use lexists which returns True even if symlink is broken. That will
force path where repo checks where symlink is pointing to and will fix
it to the correct location.

Bug: b/281746795
Change-Id: Id3f7dc3a3cb6499d02ce7335eca992ddc7deb645
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/415197
Tested-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: George Engelbrecht <engeg@google.com>
Reviewed-by: Greg Edelston <gredelston@google.com>

docs/python-support.md

@@ -33,9 +33,8 @@ you have newer versions installed, your choices are:
 
 *   Modify the [repo launcher]'s shebang to suite your environment.
 *   Download an older version of the [repo launcher] and don't upgrade it.
-    Be aware that there is no guarantee old repo launchers are WILL work with
-    current versions of repo.  Bug reports using old launchers will not be
-    accepted.
+    Be aware that we do not guarantee old repo launchers will work with current
+    versions of repo.  Bug reports using old launchers will not be accepted.
 
 ## When to drop support
 

docs/release-process.md

@@ -202,7 +202,7 @@ still support them.
 Things in italics are things we used to care about but probably don't anymore.
 
 |   Date   |     EOL      | [Git][rel-g] | [Python][rel-p] | [SSH][rel-o] | [Ubuntu][rel-u] / [Debian][rel-d] | Git | Python | SSH |
-|:--------:|:------------:|:------------:|:---------------:|:------------:|-----------------------------------|-----|--------|-----|
+|:--------:|:------------:|:------------:|:---------------:|:------------:|-----------------------------------|:---:|:------:|:---:|
 | Apr 2008 |              |              |                 | 5.0          |
 | Jun 2008 |              |              |                 | 5.1          |
 | Oct 2008 | *Oct 2013*   |              | 2.6.0           |              | *10.04 Lucid* - 10.10 Maverick / *Squeeze* |
@@ -241,7 +241,7 @@ Things in italics are things we used to care about but probably don't anymore.
 | Feb 2014 | *Dec 2014*   | **1.9.0**    |                 |              | *14.04 Trusty* |
 | Mar 2014 | *Mar 2019*   |              | *3.4.0*         |              | *14.04 Trusty* - 15.10 Wily / *Jessie* |
 | Mar 2014 |              |              |                 | 6.6          | *14.04 Trusty* - 14.10 Utopic |
-| Apr 2014 | *Apr 2022*   |              |                 |              | *14.04 Trusty*       | 1.9.1    | 2.7.5 3.4.0  | 6.6 |
+| Apr 2014 | *Apr 2024*   |              |                 |              | *14.04 Trusty*       | 1.9.1    | 2.7.5 3.4.0  | 6.6 |
 | May 2014 | *Dec 2014*   | 2.0.0        |
 | Aug 2014 | *Dec 2014*   | *2.1.0*      |                 |              | 14.10 Utopic - 15.04 Vivid / *Jessie* |
 | Oct 2014 |              |              |                 | 6.7          | 15.04 Vivid |
@@ -262,7 +262,7 @@ Things in italics are things we used to care about but probably don't anymore.
 | Jan 2016 | *Jul 2017*   | *2.7.0*      |                 |              | *16.04 Xenial* |
 | Feb 2016 |              |              |                 | 7.2          | *16.04 Xenial* |
 | Mar 2016 | *Jul 2017*   | 2.8.0        |
-| Apr 2016 | *Apr 2024*   |              |                 |              | *16.04 Xenial*       | 2.7.4    | 2.7.11 3.5.1 | 7.2 |
+| Apr 2016 | *Apr 2026*   |              |                 |              | *16.04 Xenial*       | 2.7.4    | 2.7.11 3.5.1 | 7.2 |
 | Jun 2016 | *Jul 2017*   | 2.9.0        |                 |              | 16.10 Yakkety |
 | Jul 2016 |              |              |                 | 7.3          | 16.10 Yakkety |
 | Sep 2016 | *Sep 2017*   | 2.10.0       |
@@ -312,14 +312,33 @@ Things in italics are things we used to care about but probably don't anymore.
 | Oct 2020 |              |              |                 |              | 20.10 Groovy         | 2.27.0   | 2.7.18 3.8.6 | 8.3 |
 | Oct 2020 | **Oct 2025** |              | 3.9.0           |              | 21.04 Hirsute / **Bullseye** |
 | Dec 2020 | *Mar 2021*   | 2.30.0       |                 |              | 21.04 Hirsute / **Bullseye** |
-| Mar 2021 |              | 2.31.0       |
-| Mar 2021 |              |              |                 | 8.5          |
+| Mar 2021 |              | 2.31.0       |                 | 8.5          |
 | Apr 2021 |              |              |                 | 8.6          |
 | Apr 2021 | *Jan 2022*   |              |                 |              | 21.04 Hirsute        | 2.30.2   | 2.7.18 3.9.4 | 8.4 |
 | Jun 2021 |              | 2.32.0       |
-| Aug 2021 |              | 2.33.0       |
-| Aug 2021 |              |              |                 | 8.7          |
+| Aug 2021 |              | 2.33.0       |                 | 8.7          |
 | Aug 2021 | **Aug 2026** |              |                 |              | **Debian 11 Bullseye** | 2.30.2 | 2.7.18 3.9.2 | 8.4 |
+| Sep 2021 |              |              |                 | 8.8          |
+| Oct 2021 |              | 2.34.0       | 3.10.0          |              | **22.04 Jammy** |
+| Jan 2022 |              | 2.35.0       |
+| Feb 2022 |              |              |                 | 8.9          | **22.04 Jammy** |
+| Apr 2022 |              | 2.36.0       |                 | 9.0          |
+| Apr 2022 | **Apr 2032** |              |                 |              | **22.04 Jammy**      | 2.34.1 | 2.7.18 3.10.6 | 8.9 |
+| Jun 2022 |              | 2.37.0       |
+| Oct 2022 |              | 2.38.0       |                 | 9.1          |
+| Oct 2022 |              |              | 3.11.0          |              | **Bookworm** |
+| Dec 2022 |              | 2.39.0       |                 |              | **Bookworm** |
+| Feb 2023 |              |              |                 | 9.2          | **Bookworm** |
+| Mar 2023 |              | 2.40.0       |                 | 9.3          |
+| Jun 2023 |              | 2.41.0       |
+| Jun 2023 | **Jun 2028** |              |                 |              | **Debian 12 Bookworm** | 2.39.2 | 3.11.2 | 9.2 |
+| Aug 2023 |              | 2.42.0       |                 | 9.4          |
+| Oct 2023 |              |              | 3.12.0          | 9.5          |
+| Nov 2022 |              | 2.43.0       |
+| Dec 2023 |              |              |                 | 9.6          |
+| Feb 2024 |              | 2.44.0       |
+| Mar 2024 |              |              |                 | 9.7          |
+| Oct 2024 |              |              | 3.13.0          |
 | **Date** |   **EOL**    | **[Git][rel-g]** | **[Python][rel-p]** | **[SSH][rel-o]** | **[Ubuntu][rel-u] / [Debian][rel-d]** | **Git** | **Python** | **SSH** |
 
 
@@ -328,7 +347,7 @@ Things in italics are things we used to care about but probably don't anymore.
 [rel-g]: https://en.wikipedia.org/wiki/Git#Releases
 [rel-o]: https://www.openssh.com/releasenotes.html
 [rel-p]: https://en.wikipedia.org/wiki/History_of_Python#Table_of_versions
-[rel-u]: https://en.wikipedia.org/wiki/Ubuntu_version_history#Table_of_versions
+[rel-u]: https://wiki.ubuntu.com/Releases
 [example announcement]: https://groups.google.com/d/topic/repo-discuss/UGBNismWo1M/discussion
 [repo-discuss@googlegroups.com]: https://groups.google.com/forum/#!forum/repo-discuss
 [go/repo-release]: https://goto.google.com/repo-release

git_command.py

@@ -135,6 +135,8 @@ def GetEventTargetPath():
     if retval == 0:
         # Strip trailing carriage-return in path.
         path = p.stdout.rstrip("\n")
+        if path == "":
+            return None
     elif retval != 1:
         # `git config --get` is documented to produce an exit status of `1`
         # if the requested variable is not present in the configuration.

git_trace2_event_log_base.py

@@ -38,6 +38,8 @@ import tempfile
 import threading
 
 
+# Timeout when sending events via socket (applies to connect, send)
+SOCK_TIMEOUT = 0.5  # in seconds
 # BaseEventLog __init__ Counter that is consistent within the same process
 p_init_count = 0
 
@@ -296,6 +298,7 @@ class BaseEventLog:
                     with socket.socket(
                         socket.AF_UNIX, socket.SOCK_STREAM
                     ) as sock:
+                        sock.settimeout(SOCK_TIMEOUT)
                         sock.connect(path)
                         self._WriteLog(sock.sendall)
                     return f"af_unix:stream:{path}"

main.py

@@ -270,10 +270,14 @@ class _Repo:
             self._PrintHelp(short=True)
             return 1
 
-        run = lambda: self._RunLong(name, gopts, argv) or 0
+        git_trace2_event_log = EventLog()
+        run = (
+            lambda: self._RunLong(name, gopts, argv, git_trace2_event_log) or 0
+        )
         with Trace(
-            "starting new command: %s",
+            "starting new command: %s [sid=%s]",
             ", ".join([name] + argv),
+            git_trace2_event_log.full_sid,
             first_trace=True,
         ):
             if gopts.trace_python:
@@ -290,12 +294,11 @@ class _Repo:
                 result = run()
         return result
 
-    def _RunLong(self, name, gopts, argv):
+    def _RunLong(self, name, gopts, argv, git_trace2_event_log):
         """Execute the (longer running) requested subcommand."""
         result = 0
         SetDefaultColoring(gopts.color)
 
-        git_trace2_event_log = EventLog()
         outer_client = RepoClient(self.repodir)
         repo_client = outer_client
         if gopts.submanifest_path:
@@ -422,7 +425,7 @@ class _Repo:
                         error_info = json.dumps(
                             {
                                 "ErrorType": type(error).__name__,
-                                "Project": project,
+                                "Project": str(project),
                                 "Message": str(error),
                             }
                         )

manifest_xml.py

@@ -133,8 +133,8 @@ def normalize_url(url: str) -> str:
     url = url.rstrip("/")
     parsed_url = urllib.parse.urlparse(url)
 
-    # This matches patterns like "git@github.com:foo/bar".
-    scp_like_url_re = r"^[^:]+@[^:]+:[^/]+/"
+    # This matches patterns like "git@github.com:foo".
+    scp_like_url_re = r"^[^/:]+@[^/:]+:[^/]+"
 
     # If our URL is missing a schema and matches git's
     # SCP-like syntax we should convert it to a proper

project.py

@@ -21,6 +21,7 @@ import random
 import re
 import shutil
 import stat
+import string
 import subprocess
 import sys
 import tarfile
@@ -266,6 +267,7 @@ class ReviewableBranch:
         dest_branch=None,
         validate_certs=True,
         push_options=None,
+        patchset_description=None,
     ):
         self.project.UploadForReview(
             branch=self.name,
@@ -281,6 +283,7 @@ class ReviewableBranch:
             dest_branch=dest_branch,
             validate_certs=validate_certs,
             push_options=push_options,
+            patchset_description=patchset_description,
         )
 
     def GetPublishedRefs(self):
@@ -1089,6 +1092,7 @@ class Project:
         dest_branch=None,
         validate_certs=True,
         push_options=None,
+        patchset_description=None,
     ):
         """Uploads the named branch for code review."""
         if branch is None:
@@ -1171,6 +1175,10 @@ class Project:
             opts += ["wip"]
         if ready:
             opts += ["ready"]
+        if patchset_description:
+            opts += [
+                f"m={self._encode_patchset_description(patchset_description)}"
+            ]
         if opts:
             ref_spec = ref_spec + "%" + ",".join(opts)
         cmd.append(ref_spec)
@@ -1183,6 +1191,30 @@ class Project:
                 R_PUB + branch.name, R_HEADS + branch.name, message=msg
             )
 
+    @staticmethod
+    def _encode_patchset_description(original):
+        """Applies percent-encoding for strings sent as patchset description.
+
+        The encoding used is based on but stricter than URL encoding (Section
+        2.1 of RFC 3986). The only non-escaped characters are alphanumerics, and
+        'SPACE' (U+0020) can be represented as 'LOW LINE' (U+005F) or
+        'PLUS SIGN' (U+002B).
+
+        For more information, see the Gerrit docs here:
+        https://gerrit-review.googlesource.com/Documentation/user-upload.html#patch_set_description
+        """
+        SAFE = {ord(x) for x in string.ascii_letters + string.digits}
+
+        def _enc(b):
+            if b in SAFE:
+                return chr(b)
+            elif b == ord(" "):
+                return "_"
+            else:
+                return f"%{b:02x}"
+
+        return "".join(_enc(x) for x in original.encode("utf-8"))
+
     def _ExtractArchive(self, tarpath, path=None):
         """Extract the given tar on its current location
 
@@ -1277,7 +1309,20 @@ class Project:
         if is_new:
             self._InitGitDir(force_sync=force_sync, quiet=quiet)
         else:
-            self._UpdateHooks(quiet=quiet)
+            try:
+                # At this point, it's possible that gitdir points to an old
+                # objdir (e.g. name changed, but objdir exists). Check
+                # references to ensure that's not the case. See
+                # https://issues.gerritcodereview.com/40013418 for more
+                # details.
+                self._CheckDirReference(self.objdir, self.gitdir)
+
+                self._UpdateHooks(quiet=quiet)
+            except GitError as e:
+                if not force_sync:
+                    raise e
+                # Let _InitGitDir fix the issue, force_sync is always True here.
+                self._InitGitDir(force_sync=True, quiet=quiet)
         self._InitRemote()
 
         if self.UseAlternates:
@@ -1470,6 +1515,7 @@ class Project:
         self,
         syncbuf,
         force_sync=False,
+        force_checkout=False,
         submodules=False,
         errors=None,
         verbose=False,
@@ -1557,7 +1603,7 @@ class Project:
                     syncbuf.info(self, "discarding %d commits", len(lost))
 
             try:
-                self._Checkout(revid, quiet=True)
+                self._Checkout(revid, force_checkout=force_checkout, quiet=True)
                 if submodules:
                     self._SyncSubmodules(quiet=True)
             except GitError as e:
@@ -1623,9 +1669,9 @@ class Project:
             elif pub == head:
                 # All published commits are merged, and thus we are a
                 # strict subset.  We can fast-forward safely.
-                syncbuf.later1(self, _doff)
+                syncbuf.later1(self, _doff, not verbose)
                 if submodules:
-                    syncbuf.later1(self, _dosubmodules)
+                    syncbuf.later1(self, _dosubmodules, not verbose)
                 return
 
         # Examine the local commits not in the remote.  Find the
@@ -1684,10 +1730,10 @@ class Project:
             def _dorebase():
                 self._Rebase(upstream="%s^1" % last_mine, onto=revid)
 
-            syncbuf.later2(self, _dorebase)
+            syncbuf.later2(self, _dorebase, not verbose)
             if submodules:
-                syncbuf.later2(self, _dosubmodules)
-            syncbuf.later2(self, _docopyandlink)
+                syncbuf.later2(self, _dosubmodules, not verbose)
+            syncbuf.later2(self, _docopyandlink, not verbose)
         elif local_changes:
             try:
                 self._ResetHard(revid)
@@ -1698,9 +1744,9 @@ class Project:
                 fail(e)
                 return
         else:
-            syncbuf.later1(self, _doff)
+            syncbuf.later1(self, _doff, not verbose)
             if submodules:
-                syncbuf.later1(self, _dosubmodules)
+                syncbuf.later1(self, _dosubmodules, not verbose)
 
     def AddCopyFile(self, src, dest, topdir):
         """Mark |src| for copying to |dest| (relative to |topdir|).
@@ -1767,11 +1813,11 @@ class Project:
                 )
             else:
                 msg = (
-                    "error: %s: Cannot remove project: uncommitted"
+                    "error: %s: Cannot remove project: uncommitted "
                     "changes are present.\n" % self.RelPath(local=False)
                 )
                 logger.error(msg)
-                raise DeleteDirtyWorktreeError(msg, project=self)
+                raise DeleteDirtyWorktreeError(msg, project=self.name)
 
         if verbose:
             print(f"{self.RelPath(local=False)}: Deleting obsolete checkout.")
@@ -1835,7 +1881,7 @@ class Project:
                     platform_utils.remove(path)
                 except OSError as e:
                     if e.errno != errno.ENOENT:
-                        logger.error("error: %s: Failed to remove: %s", path, e)
+                        logger.warning("%s: Failed to remove: %s", path, e)
                         failed = True
                         errors.append(e)
             dirs[:] = [
@@ -1854,7 +1900,7 @@ class Project:
                     platform_utils.remove(d)
                 except OSError as e:
                     if e.errno != errno.ENOENT:
-                        logger.error("error: %s: Failed to remove: %s", d, e)
+                        logger.warning("%s: Failed to remove: %s", d, e)
                         failed = True
                         errors.append(e)
             elif not platform_utils.listdir(d):
@@ -1862,18 +1908,30 @@ class Project:
                     platform_utils.rmdir(d)
                 except OSError as e:
                     if e.errno != errno.ENOENT:
-                        logger.error("error: %s: Failed to remove: %s", d, e)
+                        logger.warning("%s: Failed to remove: %s", d, e)
                         failed = True
                         errors.append(e)
         if failed:
-            logger.error(
-                "error: %s: Failed to delete obsolete checkout.",
-                self.RelPath(local=False),
+            rename_path = (
+                f"{self.worktree}_repo_to_be_deleted_{int(time.time())}"
             )
-            logger.error(
-                "       Remove manually, then run `repo sync -l`.",
-            )
-            raise DeleteWorktreeError(aggregate_errors=errors)
+            try:
+                platform_utils.rename(self.worktree, rename_path)
+                logger.warning(
+                    "warning: renamed %s to %s. You can delete it, but you "
+                    "might need elevated permissions (e.g. root)",
+                    self.worktree,
+                    rename_path,
+                )
+                # Rename successful! Clear the errors.
+                errors = []
+            except OSError:
+                logger.error(
+                    "%s: Failed to delete obsolete checkout.\n",
+                    "       Remove manually, then run `repo sync -l`.",
+                    self.RelPath(local=False),
+                )
+                raise DeleteWorktreeError(aggregate_errors=errors)
 
         # Try deleting parent dirs if they are empty.
         path = self.worktree
@@ -2800,10 +2858,12 @@ class Project:
         except OSError:
             return False
 
-    def _Checkout(self, rev, quiet=False):
+    def _Checkout(self, rev, force_checkout=False, quiet=False):
         cmd = ["checkout"]
         if quiet:
             cmd.append("-q")
+        if force_checkout:
+            cmd.append("-f")
         cmd.append(rev)
         cmd.append("--")
         if GitCommand(self, cmd).Wait() != 0:
@@ -2870,10 +2930,12 @@ class Project:
         if GitCommand(self, cmd).Wait() != 0:
             raise GitError(f"{self.name} rebase {upstream} ", project=self.name)
 
-    def _FastForward(self, head, ffonly=False):
+    def _FastForward(self, head, ffonly=False, quiet=True):
         cmd = ["merge", "--no-stat", head]
         if ffonly:
             cmd.append("--ff-only")
+        if quiet:
+            cmd.append("-q")
         if GitCommand(self, cmd).Wait() != 0:
             raise GitError(f"{self.name} merge {head} ", project=self.name)
 
@@ -3279,7 +3341,7 @@ class Project:
         if not platform_utils.islink(dotgit) and platform_utils.isdir(dotgit):
             self._MigrateOldWorkTreeGitDir(dotgit, project=self.name)
 
-        init_dotgit = not os.path.exists(dotgit)
+        init_dotgit = not os.path.lexists(dotgit)
         if self.use_git_worktrees:
             if init_dotgit:
                 self._InitGitWorktree()
@@ -3746,17 +3808,20 @@ class _Failure:
 
 
 class _Later:
-    def __init__(self, project, action):
+    def __init__(self, project, action, quiet):
         self.project = project
         self.action = action
+        self.quiet = quiet
 
     def Run(self, syncbuf):
         out = syncbuf.out
-        out.project("project %s/", self.project.RelPath(local=False))
-        out.nl()
+        if not self.quiet:
+            out.project("project %s/", self.project.RelPath(local=False))
+            out.nl()
         try:
             self.action()
-            out.nl()
+            if not self.quiet:
+                out.nl()
             return True
         except GitError:
             out.nl()
@@ -3792,11 +3857,11 @@ class SyncBuffer:
         self._failures.append(_Failure(project, err))
         self._MarkUnclean()
 
-    def later1(self, project, what):
-        self._later_queue1.append(_Later(project, what))
+    def later1(self, project, what, quiet):
+        self._later_queue1.append(_Later(project, what, quiet))
 
-    def later2(self, project, what):
-        self._later_queue2.append(_Later(project, what))
+    def later2(self, project, what, quiet):
+        self._later_queue2.append(_Later(project, what, quiet))
 
     def Finish(self):
         self._PrintMessages()

repo

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 # Copyright (C) 2008 The Android Open Source Project
 #
@@ -79,7 +79,7 @@ def check_python_version():
     major = ver.major
     minor = ver.minor
 
-    # Try to re-exec the version specific Python 3 if needed.
+    # Try to re-exec the version specific Python if needed.
     if (major, minor) < MIN_PYTHON_VERSION_SOFT:
         # Python makes releases ~once a year, so try our min version +10 to help
         # bridge the gap.  This is the fallback anyways so perf isn't critical.
@@ -96,36 +96,10 @@ def check_python_version():
                 break
             reexec(f"python{min_major}.{min_minor - inc}")
 
-        # Try the generic Python 3 wrapper, but only if it's new enough.  If it
-        # isn't, we want to just give up below and make the user resolve things.
-        try:
-            proc = subprocess.Popen(
-                [
-                    "python3",
-                    "-c",
-                    "import sys; "
-                    "print(sys.version_info.major, sys.version_info.minor)",
-                ],
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-            )
-            (output, _) = proc.communicate()
-            python3_ver = tuple(int(x) for x in output.decode("utf-8").split())
-        except (OSError, subprocess.CalledProcessError):
-            python3_ver = None
-
-        # If the python3 version looks like it's new enough, give it a try.
-        if (
-            python3_ver
-            and python3_ver >= MIN_PYTHON_VERSION_HARD
-            and python3_ver != (major, minor)
-        ):
-            reexec("python3")
-
         # We're still here, so diagnose things for the user.
         if (major, minor) < MIN_PYTHON_VERSION_HARD:
             print(
-                "repo: error: Python 3 version is too old; "
+                "repo: error: Python version is too old; "
                 "Please use Python {}.{} or newer.".format(
                     *MIN_PYTHON_VERSION_HARD
                 ),
@@ -150,7 +124,7 @@ if not REPO_REV:
 BUG_URL = "https://issues.gerritcodereview.com/issues/new?component=1370071"
 
 # increment this whenever we make important changes to this script
-VERSION = (2, 40)
+VERSION = (2, 45)
 
 # increment this if the MAINTAINER_KEYS block is modified
 KEYRING_VERSION = (2, 3)
@@ -236,9 +210,8 @@ GIT = "git"  # our git command
 # NB: The version of git that the repo launcher requires may be much older than
 # the version of git that the main repo source tree requires.  Keeping this at
 # an older version also makes it easier for users to upgrade/rollback as needed.
-#
-# git-1.7 is in (EOL) Ubuntu Precise.
-MIN_GIT_VERSION = (1, 7, 2)  # minimum supported git version
+# See requirements.json for versions.
+MIN_GIT_VERSION = (1, 7, 9)  # minimum supported git version
 repodir = ".repo"  # name of repo's private directory
 S_repo = "repo"  # special repo repository
 S_manifests = "manifests"  # special manifest repository
@@ -1245,7 +1218,6 @@ class Requirements:
             with open(path, "rb") as f:
                 data = f.read()
         except OSError:
-            # NB: EnvironmentError is used for Python 2 & 3 compatibility.
             # If we couldn't open the file, assume it's an old source tree.
             return None
 
@@ -1364,13 +1336,9 @@ def _Version():
     print(f"git {ParseGitVersion().full}")
     print(f"Python {sys.version}")
     uname = platform.uname()
-    if sys.version_info.major < 3:
-        # Python 3 returns a named tuple, but Python 2 is simpler.
-        print(uname)
-    else:
-        print(f"OS {uname.system} {uname.release} ({uname.version})")
-        processor = uname.processor if uname.processor else "unknown"
-        print(f"CPU {uname.machine} ({processor})")
+    print(f"OS {uname.system} {uname.release} ({uname.version})")
+    processor = uname.processor if uname.processor else "unknown"
+    print(f"CPU {uname.machine} ({processor})")
     print("Bug reports:", BUG_URL)
     sys.exit(0)
 

requirements.json

@@ -46,12 +46,16 @@
 
   # Supported git versions.
   #
-  # git-1.7.2 is in Debian Squeeze.
   # git-1.7.9 is in Ubuntu Precise.
-  # git-1.9.1 is in Ubuntu Trusty.
   # git-1.7.10 is in Debian Wheezy.
+  # git-1.9.1 is in Ubuntu Trusty.
+  # git-2.1.4 is in Debian Jessie.
+  # git-2.7.4 is in Ubuntu Xenial.
+  # git-2.11.0 is in Debian Stretch.
+  # git-2.17.0 is in Ubuntu Bionic.
+  # git-2.20.1 is in Debian Buster.
   "git": {
-    "hard": [1, 7, 2],
-    "soft": [1, 9, 1]
+    "hard": [1, 7, 9],
+    "soft": [2, 7, 4]
   }
 }

ssh.py

@@ -57,8 +57,12 @@ def version():
     except FileNotFoundError:
         print("fatal: ssh not installed", file=sys.stderr)
         sys.exit(1)
-    except subprocess.CalledProcessError:
-        print("fatal: unable to detect ssh version", file=sys.stderr)
+    except subprocess.CalledProcessError as e:
+        print(
+            "fatal: unable to detect ssh version"
+            f" (code={e.returncode}, output={e.stdout})",
+            file=sys.stderr,
+        )
         sys.exit(1)
 
 

subcmds/sync.py

@@ -21,6 +21,7 @@ import multiprocessing
 import netrc
 import optparse
 import os
+from pathlib import Path
 import sys
 import tempfile
 import time
@@ -82,16 +83,54 @@ from wrapper import Wrapper
 
 _ONE_DAY_S = 24 * 60 * 60
 
-# Env var to implicitly turn auto-gc back on.  This was added to allow a user to
-# revert a change in default behavior in v2.29.9.  Remove after 2023-04-01.
-_REPO_AUTO_GC = "REPO_AUTO_GC"
-_AUTO_GC = os.environ.get(_REPO_AUTO_GC) == "1"
-
 _REPO_ALLOW_SHALLOW = os.environ.get("REPO_ALLOW_SHALLOW")
 
 logger = RepoLogger(__file__)
 
 
+def _SafeCheckoutOrder(checkouts: List[Project]) -> List[List[Project]]:
+    """Generate a sequence of checkouts that is safe to perform. The client
+    should checkout everything from n-th index before moving to n+1.
+
+    This is only useful if manifest contains nested projects.
+
+    E.g. if foo, foo/bar and foo/bar/baz are project paths, then foo needs to
+    finish before foo/bar can proceed, and foo/bar needs to finish before
+    foo/bar/baz."""
+    res = [[]]
+    current = res[0]
+
+    # depth_stack contains a current stack of parent paths.
+    depth_stack = []
+    # Checkouts are iterated in the hierarchical order. That way, it can easily
+    # be determined if the previous checkout is parent of the current checkout.
+    # We are splitting by the path separator so the final result is
+    # hierarchical, and not just lexicographical. For example, if the projects
+    # are: foo, foo/bar, foo-bar, lexicographical order produces foo, foo-bar
+    # and foo/bar, which doesn't work.
+    for checkout in sorted(checkouts, key=lambda x: x.relpath.split("/")):
+        checkout_path = Path(checkout.relpath)
+        while depth_stack:
+            try:
+                checkout_path.relative_to(depth_stack[-1])
+            except ValueError:
+                # Path.relative_to returns ValueError if paths are not relative.
+                # TODO(sokcevic): Switch to is_relative_to once min supported
+                # version is py3.9.
+                depth_stack.pop()
+            else:
+                if len(depth_stack) >= len(res):
+                    # Another depth created.
+                    res.append([])
+                break
+
+        current = res[len(depth_stack)]
+        current.append(checkout)
+        depth_stack.append(checkout_path)
+
+    return res
+
+
 class _FetchOneResult(NamedTuple):
     """_FetchOne return value.
 
@@ -243,6 +282,11 @@ directories if they have previously been linked to a different
 object directory. WARNING: This may cause data to be lost since
 refs may be removed when overwriting.
 
+The --force-checkout option can be used to force git to switch revs even if the
+index or the working tree differs from HEAD, and if there are untracked files.
+WARNING: This may cause data to be lost since uncommitted changes may be
+removed.
+
 The --force-remove-dirty option can be used to remove previously used
 projects with uncommitted changes. WARNING: This may cause data to be
 lost since uncommitted changes may be removed with projects that no longer
@@ -340,6 +384,14 @@ later is required to fix a server side protocol bug.
             "point to a different object directory. WARNING: this "
             "may cause loss of data",
         )
+        p.add_option(
+            "--force-checkout",
+            dest="force_checkout",
+            action="store_true",
+            help="force checkout even if it results in throwing away "
+            "uncommitted modifications. "
+            "WARNING: this may cause loss of data",
+        )
         p.add_option(
             "--force-remove-dirty",
             dest="force_remove_dirty",
@@ -618,7 +670,7 @@ later is required to fix a server side protocol bug.
 
             if not use_super:
                 continue
-            m.superproject.SetQuiet(opt.quiet)
+            m.superproject.SetQuiet(not opt.verbose)
             print_messages = git_superproject.PrintMessages(
                 opt.use_superproject, m
             )
@@ -956,12 +1008,17 @@ later is required to fix a server side protocol bug.
 
         return _FetchMainResult(all_projects)
 
-    def _CheckoutOne(self, detach_head, force_sync, verbose, project):
+    def _CheckoutOne(
+        self, detach_head, force_sync, force_checkout, verbose, project
+    ):
         """Checkout work tree for one project
 
         Args:
             detach_head: Whether to leave a detached HEAD.
-            force_sync: Force checking out of the repo.
+            force_sync: Force checking out of .git directory (e.g. overwrite
+            existing git directory that was previously linked to a different
+            object directory).
+            force_checkout: Force checking out of the repo content.
             verbose: Whether to show verbose messages.
             project: Project object for the project to checkout.
 
@@ -976,7 +1033,11 @@ later is required to fix a server side protocol bug.
         errors = []
         try:
             project.Sync_LocalHalf(
-                syncbuf, force_sync=force_sync, errors=errors, verbose=verbose
+                syncbuf,
+                force_sync=force_sync,
+                force_checkout=force_checkout,
+                errors=errors,
+                verbose=verbose,
             )
             success = syncbuf.Finish()
         except GitError as e:
@@ -1040,15 +1101,22 @@ later is required to fix a server side protocol bug.
                 pm.update(msg=project.name)
             return ret
 
-        proc_res = self.ExecuteInParallel(
-            opt.jobs_checkout,
-            functools.partial(
-                self._CheckoutOne, opt.detach_head, opt.force_sync, opt.verbose
-            ),
-            all_projects,
-            callback=_ProcessResults,
-            output=Progress("Checking out", len(all_projects), quiet=opt.quiet),
-        )
+        for projects in _SafeCheckoutOrder(all_projects):
+            proc_res = self.ExecuteInParallel(
+                opt.jobs_checkout,
+                functools.partial(
+                    self._CheckoutOne,
+                    opt.detach_head,
+                    opt.force_sync,
+                    opt.force_checkout,
+                    opt.verbose,
+                ),
+                projects,
+                callback=_ProcessResults,
+                output=Progress(
+                    "Checking out", len(all_projects), quiet=opt.quiet
+                ),
+            )
 
         self._local_sync_state.Save()
         return proc_res and not err_results
@@ -1501,7 +1569,7 @@ later is required to fix a server side protocol bug.
             buf = TeeStringIO(sys.stdout)
             try:
                 result = mp.Sync_NetworkHalf(
-                    quiet=opt.quiet,
+                    quiet=not opt.verbose,
                     output_redir=buf,
                     verbose=opt.verbose,
                     current_branch_only=self._GetCurrentBranchOnly(
@@ -1544,9 +1612,7 @@ later is required to fix a server side protocol bug.
                 mp, event_log.TASK_SYNC_LOCAL, start, time.time(), clean
             )
             if not clean:
-                raise UpdateManifestError(
-                    aggregate_errors=errors, project=mp.name
-                )
+                raise UpdateManifestError(aggregate_errors=errors)
             self._ReloadManifest(manifest_name, mp.manifest)
 
     def ValidateOptions(self, opt, args):
@@ -1577,16 +1643,6 @@ later is required to fix a server side protocol bug.
         if opt.prune is None:
             opt.prune = True
 
-        if opt.auto_gc is None and _AUTO_GC:
-            logger.error(
-                "Will run `git gc --auto` because %s is set. %s is deprecated "
-                "and will be removed in a future release.  Use `--auto-gc` "
-                "instead.",
-                _REPO_AUTO_GC,
-                _REPO_AUTO_GC,
-            )
-            opt.auto_gc = True
-
     def _ValidateOptionsWithManifest(self, opt, mp):
         """Like ValidateOptions, but after we've updated the manifest.
 
@@ -1630,7 +1686,7 @@ later is required to fix a server side protocol bug.
         errors = []
         try:
             self._ExecuteHelper(opt, args, errors)
-        except RepoExitError:
+        except (RepoExitError, RepoChangedException):
             raise
         except (KeyboardInterrupt, Exception) as e:
             raise RepoUnhandledExceptionError(e, aggregate_errors=errors)

subcmds/upload.py

@@ -244,6 +244,12 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
             default=[],
             help="add a label when uploading",
         )
+        p.add_option(
+            "--pd",
+            "--patchset-description",
+            dest="patchset_description",
+            help="description for patchset",
+        )
         p.add_option(
             "--re",
             "--reviewers",
@@ -655,6 +661,7 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
             dest_branch=destination,
             validate_certs=opt.validate_certs,
             push_options=opt.push_options,
+            patchset_description=opt.patchset_description,
         )
 
         branch.uploaded = True

tests/conftest.py

@@ -72,3 +72,12 @@ def tmp_home_dir(monkeypatch, tmp_path_factory):
     the function scope.
     """
     return _set_home(monkeypatch, tmp_path_factory.mktemp("home"))
+
+
+@pytest.fixture(autouse=True)
+def setup_user_identity(monkeysession, scope="session"):
+    """Set env variables for author and committer name and email."""
+    monkeysession.setenv("GIT_AUTHOR_NAME", "Foo Bar")
+    monkeysession.setenv("GIT_COMMITTER_NAME", "Foo Bar")
+    monkeysession.setenv("GIT_AUTHOR_EMAIL", "foo@bar.baz")
+    monkeysession.setenv("GIT_COMMITTER_EMAIL", "foo@bar.baz")

tests/test_manifest_xml.py

@@ -1139,6 +1139,20 @@ class NormalizeUrlTests(ManifestParseTestCase):
             "http://foo.com/bar/baz", manifest_xml.normalize_url(url)
         )
 
+        url = "http://foo.com/bar/"
+        self.assertEqual("http://foo.com/bar", manifest_xml.normalize_url(url))
+
+    def test_has_leading_slash(self):
+        """SCP-like syntax except a / comes before the : which git disallows."""
+        url = "/git@foo.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "gi/t@foo.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "git@fo/o.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
     def test_has_no_scheme(self):
         """Deal with cases where we have no scheme, but we also
         aren't dealing with the git SCP-like syntax
@@ -1146,9 +1160,15 @@ class NormalizeUrlTests(ManifestParseTestCase):
         url = "foo.com/baf/bat"
         self.assertEqual(url, manifest_xml.normalize_url(url))
 
+        url = "foo.com/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
         url = "git@foo.com/baf/bat"
         self.assertEqual(url, manifest_xml.normalize_url(url))
 
+        url = "git@foo.com/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
         url = "/file/path/here"
         self.assertEqual(url, manifest_xml.normalize_url(url))
 
@@ -1157,3 +1177,30 @@ class NormalizeUrlTests(ManifestParseTestCase):
         self.assertEqual(
             "ssh://git@foo.com/bar/baf", manifest_xml.normalize_url(url)
         )
+
+        url = "git@foo.com:bar/"
+        self.assertEqual(
+            "ssh://git@foo.com/bar", manifest_xml.normalize_url(url)
+        )
+
+    def test_remote_url_resolution(self):
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="git@github.com:org2/",
+            manifestUrl="git@github.com:org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)
+
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="ssh://git@github.com/org2/",
+            manifestUrl="git@github.com:org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)
+
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="git@github.com:org2/",
+            manifestUrl="ssh://git@github.com/org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)

tests/test_project.py

@@ -107,6 +107,16 @@ class ReviewableBranchTests(unittest.TestCase):
             self.assertTrue(rb.date)
 
 
+class ProjectTests(unittest.TestCase):
+    """Check Project behavior."""
+
+    def test_encode_patchset_description(self):
+        self.assertEqual(
+            project.Project._encode_patchset_description("abcd00!! +"),
+            "abcd00%21%21_%2b",
+        )
+
+
 class CopyLinkTestCase(unittest.TestCase):
     """TestCase for stub repo client checkouts.
 

tests/test_subcmds_sync.py

@@ -304,6 +304,57 @@ class LocalSyncState(unittest.TestCase):
         self.assertEqual(self.state.GetFetchTime(projA), 5)
 
 
+class FakeProject:
+    def __init__(self, relpath):
+        self.relpath = relpath
+
+    def __str__(self):
+        return f"project: {self.relpath}"
+
+    def __repr__(self):
+        return str(self)
+
+
+class SafeCheckoutOrder(unittest.TestCase):
+    def test_no_nested(self):
+        p_f = FakeProject("f")
+        p_foo = FakeProject("foo")
+        out = sync._SafeCheckoutOrder([p_f, p_foo])
+        self.assertEqual(out, [[p_f, p_foo]])
+
+    def test_basic_nested(self):
+        p_foo = p_foo = FakeProject("foo")
+        p_foo_bar = FakeProject("foo/bar")
+        out = sync._SafeCheckoutOrder([p_foo, p_foo_bar])
+        self.assertEqual(out, [[p_foo], [p_foo_bar]])
+
+    def test_complex_nested(self):
+        p_foo = FakeProject("foo")
+        p_foobar = FakeProject("foobar")
+        p_foo_dash_bar = FakeProject("foo-bar")
+        p_foo_bar = FakeProject("foo/bar")
+        p_foo_bar_baz_baq = FakeProject("foo/bar/baz/baq")
+        p_bar = FakeProject("bar")
+        out = sync._SafeCheckoutOrder(
+            [
+                p_foo_bar_baz_baq,
+                p_foo,
+                p_foobar,
+                p_foo_dash_bar,
+                p_foo_bar,
+                p_bar,
+            ]
+        )
+        self.assertEqual(
+            out,
+            [
+                [p_bar, p_foo, p_foo_dash_bar, p_foobar],
+                [p_foo_bar],
+                [p_foo_bar_baz_baq],
+            ],
+        )
+
+
 class GetPreciousObjectsState(unittest.TestCase):
     """Tests for _GetPreciousObjectsState."""