launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/turnip/enable-reflog into lp:turnip.

Commit message:
Set core.logallrefupdates to true before any write operation.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/enable-reflog/+merge/258084

Set core.logallrefupdates to true before any write operation.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/turnip/enable-reflog into lp:turnip.

=== modified file 'turnip/pack/git.py'
--- turnip/pack/git.py	2015-04-14 16:41:23 +0000
+++ turnip/pack/git.py	2015-05-01 21:20:57 +0000
@@ -25,6 +25,7 @@
     decode_request,
     encode_packet,
     encode_request,
+    ensure_config,
     ensure_hooks,
     INCOMPLETE_PKT,
     )
@@ -330,8 +331,9 @@
 
         env = {}
         if subcmd == b'receive-pack' and self.factory.hookrpc_handler:
-            # This is a write operation, so prepare hooks, the hook RPC
-            # server, and the environment variables that link them up.
+            # This is a write operation, so prepare config, hooks, the hook
+            # RPC server, and the environment variables that link them up.
+            ensure_config(path)
             self.hookrpc_key = str(uuid.uuid4())
             self.factory.hookrpc_handler.registerKey(
                 self.hookrpc_key, raw_pathname, [])

=== modified file 'turnip/pack/helpers.py'
--- turnip/pack/helpers.py	2015-04-26 15:44:21 +0000
+++ turnip/pack/helpers.py	2015-05-01 21:20:57 +0000
@@ -13,6 +13,8 @@
     NamedTemporaryFile,
     )
 
+from pygit2 import Repository
+
 import turnip.pack.hooks
 
 
@@ -96,6 +98,17 @@
     return command + b' ' + b'\0'.join(bits) + b'\0'
 
 
+def ensure_config(repo_root):
+    """Put a repository's configuration into the desired state.
+
+    pygit2.Config handles locking itself, so we don't need to think too hard
+    about concurrency.
+    """
+
+    config = Repository(repo_root).config
+    config['core.logallrefupdates'] = True
+
+
 def ensure_hooks(repo_root):
     """Put a repository's hooks into the desired state.
 

=== modified file 'turnip/pack/tests/test_helpers.py'
--- turnip/pack/tests/test_helpers.py	2015-04-26 15:44:21 +0000
+++ turnip/pack/tests/test_helpers.py	2015-05-01 21:20:57 +0000
@@ -7,8 +7,14 @@
 import hashlib
 import os.path
 import stat
+from textwrap import dedent
+import time
 
 from fixtures import TempDir
+from pygit2 import (
+    Config,
+    init_repository,
+    )
 from testtools import TestCase
 
 from turnip.pack import helpers
@@ -161,6 +167,43 @@
             b'Metacharacter in arguments')
 
 
+class TestEnsureConfig(TestCase):
+    """Test repository configuration maintenance."""
+
+    def setUp(self):
+        super(TestEnsureConfig, self).setUp()
+        self.repo_dir = self.useFixture(TempDir()).path
+        init_repository(self.repo_dir, bare=True)
+        self.config_path = os.path.join(self.repo_dir, 'config')
+
+    def assertWritesCorrectConfig(self):
+        helpers.ensure_config(self.repo_dir)
+        config = Config(path=self.config_path)
+        self.assertTrue(config['core.logallrefupdates'])
+
+    def test_writes_new(self):
+        self.assertWritesCorrectConfig()
+
+    def test_preserves_existing(self):
+        # If the configuration file is already in the correct state, then
+        # the file is left unchanged; for efficiency we do not even write
+        # out a new file.  (Currently, pygit2/libgit2 take care of this; if
+        # they ever stop doing so then we should take extra care ourselves.)
+        helpers.ensure_config(self.repo_dir)
+        now = time.time()
+        os.utime(self.config_path, (now - 60, now - 60))
+        self.assertWritesCorrectConfig()
+        self.assertEqual(now - 60, os.stat(self.config_path).st_mtime)
+
+    def test_fixes_incorrect(self):
+        with open(self.config_path, 'w') as f:
+            f.write(dedent("""\
+                [core]
+                \tlogallrefupdates = false
+                """))
+        self.assertWritesCorrectConfig()
+
+
 class TestEnsureHooks(TestCase):
     """Test repository hook maintenance."""