From 39f12bd49a49b96d435c0ab7915bde6011d34f0f Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:19:51 +0100 Subject: 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 92d95379..84d0b015 100644 --- a/git/util.py +++ b/git/util.py @@ -704,7 +704,11 @@ class Actor(object): 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()) -- cgit v1.2.1 From ff0ecf7ff56ea1e19f0c4e3be24b893049939916 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:43:36 +0100 Subject: Fix mypy --- git/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index 011d0e0b..d65360fe 100644 --- a/git/config.py +++ b/git/config.py @@ -708,12 +708,12 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder): 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: ... -- cgit v1.2.1 From 335e59dc2cece491a5c5d42396ce70d4ed0715b5 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 16:44:10 +0100 Subject: 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 d65360fe..3c08dd5f 100644 --- a/git/config.py +++ b/git/config.py @@ -708,12 +708,12 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder): 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: ... -- cgit v1.2.1 From 994f387cec4848ab854a5c06ad092c2850a3bf73 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 17:15:59 +0100 Subject: 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 4463b092..b81332ea 100644 --- a/git/util.py +++ b/git/util.py @@ -706,7 +706,7 @@ class Actor(object): 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) -- cgit v1.2.1 From 70b50e068dc2496d923ee336901ed55d212fc83d Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Tue, 3 Aug 2021 19:16:41 +0100 Subject: 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 ddc5f628..3058dc74 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -238,16 +238,16 @@ class TestUtils(TestBase): @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)) -- cgit v1.2.1 From d490d66e4b82d94b378daa9ad1e1a286e0e09258 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Wed, 4 Aug 2021 10:15:42 +0100 Subject: 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 3058dc74..e9701f0c 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -217,8 +217,23 @@ class TestUtils(TestBase): 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 @@ class TestUtils(TestBase): "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 @@ class TestUtils(TestBase): 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') -- cgit v1.2.1 From ec04ea01dbbab9c36105dfc3e34c94bbbbf298b2 Mon Sep 17 00:00:00 2001 From: Eric Wieser Date: Wed, 4 Aug 2021 10:20:44 +0100 Subject: 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 e9701f0c..3961ff35 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -22,7 +22,10 @@ from git.objects.util import ( parse_date, tzoffset, from_timestamp) -from test.lib import TestBase +from test.lib import ( + TestBase, + with_rw_repo, +) from git.util import ( LockFile, BlockingLockFile, -- cgit v1.2.1