[WILD] CodeReviewer APPEARS

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 @@ def ForUser(cls):
 
     @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 @@ def GetSubprojectPaths(self, parent, name, path):
         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 @@ def __init__(
         # 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 @@ def fail(error: Exception):
             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 @@ def GetDerivedSubprojects(self):
             )
             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 @@ def _SyncSubmodules(self, quiet=True):
                 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 @@ def _InitWorkTree(self, force_sync=False, submodules=False):
         """
         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,34 +3452,76 @@ def _InitWorkTree(self, force_sync=False, submodules=False):
                 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.
                 cmd = ["read-tree", "--reset", "-u", "-v", HEAD]
-                if GitCommand(self, cmd).Wait() != 0:
-                    raise GitError(
-                        "Cannot initialize work tree for " + self.name,
-                        project=self.name,
-                    )
+                try:
+                    if GitCommand(self, cmd).Wait() != 0:
+                        raise GitError(
+                            "Cannot initialize work tree for " + self.name,
+                            project=self.name,
+                        )
+                except Exception as e:
+                    # Something went wrong with read-tree (perhaps fetching
+                    # missing blobs), so remove .git to avoid half initialized
+                    # workspace from which repo can't recover on its own.
+                    platform_utils.remove(dotgit)
+                    raise e
 
                 if submodules:
                     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.
@@ -3541,6 +3610,28 @@ def _MigrateOldWorkTreeGitDir(cls, dotgit, project=None):
             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 (