launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:count-objects-directory into turnip:master.

Commit message:
Run "git count-objects -v" in the correct directory

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/399929

get_repack_data works fine when called from a git hook, because they're already run from the correct directory; but it breaks when called from a turnip-set-symbolic-ref handler, because those don't run with the repository path as their working directory.  Explicitly set the working directory in the latter case to fix this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:count-objects-directory into turnip:master.

diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 46ffe0c..2fe3ff4 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -680,7 +680,7 @@ class PackBackendProtocol(PackServerProtocol):
         if self.command == b'turnip-set-symbolic-ref':
             if reason.check(error.ProcessDone):
                 try:
-                    loose_object_count, pack_count = get_repack_data()
+                    loose_object_count, pack_count = get_repack_data(self.path)
                     yield self.factory.hookrpc_handler.notify(
                         self.raw_pathname, loose_object_count, pack_count,
                         self.factory.hookrpc_handler.auth_params)
diff --git a/turnip/pack/helpers.py b/turnip/pack/helpers.py
index d6fd344..561bcb6 100644
--- a/turnip/pack/helpers.py
+++ b/turnip/pack/helpers.py
@@ -205,9 +205,9 @@ def ensure_hooks(repo_root):
                 pass
 
 
-def get_repack_data():
+def get_repack_data(path=None):
     output = subprocess.check_output(
-        ['git', 'count-objects', '-v'], universal_newlines=True)
+        ['git', 'count-objects', '-v'], cwd=path, universal_newlines=True)
     if not output:
         return None, None
     match = re.search(r'^packs: (.*)', output, flags=re.M)
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index 9c51a86..a695168 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -851,11 +851,9 @@ class TestSmartHTTPFrontendFunctional(FrontendFunctionalTestMixin, TestCase):
         self.assertEqual(
             six.ensure_text(self.internal_name),
             self.virtinfo.push_notifications[0][0])
-        self.assertNotEqual(
-            0,
+        self.assertIsNotNone(
             self.virtinfo.push_notifications[0][1].get('pack_count'))
-        self.assertNotEqual(
-            0,
+        self.assertIsNotNone(
             self.virtinfo.push_notifications[0][1].get('loose_object_count'))
 
     @defer.inlineCallbacks
diff --git a/turnip/pack/tests/test_helpers.py b/turnip/pack/tests/test_helpers.py
index 2ffba15..f29cd74 100644
--- a/turnip/pack/tests/test_helpers.py
+++ b/turnip/pack/tests/test_helpers.py
@@ -18,7 +18,10 @@ from textwrap import dedent
 import time
 import uuid
 
-from fixtures import TempDir
+from fixtures import (
+    MockPatch,
+    TempDir,
+    )
 from pygit2 import (
     Config,
     GIT_FILEMODE_BLOB,
@@ -431,6 +434,30 @@ class TestGetRepackData(TestCase):
     def test_get_repack_data(self):
         curdir = os.getcwd()
         os.chdir(self.repo_dir)
+        self.addCleanup(os.chdir, curdir)
+        # create a test file
+        blob_content = (
+            b'commit ' + 'file content'.encode('ascii') + b' - '
+            + uuid.uuid1().hex.encode('ascii'))
+        test_file = 'test.txt'
+        # stage the changes
+        self.repo.index.add(IndexEntry(
+            test_file, self.repo.create_blob(blob_content),
+            GIT_FILEMODE_BLOB))
+        self.repo.index.write_tree()
+
+        with MockPatch(
+                'subprocess.check_output',
+                wraps=subprocess.check_output) as spy:
+            objects, packs = get_repack_data()
+            spy.mock.assert_called_once_with(
+                ['git', 'count-objects', '-v'],
+                cwd=None, universal_newlines=True)
+
+        self.assertIsNotNone(objects)
+        self.assertIsNotNone(packs)
+
+    def test_get_repack_data_with_path(self):
         # create a test file
         blob_content = (
             b'commit ' + 'file content'.encode('ascii') + b' - '
@@ -442,8 +469,13 @@ class TestGetRepackData(TestCase):
             GIT_FILEMODE_BLOB))
         self.repo.index.write_tree()
 
-        objects, packs = get_repack_data()
-        os.chdir(curdir)
+        with MockPatch(
+                'subprocess.check_output',
+                wraps=subprocess.check_output) as spy:
+            objects, packs = get_repack_data(self.repo_dir)
+            spy.mock.assert_called_once_with(
+                ['git', 'count-objects', '-v'],
+                cwd=self.repo_dir, universal_newlines=True)
 
         self.assertIsNotNone(objects)
         self.assertIsNotNone(packs)