From 0ae9503a867955cf4818b9952fa73f1c04c55e37 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 4 May 2021 07:21:19 -0400 Subject: [PATCH 1/3] sync: fix print error when handling server error When converting this logic from print() to the output buffer, this error codepath should have dropped the use of the file= redirect. Bug: https://crbug.com/gerrit/14482 Change-Id: Ib484924a2031ba3295c1c1a5b9a2d816b9912279 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/305142 Reviewed-by: Raman Tenneti Tested-by: Mike Frysinger --- project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project.py b/project.py index 6679ee31..992a0c07 100644 --- a/project.py +++ b/project.py @@ -2205,7 +2205,7 @@ class Project(object): # Figure out how long to sleep before the next attempt, if there is one. if not verbose: - output_redir.write('\n%s:\n%s' % (self.name, gitcmd.stdout), file=sys.stderr) + output_redir.write('\n%s:\n%s' % (self.name, gitcmd.stdout)) if try_n < retry_fetches - 1: output_redir.write('sleeping %s seconds before retrying' % retry_cur_sleep) time.sleep(retry_cur_sleep) From 32ca6687ae422bda773f10fe7d88100de034b541 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 4 May 2021 17:58:26 -0400 Subject: [PATCH 2/3] git_config: hoist Windows ssh check earlier The ssh master logic has never worked under Windows which is why this code always returned False when running there (including cygwin). But the OS check was still done while holding the threading lock. While it might be a little slower than necessary, it still worked. The switch from the threading module to the multiprocessing module changed global behavior subtly under Windows and broke things: the globals previously would stay valid, but now they get cleared. So the lock is reset to None in children workers. We could tweak the logic to pass the lock through, but there isn't much point when the rest of the code is still disabled in Windows. So perform the platform check before we grab the lock. This fixes the crash, and probably speeds things up a few nanoseconds. This shouldn't be a problem on Linux systems as the platform fork will duplicate the existing process memory (including globals). Bug: https://crbug.com/gerrit/14480 Change-Id: I1d1da82c6d7bd6b8cdc1f03f640a520ecd047063 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/305149 Reviewed-by: Raman Tenneti Tested-by: Mike Frysinger --- git_config.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/git_config.py b/git_config.py index 914b2924..fcd0446c 100644 --- a/git_config.py +++ b/git_config.py @@ -459,6 +459,11 @@ def init_ssh(): def _open_ssh(host, port=None): global _ssh_master + # Bail before grabbing the lock if we already know that we aren't going to + # try creating new masters below. + if sys.platform in ('win32', 'cygwin'): + return False + # Acquire the lock. This is needed to prevent opening multiple masters for # the same host when we're running "repo sync -jN" (for N > 1) _and_ the # manifest specifies a different host from the @@ -476,11 +481,8 @@ def _open_ssh(host, port=None): if key in _master_keys: return True - if (not _ssh_master - or 'GIT_SSH' in os.environ - or sys.platform in ('win32', 'cygwin')): - # failed earlier, or cygwin ssh can't do this - # + if not _ssh_master or 'GIT_SSH' in os.environ: + # Failed earlier, so don't retry. return False # We will make two calls to ssh; this is the common part of both calls. From 148e1ce81addc9d41238e6f67bde8ef930851f58 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 4 May 2021 19:46:09 -0400 Subject: [PATCH 3/3] sync: fix recursive fetching Commit b2fa30a2b891b22c173c960a67bf38ccbba8de1b ("sync: switch network fetch to multiprocessing") accidentally changed the variable passed to the 2nd fetch call from |missing| to |to_fetch| due to a copy & paste of the earlier changed logic. Undo that to fix git submodule fetching. Bug: https://crbug.com/gerrit/14489 Change-Id: I627954f80fd2e80d9d5809b530aa6b0ef9260abb Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/305262 Reviewed-by: Raman Tenneti Tested-by: Mike Frysinger --- subcmds/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcmds/sync.py b/subcmds/sync.py index aafec1d2..d41052d7 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -887,7 +887,7 @@ later is required to fix a server side protocol bug. if previously_missing_set == missing_set: break previously_missing_set = missing_set - success, new_fetched = self._Fetch(to_fetch, opt, err_event) + success, new_fetched = self._Fetch(missing, opt, err_event) if not success: err_event.set() fetched.update(new_fetched)