gc: Add tags to remote pack list

If tags are omitted from the remote pack list, they must be present in
local pack. However, local packs don't have promisor objects, meaning
that all blobs must be available locally, and therefore all missing
blobs will be downloaded during rev-list phase. Git downloads those
sequentially, by invokving fetch operation (rev-list/fetch).

Instead of downloading tags' blobs, instruct Git to include all tags in
remote rev-list operation. This change was tested with `git fsck --all`.

R=yiwzhang@google.com

Bug: b/392732561
Change-Id: Id94a40aebbe4f084c952329583d559d296db1a11
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/451422
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Tested-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>

docs/internal-fs-layout.md

@@ -141,7 +141,7 @@ Instead, you should use standard Git workflows like [git worktree] or
     (e.g. a local mirror & a public review server) while avoiding duplicating
     the content.  However, this can run into problems if different remotes use
     the same path on their respective servers.  Best to avoid that.
-*   `subprojects/`: Like `projects/`, but for git submodules.
+*   `modules/`: Like `projects/`, but for git submodules.
 *   `subproject-objects/`: Like `project-objects/`, but for git submodules.
 *   `worktrees/`: Bare checkouts of every project synced by the manifest.  The
     filesystem layout matches the `<project name=...` setting in the manifest

docs/manifest-format.md

@@ -231,26 +231,7 @@ At most one manifest-server may be specified. The url attribute
 is used to specify the URL of a manifest server, which is an
 XML RPC service.
 
-The manifest server should implement the following RPC methods:
-
-    GetApprovedManifest(branch, target)
-
-Return a manifest in which each project is pegged to a known good revision
-for the current branch and target. This is used by repo sync when the
---smart-sync option is given.
-
-The target to use is defined by environment variables TARGET_PRODUCT
-and TARGET_BUILD_VARIANT. These variables are used to create a string
-of the form $TARGET_PRODUCT-$TARGET_BUILD_VARIANT, e.g. passion-userdebug.
-If one of those variables or both are not present, the program will call
-GetApprovedManifest without the target parameter and the manifest server
-should choose a reasonable default target.
-
-    GetManifest(tag)
-
-Return a manifest in which each project is pegged to the revision at
-the specified tag. This is used by repo sync when the --smart-tag option
-is given.
+See the [smart sync documentation](./smart-sync.md) for more details.
 
 
 ### Element submanifest

docs/smart-sync.md

