Merge ~racb/git-ubuntu:derive-env into git-ubuntu:master

[Robie Basak]

Make Jenkins happy

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:a71ff92849ae06203ce7c2c5d7bdce355f042560
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/279/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/279/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:4d1d2bdb1d514bcf5bf0ac3e736bf88ff372dbca
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/281/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/281/rebuild

review: Approve (continuous-integration)

Unmerged commits

4d1d2bd... by Robie Basak on 2018-02-06

Replace self._env with a derived property

As part of the drive to reduce stored state to improve testability,
derive self.env when we need it instead of working out in advance in the
constructor.

This is intended to have no functional changes.

GIT_WORK_TREE will end up differently normalised now, such as in
trailing slashes, since in testing pygit2.Repository.workdir was not
normalised like this and so a normalised assertion was needed. This
should not result in any change to git's behaviour.

diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
index f3bd406..d2768f8 100644
--- a/gitubuntu/git_repository.py
+++ b/gitubuntu/git_repository.py
@@ -779,10 +779,24 @@ class GitUbuntuRepository:
             self._local_dir = local_dir
 
         self.raw_repo = pygit2.init_repository(self._local_dir)
+        # We rely on raw_repo.workdir to be identical to self._local_dir to
+        # avoid changing previous behaviour in the setting of GIT_WORK_TREE, so
+        # assert that it is so. This may not be the case if the git repository
+        # has a different workdir stored in its configuration or if the git
+        # repository is a bare repository. We didn't handle these cases before
+        # anyway, so with this assertion we can fail noisily and early.
+        assert (
+            os.path.normpath(self.raw_repo.workdir) ==
+                os.path.normpath(self._local_dir)
+        )
 
-        self._env = os.environ.copy()
-        self._env['GIT_DIR'] = self.raw_repo.path
-        self._env['GIT_WORK_TREE'] = self._local_dir
+        # Since previous behaviour of this class depended on the state of the
+        # environment at the time it was constructed, save this for later use
+        # (for example in deriving the environment to use for calls to the git
+        # CLI). This permits the behaviour to remain identical for now.
+        # Eventually we can break previous behaviour and eliminate the need for
+        # this.
+        self._initial_env = os.environ.copy()
 
         self.set_git_attributes()
 
@@ -1224,7 +1238,20 @@ class GitUbuntuRepository:
 
     @property
     def env(self):
-        return self._env
+        return self._derive_env()
+
+    @lru_cache()
+    def _derive_env(self):
+        """Determine what the git CLI environment should be
+
+        This depends on the initial environment saved from the constructor and
+        the paths associated with self.raw_repo, neither of which should change
+        in the lifetime of this class instance.
+        """
+        env = self._initial_env.copy()
+        env['GIT_DIR'] = self.raw_repo.path
+        env['GIT_WORK_TREE'] = self.raw_repo.workdir
+        return env
 
     @property
     def local_dir(self):
@@ -1308,9 +1335,9 @@ class GitUbuntuRepository:
         return stdout.strip()
 
     def git_run(self, args, env=None, **kwargs):
-        # Explicitly take a copy of self._env so the following update doesn't
-        # accidentally update the main instance _env dictionary in-place
-        combined_env = dict(self._env)
+        # Explicitly take a copy of self.env so the following update doesn't
+        # accidentally update the main instance env dictionary in-place
+        combined_env = dict(self.env)
         if env is not None:
             combined_env.update(env)
         return run(['git'] + args, env=combined_env, **kwargs)
@@ -1626,9 +1653,9 @@ class GitUbuntuRepository:
     def clean_repository_state(self):
         """Cleanup working tree"""
         runq(['git', 'checkout', '--orphan', 'master'],
-             check=False, env=self._env)
-        runq(['git', 'reset', '--hard'], env=self._env)
-        runq(['git', 'clean', '-f', '-d'], env=self._env)
+             check=False, env=self.env)
+        runq(['git', 'reset', '--hard'], env=self.env)
+        runq(['git', 'clean', '-f', '-d'], env=self.env)
 
     def get_all_changelog_versions_from_treeish(self, treeish):
         changelog = self.get_changelog_from_treeish(treeish)