From 39f12bd49a49b96d435c0ab7915bde6011d34f0f Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:19:51 +0100 Subject: [PATCH 1/7] Do not call get_user_id if it is not needed On systems without any environment variables and no pwd module, gitpython crashes as it tries to read the environment variable before looking at its config. --- git/util.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/git/util.py b/git/util.py index 92d95379e..84d0b0156 100644 --- a/git/util.py +++ b/git/util.py @@ -704,7 +704,11 @@ def default_name() -> str: setattr(actor, attr, val) except KeyError: if config_reader is not None: - setattr(actor, attr, config_reader.get_value('user', cvar, default())) + try: + val = config_reader.get_value('user', cvar) + except Exception: + val = default() + setattr(actor, attr, val) # END config-reader handling if not getattr(actor, attr): setattr(actor, attr, default()) From ff0ecf7ff56ea1e19f0c4e3be24b893049939916 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:43:36 +0100 Subject: [PATCH 2/7] Fix mypy --- git/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index 011d0e0b1..d65360fe2 100644 --- a/git/config.py +++ b/git/config.py @@ -708,12 +708,12 @@ def read_only(self) -> bool: return self._read_only @overload - def get_value(self, section: str, option: str, default: str + def get_value(self, section: str, option: str, default: Optional[str] ) -> str: ... @overload - def get_value(self, section: str, option: str, default: float + def get_value(self, section: str, option: str, default: Optional[float] ) -> float: ... From 335e59dc2cece491a5c5d42396ce70d4ed0715b5 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:44:10 +0100 Subject: [PATCH 3/7] Update config.py --- git/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index d65360fe2..3c08dd5fc 100644 --- a/git/config.py +++ b/git/config.py @@ -708,12 +708,12 @@ def read_only(self) -> bool: return self._read_only @overload - def get_value(self, section: str, option: str, default: Optional[str] + def get_value(self, section: str, option: str, default: Optional[str] = None ) -> str: ... @overload - def get_value(self, section: str, option: str, default: Optional[float] + def get_value(self, section: str, option: str, default: Optional[float] = None ) -> float: ... From 994f387cec4848ab854a5c06ad092c2850a3bf73 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 17:15:59 +0100 Subject: [PATCH 4/7] Use get instead of get_value This won't try and do something silly like convert `username=1` to a number. --- git/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/util.py b/git/util.py index 4463b092d..b81332ea4 100644 --- a/git/util.py +++ b/git/util.py @@ -706,7 +706,7 @@ def default_name() -> str: except KeyError: if config_reader is not None: try: - val = config_reader.get_value('user', cvar) + val = config_reader.get('user', cvar) except Exception: val = default() setattr(actor, attr, val) From 70b50e068dc2496d923ee336901ed55d212fc83d Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 19:16:41 +0100 Subject: [PATCH 5/7] Fix test --- test/test_util.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index ddc5f628f..3058dc745 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -238,16 +238,16 @@ def test_actor_get_uid_laziness_not_called(self, mock_get_uid): @mock.patch("getpass.getuser") def test_actor_get_uid_laziness_called(self, mock_get_uid): mock_get_uid.return_value = "user" - for cr in (None, self.rorepo.config_reader()): - committer = Actor.committer(cr) - author = Actor.author(cr) - if cr is None: # otherwise, use value from config_reader - self.assertEqual(committer.name, 'user') - self.assertTrue(committer.email.startswith('user@')) - self.assertEqual(author.name, 'user') - self.assertTrue(committer.email.startswith('user@')) + committer = Actor.committer(None) + author = Actor.author(None) + # We can't test with `self.rorepo.config_reader()` here, as the uuid laziness + # depends on whether the user running the test has their user.name config set. + self.assertEqual(committer.name, 'user') + self.assertTrue(committer.email.startswith('user@')) + self.assertEqual(author.name, 'user') + self.assertTrue(committer.email.startswith('user@')) self.assertTrue(mock_get_uid.called) - self.assertEqual(mock_get_uid.call_count, 4) + self.assertEqual(mock_get_uid.call_count, 2) def test_actor_from_string(self): self.assertEqual(Actor._from_string("name"), Actor("name", None)) From d490d66e4b82d94b378daa9ad1e1a286e0e09258 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Wed, 4 Aug 2021 10:15:42 +0100 Subject: [PATCH 6/7] Try a better test --- test/test_util.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index 3058dc745..e9701f0c4 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -217,8 +217,23 @@ def test_actor(self): self.assertIsInstance(Actor.author(cr), Actor) # END assure config reader is handled + @with_rw_repo('HEAD') @mock.patch("getpass.getuser") - def test_actor_get_uid_laziness_not_called(self, mock_get_uid): + def test_actor_get_uid_laziness_not_called(self, rwrepo, mock_get_uid): + with rwrepo.config_writer() as cw: + cw.set_value("user", "name", "John Config Doe") + cw.set_value("user", "email", "jcdoe@example.com") + + cr = rwrepo.config_reader() + committer = Actor.committer(cr) + author = Actor.author(cr) + + self.assertEqual(committer.name, 'John Config Doe') + self.assertEqual(committer.email, 'jcdoe@example.com') + self.assertEqual(author.name, 'John Config Doe') + self.assertEqual(author.email, 'jcdoe@example.com') + self.assertFalse(mock_get_uid.called) + env = { "GIT_AUTHOR_NAME": "John Doe", "GIT_AUTHOR_EMAIL": "jdoe@example.com", @@ -226,7 +241,7 @@ def test_actor_get_uid_laziness_not_called(self, mock_get_uid): "GIT_COMMITTER_EMAIL": "jane@example.com", } os.environ.update(env) - for cr in (None, self.rorepo.config_reader()): + for cr in (None, rwrepo.config_reader()): committer = Actor.committer(cr) author = Actor.author(cr) self.assertEqual(committer.name, 'Jane Doe') @@ -241,7 +256,7 @@ def test_actor_get_uid_laziness_called(self, mock_get_uid): committer = Actor.committer(None) author = Actor.author(None) # We can't test with `self.rorepo.config_reader()` here, as the uuid laziness - # depends on whether the user running the test has their user.name config set. + # depends on whether the user running the test has their global user.name config set. self.assertEqual(committer.name, 'user') self.assertTrue(committer.email.startswith('user@')) self.assertEqual(author.name, 'user') From ec04ea01dbbab9c36105dfc3e34c94bbbbf298b2 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Wed, 4 Aug 2021 10:20:44 +0100 Subject: [PATCH 7/7] Update test_util.py --- test/test_util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_util.py b/test/test_util.py index e9701f0c4..3961ff356 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -22,7 +22,10 @@ parse_date, tzoffset, from_timestamp) -from test.lib import TestBase +from test.lib import ( + TestBase, + with_rw_repo, +) from git.util import ( LockFile, BlockingLockFile,