Revert "Fix shallow clones when upstream attribute is present"

This reverts commit d9cc0a1526.

Reason for revert: AttributeError: 'Project' object has no attribute 'use_superproject'

Bug: b/427093249
Change-Id: I57b285ab21f58b040e68ec14b85425f43f0abcca
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/498641
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

git_superproject.py

@@ -190,7 +190,8 @@ class Superproject:
         message = f"{self._LogMessagePrefix()} {fmt.format(*inputs)}"
         if self._print_messages:
             print(message, file=sys.stderr)
-        self._git_event_log.ErrorEvent(message, fmt)
+        if self._git_event_log:
+            self._git_event_log.ErrorEvent(message, fmt)
 
     def _LogMessagePrefix(self):
         """Returns the prefix string to be logged in each log message"""

hooks.py

@@ -101,12 +101,11 @@ class RepoHook:
         self._abort_if_user_denies = abort_if_user_denies
 
         # Store the full path to the script for convenience.
-        if self._hooks_project:
+        self._script_fullpath = None
+        if self._hooks_project and self._hooks_project.worktree:
             self._script_fullpath = os.path.join(
                 self._hooks_project.worktree, self._hook_type + ".py"
             )
-        else:
-            self._script_fullpath = None
 
     def _GetHash(self):
         """Return a hash of the contents of the hooks directory.
@@ -443,6 +442,7 @@ class RepoHook:
         if (
             self._bypass_hooks
             or not self._hooks_project
+            or not self._script_fullpath
             or self._hook_type not in self._hooks_project.enabled_repo_hooks
         ):
             return True

progress.py

@@ -25,7 +25,10 @@ except ImportError:
 from repo_trace import IsTraceToStderr
 
 
-_TTY = sys.stderr.isatty()
+# Capture the original stderr stream. We use this exclusively for progress
+# updates to ensure we talk to the terminal even if stderr is redirected.
+_STDERR = sys.stderr
+_TTY = _STDERR.isatty()
 
 # This will erase all content in the current line (wherever the cursor is).
 # It does not move the cursor, so this is usually followed by \r to move to
@@ -133,11 +136,11 @@ class Progress:
     def _write(self, s):
         s = "\r" + s
         if self._elide:
-            col = os.get_terminal_size(sys.stderr.fileno()).columns
+            col = os.get_terminal_size(_STDERR.fileno()).columns
             if len(s) > col:
                 s = s[: col - 1] + ".."
-        sys.stderr.write(s)
-        sys.stderr.flush()
+        _STDERR.write(s)
+        _STDERR.flush()
 
     def start(self, name):
         self._active += 1
@@ -211,9 +214,9 @@ class Progress:
 
         # Erase the current line, print the message with a newline,
         # and then immediately redraw the progress bar on the new line.
-        sys.stderr.write("\r" + CSI_ERASE_LINE)
-        sys.stderr.write(msg + "\n")
-        sys.stderr.flush()
+        _STDERR.write("\r" + CSI_ERASE_LINE)
+        _STDERR.write(msg + "\n")
+        _STDERR.flush()
         self.update(inc=0)
 
     def end(self):

project.py

@@ -1539,18 +1539,14 @@ class Project:
         force_checkout=False,
         force_rebase=False,
         submodules=False,
-        errors=None,
         verbose=False,
     ):
         """Perform only the local IO portion of the sync process.
 
         Network access is not required.
         """
-        if errors is None:
-            errors = []
 
         def fail(error: Exception):
-            errors.append(error)
             syncbuf.fail(self, error)
 
         if not os.path.exists(self.gitdir):
@@ -3835,10 +3831,35 @@ class Project:
         def GetHead(self):
             """Return the ref that HEAD points to."""
             try:
-                return self.rev_parse("--symbolic-full-name", HEAD)
+                symbolic_head = self.rev_parse("--symbolic-full-name", HEAD)
+                if symbolic_head == HEAD:
+                    # Detached HEAD. Return the commit SHA instead.
+                    return self.rev_parse(HEAD)
+                return symbolic_head
             except GitError as e:
+                logger.warning(
+                    "project %s: unparseable HEAD; trying to recover.\n"
+                    "Check that HEAD ref in .git/HEAD is valid. The error "
+                    "was: %s",
+                    self._project.RelPath(local=False),
+                    e,
+                )
+
+                # Fallback to direct file reading for compatibility with broken
+                # repos, e.g. if HEAD points to an unborn branch.
                 path = self.GetDotgitPath(subpath=HEAD)
-                raise NoManifestException(path, str(e))
+                try:
+                    with open(path) as fd:
+                        line = fd.readline()
+                except OSError:
+                    raise NoManifestException(path, str(e))
+                try:
+                    line = line.decode()
+                except AttributeError:
+                    pass
+                if line.startswith("ref: "):
+                    return line[5:-1]
+                return line[:-1]
 
         def SetHead(self, ref, message=None):
             cmdv = []
@@ -4002,7 +4023,8 @@ class _Later:
             if not self.quiet:
                 out.nl()
             return True
-        except GitError:
+        except GitError as e:
+            syncbuf.fail(self.project, e)
             out.nl()
             return False
 
@@ -4018,7 +4040,12 @@ class _SyncColoring(Coloring):
 class SyncBuffer:
     def __init__(self, config, detach_head=False):
         self._messages = []
-        self._failures = []
+
+        # Failures that have not yet been printed. Cleared after printing.
+        self._pending_failures = []
+        # A persistent record of all failures during the buffer's lifetime.
+        self._all_failures = []
+
         self._later_queue1 = []
         self._later_queue2 = []
 
@@ -4033,7 +4060,9 @@ class SyncBuffer:
         self._messages.append(_InfoMessage(project, fmt % args))
 
     def fail(self, project, err=None):
-        self._failures.append(_Failure(project, err))
+        failure = _Failure(project, err)
+        self._pending_failures.append(failure)
+        self._all_failures.append(failure)
         self._MarkUnclean()
 
     def later1(self, project, what, quiet):
@@ -4053,6 +4082,11 @@ class SyncBuffer:
         self.recent_clean = True
         return recent_clean
 
+    @property
+    def errors(self):
+        """Returns a list of all exceptions accumulated in the buffer."""
+        return [f.why for f in self._all_failures if f.why]
+
     def _MarkUnclean(self):
         self.clean = False
         self.recent_clean = False
@@ -4071,18 +4105,18 @@ class SyncBuffer:
         return True
 
     def _PrintMessages(self):
-        if self._messages or self._failures:
+        if self._messages or self._pending_failures:
             if os.isatty(2):
                 self.out.write(progress.CSI_ERASE_LINE)
             self.out.write("\r")
 
         for m in self._messages:
             m.Print(self)
-        for m in self._failures:
+        for m in self._pending_failures:
             m.Print(self)
 
         self._messages = []
-        self._failures = []
+        self._pending_failures = []
 
 
 class MetaProject(Project):

subcmds/sync.py

@@ -204,14 +204,13 @@ class _SyncResult(NamedTuple):
       relpath (str): The project's relative path from the repo client top.
       remote_fetched (bool): True if the remote was actually queried.
       fetch_success (bool): True if the fetch operation was successful.
-      fetch_error (Optional[Exception]): The Exception from a failed fetch,
-          or None.
+      fetch_errors (List[Exception]): The Exceptions from a failed fetch.
       fetch_start (Optional[float]): The time.time() when fetch started.
       fetch_finish (Optional[float]): The time.time() when fetch finished.
       checkout_success (bool): True if the checkout operation was
           successful.
-      checkout_error (Optional[Exception]): The Exception from a failed
-          checkout, or None.
+      checkout_errors (List[Exception]): The Exceptions from a failed
+          checkout.
       checkout_start (Optional[float]): The time.time() when checkout
           started.
       checkout_finish (Optional[float]): The time.time() when checkout
@@ -224,12 +223,12 @@ class _SyncResult(NamedTuple):
 
     remote_fetched: bool
     fetch_success: bool
-    fetch_error: Optional[Exception]
+    fetch_errors: List[Exception]
     fetch_start: Optional[float]
     fetch_finish: Optional[float]
 
     checkout_success: bool
-    checkout_error: Optional[Exception]
+    checkout_errors: List[Exception]
     checkout_start: Optional[float]
     checkout_finish: Optional[float]
 
@@ -1092,10 +1091,10 @@ later is required to fix a server side protocol bug.
                 force_sync=force_sync,
                 force_checkout=force_checkout,
                 force_rebase=force_rebase,
-                errors=errors,
                 verbose=verbose,
             )
             success = syncbuf.Finish()
+            errors.extend(syncbuf.errors)
         except KeyboardInterrupt:
             logger.error("Keyboard interrupt while processing %s", project.name)
         except GitError as e:
@@ -1753,10 +1752,10 @@ later is required to fix a server side protocol bug.
             mp.Sync_LocalHalf(
                 syncbuf,
                 submodules=mp.manifest.HasSubmodules,
-                errors=errors,
                 verbose=opt.verbose,
             )
             clean = syncbuf.Finish()
+            errors.extend(syncbuf.errors)
             self.event_log.AddSync(
                 mp, event_log.TASK_SYNC_LOCAL, start, time.time(), clean
             )
@@ -2210,7 +2209,7 @@ later is required to fix a server side protocol bug.
         """Syncs a single project for interleaved sync."""
         fetch_success = False
         remote_fetched = False
-        fetch_error = None
+        fetch_errors = []
         fetch_start = None
         fetch_finish = None
         network_output = ""
@@ -2243,16 +2242,17 @@ later is required to fix a server side protocol bug.
                 )
                 fetch_success = sync_result.success
                 remote_fetched = sync_result.remote_fetched
-                fetch_error = sync_result.error
+                if sync_result.error:
+                    fetch_errors.append(sync_result.error)
             except KeyboardInterrupt:
                 logger.error(
                     "Keyboard interrupt while processing %s", project.name
                 )
             except GitError as e:
-                fetch_error = e
+                fetch_errors.append(e)
                 logger.error("error.GitError: Cannot fetch %s", e)
             except Exception as e:
-                fetch_error = e
+                fetch_errors.append(e)
                 logger.error(
                     "error: Cannot fetch %s (%s: %s)",
                     project.name,
@@ -2264,56 +2264,58 @@ later is required to fix a server side protocol bug.
                 network_output = network_output_capture.getvalue()
 
         checkout_success = False
-        checkout_error = None
+        checkout_errors = []
         checkout_start = None
         checkout_finish = None
         checkout_stderr = ""
 
-        if fetch_success and not opt.network_only:
-            checkout_start = time.time()
-            stderr_capture = io.StringIO()
-            try:
-                with contextlib.redirect_stderr(stderr_capture):
-                    syncbuf = SyncBuffer(
-                        project.manifest.manifestProject.config,
-                        detach_head=opt.detach_head,
-                    )
-                    local_half_errors = []
-                    project.Sync_LocalHalf(
-                        syncbuf,
-                        force_sync=opt.force_sync,
-                        force_checkout=opt.force_checkout,
-                        force_rebase=opt.rebase,
-                        errors=local_half_errors,
-                        verbose=opt.verbose,
-                    )
-                    checkout_success = syncbuf.Finish()
-                    if local_half_errors:
-                        checkout_error = SyncError(
-                            aggregate_errors=local_half_errors
+        if fetch_success:
+            # We skip checkout if it's network-only or if the project has no
+            # working tree (e.g., a mirror).
+            if opt.network_only or not project.worktree:
+                checkout_success = True
+            else:
+                # This is a normal project that needs a checkout.
+                checkout_start = time.time()
+                stderr_capture = io.StringIO()
+                try:
+                    with contextlib.redirect_stderr(stderr_capture):
+                        syncbuf = SyncBuffer(
+                            project.manifest.manifestProject.config,
+                            detach_head=opt.detach_head,
                         )
-            except KeyboardInterrupt:
-                logger.error(
-                    "Keyboard interrupt while processing %s", project.name
-                )
-            except GitError as e:
-                checkout_error = e
-                logger.error(
-                    "error.GitError: Cannot checkout %s: %s", project.name, e
-                )
-            except Exception as e:
-                checkout_error = e
-                logger.error(
-                    "error: Cannot checkout %s: %s: %s",
-                    project.name,
-                    type(e).__name__,
-                    e,
-                )
-            finally:
-                checkout_finish = time.time()
-                checkout_stderr = stderr_capture.getvalue()
-        elif fetch_success:
-            checkout_success = True
+                        project.Sync_LocalHalf(
+                            syncbuf,
+                            force_sync=opt.force_sync,
+                            force_checkout=opt.force_checkout,
+                            force_rebase=opt.rebase,
+                            verbose=opt.verbose,
+                        )
+                        checkout_success = syncbuf.Finish()
+                        if syncbuf.errors:
+                            checkout_errors.extend(syncbuf.errors)
+                except KeyboardInterrupt:
+                    logger.error(
+                        "Keyboard interrupt while processing %s", project.name
+                    )
+                except GitError as e:
+                    checkout_errors.append(e)
+                    logger.error(
+                        "error.GitError: Cannot checkout %s: %s",
+                        project.name,
+                        e,
+                    )
+                except Exception as e:
+                    checkout_errors.append(e)
+                    logger.error(
+                        "error: Cannot checkout %s: %s: %s",
+                        project.name,
+                        type(e).__name__,
+                        e,
+                    )
+                finally:
+                    checkout_finish = time.time()
+                    checkout_stderr = stderr_capture.getvalue()
 
         # Consolidate all captured output.
         captured_parts = []
@@ -2329,8 +2331,8 @@ later is required to fix a server side protocol bug.
             fetch_success=fetch_success,
             remote_fetched=remote_fetched,
             checkout_success=checkout_success,
-            fetch_error=fetch_error,
-            checkout_error=checkout_error,
+            fetch_errors=fetch_errors,
+            checkout_errors=checkout_errors,
             stderr_text=stderr_text.strip(),
             fetch_start=fetch_start,
             fetch_finish=fetch_finish,
@@ -2376,7 +2378,7 @@ later is required to fix a server side protocol bug.
 
     def _ProcessSyncInterleavedResults(
         self,
-        synced_relpaths: Set[str],
+        finished_relpaths: Set[str],
         err_event: _threading.Event,
         errors: List[Exception],
         opt: optparse.Values,
@@ -2392,7 +2394,8 @@ later is required to fix a server side protocol bug.
                 pm.update()
                 project = projects[result.project_index]
 
-                if opt.verbose and result.stderr_text:
+                success = result.fetch_success and result.checkout_success
+                if result.stderr_text and (opt.verbose or not success):
                     pm.display_message(result.stderr_text)
 
                 if result.fetch_start:
@@ -2419,19 +2422,19 @@ later is required to fix a server side protocol bug.
                         result.checkout_success,
                     )
 
-                if result.fetch_success and result.checkout_success:
-                    synced_relpaths.add(result.relpath)
-                else:
+                finished_relpaths.add(result.relpath)
+
+                if not success:
                     ret = False
                     err_event.set()
-                    if result.fetch_error:
-                        errors.append(result.fetch_error)
+                    if result.fetch_errors:
+                        errors.extend(result.fetch_errors)
                         self._interleaved_err_network = True
                         self._interleaved_err_network_results.append(
                             result.relpath
                         )
-                    if result.checkout_error:
-                        errors.append(result.checkout_error)
+                    if result.checkout_errors:
+                        errors.extend(result.checkout_errors)
                         self._interleaved_err_checkout = True
                         self._interleaved_err_checkout_results.append(
                             result.relpath
@@ -2473,7 +2476,7 @@ later is required to fix a server side protocol bug.
         self._interleaved_err_checkout_results = []
 
         err_event = multiprocessing.Event()
-        synced_relpaths = set()
+        finished_relpaths = set()
         project_list = list(all_projects)
         pm = Progress(
             "Syncing",
@@ -2507,7 +2510,7 @@ later is required to fix a server side protocol bug.
                         projects_to_sync = [
                             p
                             for p in project_list
-                            if p.relpath not in synced_relpaths
+                            if p.relpath not in finished_relpaths
                         ]
                         if not projects_to_sync:
                             break
@@ -2524,12 +2527,6 @@ later is required to fix a server side protocol bug.
                                 stalled_projects_str,
                             )
                             err_event.set()
-
-                            # Include these in the final error report.
-                            self._interleaved_err_checkout = True
-                            self._interleaved_err_checkout_results.extend(
-                                list(pending_relpaths)
-                            )
                             break
                         previously_pending_relpaths = pending_relpaths
 
@@ -2564,7 +2561,7 @@ later is required to fix a server side protocol bug.
                             jobs = max(1, min(opt.jobs, len(work_items)))
                             callback = functools.partial(
                                 self._ProcessSyncInterleavedResults,
-                                synced_relpaths,
+                                finished_relpaths,
                                 err_event,
                                 errors,
                                 opt,
@@ -2576,6 +2573,7 @@ later is required to fix a server side protocol bug.
                                 callback=callback,
                                 output=pm,
                                 chunksize=1,
+                                initializer=self.InitWorker,
                             ):
                                 err_event.set()
 

tests/test_subcmds_sync.py

@@ -309,6 +309,7 @@ class FakeProject:
         self.relpath = relpath
         self.name = name or relpath
         self.objdir = objdir or relpath
+        self.worktree = relpath
 
         self.use_git_worktrees = False
         self.UseAlternates = False
@@ -800,6 +801,7 @@ class InterleavedSyncTest(unittest.TestCase):
         with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
             mock_sync_buf_instance = mock.MagicMock()
             mock_sync_buf_instance.Finish.return_value = True
+            mock_sync_buf_instance.errors = []
             mock_sync_buffer.return_value = mock_sync_buf_instance
 
             result_obj = self.cmd._SyncProjectList(opt, [0])
@@ -808,8 +810,8 @@ class InterleavedSyncTest(unittest.TestCase):
             result = result_obj.results[0]
             self.assertTrue(result.fetch_success)
             self.assertTrue(result.checkout_success)
-            self.assertIsNone(result.fetch_error)
-            self.assertIsNone(result.checkout_error)
+            self.assertEqual(result.fetch_errors, [])
+            self.assertEqual(result.checkout_errors, [])
             project.Sync_NetworkHalf.assert_called_once()
             project.Sync_LocalHalf.assert_called_once()
 
@@ -831,8 +833,27 @@ class InterleavedSyncTest(unittest.TestCase):
 
         self.assertFalse(result.fetch_success)
         self.assertFalse(result.checkout_success)
-        self.assertEqual(result.fetch_error, fetch_error)
-        self.assertIsNone(result.checkout_error)
+        self.assertEqual(result.fetch_errors, [fetch_error])
+        self.assertEqual(result.checkout_errors, [])
+        project.Sync_NetworkHalf.assert_called_once()
+        project.Sync_LocalHalf.assert_not_called()
+
+    def test_worker_no_worktree(self):
+        """Test interleaved sync does not checkout with no worktree."""
+        opt = self._get_opts()
+        project = self.projA
+        project.worktree = None
+        project.Sync_NetworkHalf = mock.Mock(
+            return_value=SyncNetworkHalfResult(error=None, remote_fetched=True)
+        )
+        project.Sync_LocalHalf = mock.Mock()
+        self.mock_context["projects"] = [project]
+
+        result_obj = self.cmd._SyncProjectList(opt, [0])
+        result = result_obj.results[0]
+
+        self.assertTrue(result.fetch_success)
+        self.assertTrue(result.checkout_success)
         project.Sync_NetworkHalf.assert_called_once()
         project.Sync_LocalHalf.assert_not_called()
 
@@ -850,7 +871,7 @@ class InterleavedSyncTest(unittest.TestCase):
 
         self.assertFalse(result.fetch_success)
         self.assertFalse(result.checkout_success)
-        self.assertEqual(result.fetch_error, fetch_error)
+        self.assertEqual(result.fetch_errors, [fetch_error])
         project.Sync_NetworkHalf.assert_called_once()
         project.Sync_LocalHalf.assert_not_called()
 
@@ -872,8 +893,8 @@ class InterleavedSyncTest(unittest.TestCase):
 
             self.assertTrue(result.fetch_success)
             self.assertFalse(result.checkout_success)
-            self.assertIsNone(result.fetch_error)
-            self.assertEqual(result.checkout_error, checkout_error)
+            self.assertEqual(result.fetch_errors, [])
+            self.assertEqual(result.checkout_errors, [checkout_error])
             project.Sync_NetworkHalf.assert_called_once()
             project.Sync_LocalHalf.assert_called_once()
 
@@ -889,6 +910,7 @@ class InterleavedSyncTest(unittest.TestCase):
         with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
             mock_sync_buf_instance = mock.MagicMock()
             mock_sync_buf_instance.Finish.return_value = True
+            mock_sync_buf_instance.errors = []
             mock_sync_buffer.return_value = mock_sync_buf_instance
 
             result_obj = self.cmd._SyncProjectList(opt, [0])