@@ -0,0 +1,129 @@
+# repo Smart Syncing
+
+Repo normally fetches & syncs manifests from the same URL specified during
+`repo init`, and that often fetches the latest revisions of all projects in
+the manifest.  This flow works well for tracking and developing with the
+latest code, but often it's desirable to sync to other points.  For example,
+to get a local build matching a specific release or build to reproduce bugs
+reported by other people.
+
+Repo's sync subcommand has support for fetching manifests from a server over
+an XML-RPC connection.  The local configuration and network API are defined by
+repo, but individual projects have to host their own server for the client to
+communicate with.
+
+This process is called "smart syncing" -- instead of blindly fetching the latest
+revision of all projects and getting an unknown state to develop against, the
+client passes a request to the server and is given a matching manifest that
+typically specifies specific commits for every project to fetch a known source
+state.
+
+[TOC]
+
+## Manifest Configuration
+
+The manifest specifies the server to communicate with via the
+the [`<manifest-server>` element](manifest-format.md#Element-manifest_server)
+element.  This is how the client knows what service to talk to.
+
+```xml
+  <manifest-server url="https://example.com/your/manifest/server/url" />
+```
+
+If the URL starts with `persistent-`, then the
+[`git-remote-persistent-https` helper](https://github.com/git/git/blob/HEAD/contrib/persistent-https/README)
+is used to communicate with the server.
+
+## Credentials
+
+Credentials may be specified directly in typical `username:password`
+[URI syntax](https://en.wikipedia.org/wiki/URI#Syntax) in the
+`<manifest-server>` element directly in the manifest.
+
+If they are not specified, `repo sync` has `--manifest-server-username=USERNAME`
+and `--manifest-server-password=PASSWORD` options.
+
+If those are not used, then repo will look up the host in your
+[`~/.netrc`](https://docs.python.org/3/library/netrc.html) database.
+
+When making the connection, cookies matching the host are automatically loaded
+from the cookiejar specified in
+[Git's `http.cookiefile` setting](https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpcookieFile).
+
+## Manifest Server
+
+Unfortunately, there are no public reference implementations.  Google has an
+internal one for Android, but it is written using Google's internal systems,
+so wouldn't be that helpful as a reference.
+
+That said, the XML-RPC API is pretty simple, so any standard XML-RPC server
+example would do.  Google's internal server uses Python's
+[xmlrpc.server.SimpleXMLRPCDispatcher](https://docs.python.org/3/library/xmlrpc.server.html).
+
+## Network API
+
+The manifest server should implement the following RPC methods.
+
+### GetApprovedManifest
+
+> `GetApprovedManifest(branch: str, target: Optional[str]) -> str`
+
+The meaning of `branch` and `target` is not strictly defined.  The server may
+interpret them however it wants.  The recommended interpretation is that the
+`branch` matches the manifest branch, and `target` is an identifier for your
+project that matches something users would build.
+
+See the client section below for how repo typically generates these values.
+
+The server will return a manifest or an error.  If it's an error, repo will
+show the output directly to the user to provide a limited feedback channel.
+
+If the user's request is ambiguous and could match multiple manifests, the
+server has to decide whether to pick one automatically (and silently such that
+the user won't know there were multiple matches), or return an error and force
+the user to be more specific.
+
+### GetManifest
+
+> `GetManifest(tag: str) -> str`
+
+The meaning of `tag` is not strictly defined.  Projects are encouraged to use
+a system where the tag matches a unique source state.
+
+See the client section below for how repo typically generates these values.
+
+The server will return a manifest or an error.  If it's an error, repo will
+show the output directly to the user to provide a limited feedback channel.
+
+If the user's request is ambiguous and could match multiple manifests, the
+server has to decide whether to pick one automatically (and silently such that
+the user won't know there were multiple matches), or return an error and force
+the user to be more specific.
+
+## Client Options
+
+Once repo has successfully downloaded the manifest from the server, it saves a
+copy into `.repo/manifests/smart_sync_override.xml` so users can examine it.
+The next time `repo sync` is run, this file is automatically replaced or removed
+based on the current set of options.
+
+### --smart-sync
+
+Repo will call `GetApprovedManifest(branch[, target])`.
+
+The `branch` is determined by the current manifest branch as specified by
+`--manifest-branch=BRANCH` when running `repo init`.
+
+The `target` is defined by environment variables in the order below.  If none
+of them match, then `target` is omitted.  These variables were decided as they
+match the settings Android build environments automatically setup.
+
+1.  `${SYNC_TARGET}`: If defined, the value is used directly.
+2.  `${TARGET_PRODUCT}-${TARGET_RELEASE}-${TARGET_BUILD_VARIANT}`: If these
+    variables are all defined, then they are merged with `-` and used.
+3.  `${TARGET_PRODUCT}-${TARGET_BUILD_VARIANT}`: If these variables are all
+    defined, then they are merged with `-` and used.
+
+### --smart-tag=TAG
+
+Repo will call `GetManifest(TAG)`.

git_config.py

@@ -90,6 +90,20 @@ class GitConfig:
 
     @staticmethod
     def _getUserConfig():
+        """Get the user-specific config file.
+
+        Prefers the XDG config location if available, with fallback to
+        ~/.gitconfig
+
+        This matches git behavior:
+        https://git-scm.com/docs/git-config#FILES
+        """
+        xdg_config_home = os.getenv(
+            "XDG_CONFIG_HOME", os.path.expanduser("~/.config")
+        )
+        xdg_config_file = os.path.join(xdg_config_home, "git", "config")
+        if os.path.exists(xdg_config_file):
+            return xdg_config_file
         return os.path.expanduser("~/.gitconfig")
 
     @classmethod

manifest_xml.py

@@ -2056,7 +2056,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         path = path.rstrip("/")
         name = name.rstrip("/")
         relpath = self._JoinRelpath(parent.relpath, path)
-        gitdir = os.path.join(parent.gitdir, "subprojects", "%s.git" % path)
+        subprojects = os.path.join(parent.gitdir, "subprojects", f"{path}.git")
+        modules = os.path.join(parent.gitdir, "modules", path)
+        if platform_utils.isdir(subprojects):
+            gitdir = subprojects
+        else:
+            gitdir = modules
         objdir = os.path.join(
             parent.gitdir, "subproject-objects", "%s.git" % name
         )

project.py

@@ -642,6 +642,10 @@ class Project:
         # project containing repo hooks.
         self.enabled_repo_hooks = []
 
+        # This will be updated later if the project has submodules and
+        # if they will be synced.
+        self.has_subprojects = False
+
     def RelPath(self, local=True):
         """Return the path for the project relative to a manifest.
 
@@ -1560,6 +1564,11 @@ class Project:
             return
 
         self._InitWorkTree(force_sync=force_sync, submodules=submodules)
+        # TODO(https://git-scm.com/docs/git-worktree#_bugs): Re-evaluate if
+        # submodules can be init when using worktrees once its support is
+        # complete.
+        if self.has_subprojects and not self.use_git_worktrees:
+            self._InitSubmodules()
         all_refs = self.bare_ref.all
         self.CleanPublishedCache(all_refs)
         revid = self.GetRevisionId(all_refs)
@@ -2347,6 +2356,8 @@ class Project:
             )
             result.append(subproject)
             result.extend(subproject.GetDerivedSubprojects())
+        if result:
+            self.has_subprojects = True
         return result
 
     def EnableRepositoryExtension(self, key, value="true", version=1):
@@ -2997,6 +3008,17 @@ class Project:
                 project=self.name,
             )
 
+    def _InitSubmodules(self, quiet=True):
+        """Initialize the submodules for the project."""
+        cmd = ["submodule", "init"]
+        if quiet:
+            cmd.append("-q")
+        if GitCommand(self, cmd).Wait() != 0:
+            raise GitError(
+                f"{self.name} submodule init",
+                project=self.name,
+            )
+
     def _Rebase(self, upstream, onto=None):
         cmd = ["rebase"]
         if onto is not None:
@@ -3415,6 +3437,11 @@ class Project:
         """
         dotgit = os.path.join(self.worktree, ".git")
 
+        # If bare checkout of the submodule is stored under the subproject dir,
+        # migrate it.
+        if self.parent:
+            self._MigrateOldSubmoduleDir()
+
         # If using an old layout style (a directory), migrate it.
         if not platform_utils.islink(dotgit) and platform_utils.isdir(dotgit):
             self._MigrateOldWorkTreeGitDir(dotgit, project=self.name)
@@ -3425,20 +3452,21 @@ class Project:
                 self._InitGitWorktree()
                 self._CopyAndLinkFiles()
         else:
+            # Remove old directory symbolic links for submodules.
+            if self.parent and platform_utils.islink(dotgit):
+                platform_utils.remove(dotgit)
+                init_dotgit = True
+
             if not init_dotgit:
                 # See if the project has changed.
-                if os.path.realpath(self.gitdir) != os.path.realpath(dotgit):
-                    platform_utils.remove(dotgit)
+                self._removeBadGitDirLink(dotgit)
 
             if init_dotgit or not os.path.exists(dotgit):
-                os.makedirs(self.worktree, exist_ok=True)
-                platform_utils.symlink(
-                    os.path.relpath(self.gitdir, self.worktree), dotgit
-                )
+                self._createDotGit(dotgit)
 
             if init_dotgit:
                 _lwrite(
-                    os.path.join(dotgit, HEAD), "%s\n" % self.GetRevisionId()
+                    os.path.join(self.gitdir, HEAD), f"{self.GetRevisionId()}\n"
                 )
 
                 # Finish checking out the worktree.
@@ -3460,6 +3488,40 @@ class Project:
                     self._SyncSubmodules(quiet=True)
                 self._CopyAndLinkFiles()
 
+    def _createDotGit(self, dotgit):
+        """Initialize .git path.
+
+        For submodule projects, create a '.git' file using the gitfile
+        mechanism, and for the rest, create a symbolic link.
+        """
+        os.makedirs(self.worktree, exist_ok=True)
+        if self.parent:
+            _lwrite(
+                dotgit,
+                f"gitdir: {os.path.relpath(self.gitdir, self.worktree)}\n",
+            )
+        else:
+            platform_utils.symlink(
+                os.path.relpath(self.gitdir, self.worktree), dotgit
+            )
+
+    def _removeBadGitDirLink(self, dotgit):
+        """Verify .git is initialized correctly, otherwise delete it."""
+        if self.parent and os.path.isfile(dotgit):
+            with open(dotgit) as fp:
+                setting = fp.read()
+            if not setting.startswith("gitdir:"):
+                raise GitError(
+                    f"'.git' in {self.worktree} must start with 'gitdir:'",
+                    project=self.name,
+                )
+            gitdir = setting.split(":", 1)[1].strip()
+            dotgit_path = os.path.normpath(os.path.join(self.worktree, gitdir))
+        else:
+            dotgit_path = os.path.realpath(dotgit)
+        if os.path.realpath(self.gitdir) != dotgit_path:
+            platform_utils.remove(dotgit)
+
     @classmethod
     def _MigrateOldWorkTreeGitDir(cls, dotgit, project=None):
         """Migrate the old worktree .git/ dir style to a symlink.
@@ -3548,6 +3610,28 @@ class Project:
             dotgit,
         )
 
+    def _MigrateOldSubmoduleDir(self):
+        """Move the old bare checkout in 'subprojects' to 'modules'
+        as bare checkouts of submodules are now in 'modules' dir.
+        """
+        subprojects = os.path.join(self.parent.gitdir, "subprojects")
+        if not platform_utils.isdir(subprojects):
+            return
+
+        modules = os.path.join(self.parent.gitdir, "modules")
+        old = self.gitdir
+        new = os.path.splitext(self.gitdir.replace(subprojects, modules))[0]
+
+        if all(map(platform_utils.isdir, [old, new])):
+            platform_utils.rmtree(old, ignore_errors=True)
+        else:
+            os.makedirs(modules, exist_ok=True)
+            platform_utils.rename(old, new)
+        self.gitdir = new
+        self.UpdatePaths(self.relpath, self.worktree, self.gitdir, self.objdir)
+        if platform_utils.isdir(subprojects) and not os.listdir(subprojects):
+            platform_utils.rmtree(subprojects, ignore_errors=True)
+
     def _get_symlink_error_message(self):
         if platform_utils.isWindows():
             return (

run_tests.vpython3

@@ -5,61 +5,55 @@
 # List of available wheels:
 #   https://chromium.googlesource.com/infra/infra/+/main/infra/tools/dockerbuild/wheels.md
 
-python_version: "3.8"
+python_version: "3.11"
 
 wheel: <
   name: "infra/python/wheels/pytest-py3"
-  version: "version:6.2.2"
+  version: "version:8.3.4"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/py-py2_py3"
-  version: "version:1.10.0"
+  version: "version:1.11.0"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/iniconfig-py3"
   version: "version:1.1.1"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/packaging-py3"
   version: "version:23.0"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/pluggy-py3"
-  version: "version:0.13.1"
+  version: "version:1.5.0"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/toml-py3"
   version: "version:0.10.1"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/pyparsing-py3"
   version: "version:3.0.7"
 >
 
-# Required by pytest==6.2.2
+# Required by pytest==8.3.4
 wheel: <
   name: "infra/python/wheels/attrs-py2_py3"
   version: "version:21.4.0"
 >
 
-# Required by packaging==16.8
-wheel: <
-  name: "infra/python/wheels/six-py2_py3"
-  version: "version:1.16.0"
->
-
 wheel: <
   name: "infra/python/wheels/black-py3"
   version: "version:23.1.0"

subcmds/gc.py

@@ -206,6 +206,7 @@ class Gc(Command):
                     "--objects",
                     f"--remotes={project.remote.name}",
                     "--filter=blob:none",
+                    "--tags",
                 ],
                 capture_stdout=True,
                 verify_command=True,
@@ -228,6 +229,7 @@ class Gc(Command):
                     "--indexed-objects",
                     "--not",
                     f"--remotes={project.remote.name}",
+                    "--tags",
                 ],
                 capture_stdout=True,
                 verify_command=True,

subcmds/sync.py

@@ -1502,6 +1502,7 @@ later is required to fix a server side protocol bug.
         if manifest_server.startswith("persistent-"):
             manifest_server = manifest_server[len("persistent-") :]
 
+        # Changes in behavior should update docs/smart-sync.md accordingly.
         try:
             server = xmlrpc.client.Server(manifest_server, transport=transport)
             if opt.smart_sync:
@@ -1833,7 +1834,7 @@ later is required to fix a server side protocol bug.
 
         self._fetch_times = _FetchTimes(manifest)
         self._local_sync_state = LocalSyncState(manifest)
-        if not opt.local_only and not opt.repo_upgraded:
+        if not opt.local_only:
             with multiprocessing.Manager() as manager:
                 with ssh.ProxyManager(manager) as ssh_proxy:
                     # Initialize the socket dir once in the parent.