From 948edc43825d3609a626b8c865da9f2c3c4a10a7 Mon Sep 17 00:00:00 2001 From: Josip Sokcevic Date: Fri, 14 Mar 2025 15:06:10 -0700 Subject: [PATCH] Revert "[git_auth] Use pathless URL for cred helper" This reverts commit ce47e785fae81261fb6b99a8ce123b37244485d3. Reason for revert: fatal: --local can only be used inside a git repository. Original change's description: > [git_auth] Use pathless URL for cred helper > > The credential helper rules should not have a path, while the URL > rewrite rules should. > > Bug: b/401338175 > Change-Id: I79b52ab4af62367363617b2a9afa03a67f5ea0b9 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6345631 > Commit-Queue: Allen Li > Reviewed-by: Josip Sokcevic Bug: b/401338175 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: Ifa887eeb7a7049665570e865431b41ac18649b90 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6357165 Bot-Commit: Rubber Stamper Auto-Submit: Josip Sokcevic Commit-Queue: Rubber Stamper --- git_auth.py | 48 ++++++------------------------------------ tests/git_auth_test.py | 10 +++++---- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/git_auth.py b/git_auth.py index 1aac07928..81c9533ef 100644 --- a/git_auth.py +++ b/git_auth.py @@ -36,7 +36,7 @@ class ConfigChanger(object): # # Increment this when making changes to the config, so that reliant # code can determine whether the config needs to be re-applied. - VERSION: int = 5 + VERSION: int = 4 def __init__( self, @@ -67,20 +67,12 @@ class ConfigChanger(object): return _url_shortname(parts) @functools.cached_property - def _host_url(self) -> str: - # Example: https://chromium.googlesource.com - # Example: https://chromium-review.googlesource.com - parts: urllib.parse.SplitResult = urllib.parse.urlsplit( - self._remote_url) - return _url_host_url(parts) - - @functools.cached_property - def _root_url(self) -> str: + def _base_url(self) -> str: # Example: https://chromium.googlesource.com/ # Example: https://chromium-review.googlesource.com/ parts: urllib.parse.SplitResult = urllib.parse.urlsplit( self._remote_url) - return _url_root_url(parts) + return parts._replace(path='/', query='', fragment='').geturl() @classmethod def new_from_env(cls, cwd: str, cl: git_cl.Changelist) -> ConfigChanger: @@ -158,7 +150,7 @@ class ConfigChanger(object): def _apply_cred_helper(self, cwd: str) -> None: """Apply config changes relating to credential helper.""" - cred_key: str = f'credential.{self._host_url}.helper' + cred_key: str = f'credential.{self._base_url}.helper' if self.mode == ConfigMode.NEW_AUTH: self._set_config(cwd, cred_key, '', modify_all=True) self._set_config(cwd, cred_key, 'luci', append=True) @@ -169,10 +161,6 @@ class ConfigChanger(object): else: raise TypeError(f'Invalid mode {self.mode!r}') - # Cleanup old from version 4 - old_key: str = f'credential.{self._root_url}.helper' - self._set_config(cwd, old_key, None, modify_all=True) - def _apply_sso(self, cwd: str) -> None: """Apply config changes relating to SSO.""" sso_key: str = f'url.sso://{self._shortname}/.insteadOf' @@ -184,7 +172,7 @@ class ConfigChanger(object): self._set_config(cwd, http_key, self._remote_url, modify_all=True) elif self.mode == ConfigMode.NEW_AUTH_SSO: self._set_config(cwd, 'protocol.sso.allow', 'always') - self._set_config(cwd, sso_key, self._root_url, modify_all=True) + self._set_config(cwd, sso_key, self._base_url, modify_all=True) self._set_config(cwd, http_key, None, modify_all=True) elif self.mode == ConfigMode.NO_AUTH: self._set_config(cwd, 'protocol.sso.allow', None) @@ -208,7 +196,7 @@ class ConfigChanger(object): def _apply_global_cred_helper(self, cwd: str) -> None: """Apply config changes relating to credential helper.""" - cred_key: str = f'credential.{self._host_url}.helper' + cred_key: str = f'credential.{self._base_url}.helper' if self.mode == ConfigMode.NEW_AUTH: self._set_config(cwd, cred_key, '', scope='global', modify_all=True) self._set_config(cwd, cred_key, 'luci', scope='global', append=True) @@ -223,10 +211,6 @@ class ConfigChanger(object): else: raise TypeError(f'Invalid mode {self.mode!r}') - # Cleanup old from version 4 - old_key: str = f'credential.{self._root_url}.helper' - self._set_config(cwd, old_key, None, modify_all=True) - def _apply_global_sso(self, cwd: str) -> None: """Apply config changes relating to SSO.""" sso_key: str = f'url.sso://{self._shortname}/.insteadOf' @@ -245,7 +229,7 @@ class ConfigChanger(object): scope='global') self._set_config(cwd, sso_key, - self._root_url, + self._base_url, scope='global', modify_all=True) elif self.mode == ConfigMode.NO_AUTH: @@ -339,21 +323,3 @@ def _url_shortname(parts: urllib.parse.SplitResult) -> str: if name.endswith('-review'): name = name[:-len('-review')] return name - - -def _url_host_url(parts: urllib.parse.SplitResult) -> str: - """Format URL with host only (no path). - - Example: https://chromium.googlesource.com - Example: https://chromium-review.googlesource.com - """ - return parts._replace(path='', query='', fragment='').geturl() - - -def _url_root_url(parts: urllib.parse.SplitResult) -> str: - """Format URL with root path. - - Example: https://chromium.googlesource.com/ - Example: https://chromium-review.googlesource.com/ - """ - return parts._replace(path='/', query='', fragment='').geturl() diff --git a/tests/git_auth_test.py b/tests/git_auth_test.py index 62e579d23..0643d87d2 100755 --- a/tests/git_auth_test.py +++ b/tests/git_auth_test.py @@ -38,7 +38,7 @@ class TestConfigChanger(unittest.TestCase): ).apply('/some/fake/dir') want = { '/some/fake/dir': { - 'credential.https://chromium.googlesource.com.helper': + 'credential.https://chromium.googlesource.com/.helper': ['', 'luci'], 'http.cookiefile': [''], 'url.https://chromium.googlesource.com/chromium/tools/depot_tools.git.insteadof': @@ -89,7 +89,7 @@ class TestConfigChanger(unittest.TestCase): ).apply('/some/fake/dir') want = { '/some/fake/dir': { - 'credential.https://chromium.googlesource.com.helper': + 'credential.https://chromium.googlesource.com/.helper': ['', 'luci'], 'http.cookiefile': [''], 'url.https://chromium.googlesource.com/chromium/tools/depot_tools.git.insteadof': @@ -160,7 +160,8 @@ class TestConfigChanger(unittest.TestCase): 'https://chromium.googlesource.com/chromium/tools/depot_tools.git', ).apply_global('/some/fake/dir') want = { - 'credential.https://chromium.googlesource.com.helper': ['', 'luci'], + 'credential.https://chromium.googlesource.com/.helper': + ['', 'luci'], } self.assertEqual(self.global_state, want) @@ -190,7 +191,8 @@ class TestConfigChanger(unittest.TestCase): ).apply_global('/some/fake/dir') want = { 'protocol.sso.allow': ['always'], - 'credential.https://chromium.googlesource.com.helper': ['', 'luci'], + 'credential.https://chromium.googlesource.com/.helper': + ['', 'luci'], } self.assertEqual(self.global_state, want)