Merge lp:~cjwatson/turnip/auto-create-repository into lp:turnip

[Colin Watson]

Attempt to automatically create non-existent repositories on push.

Attempt to automatically create non-existent repositories on push.

This relies on a new createRepository XML-RPC call. I haven't planned to implement this in turnipcake, but I have a working (though not yet pushed anywhere) implementation in Launchpad.

[William Grant (wgrant)]

I was thinking that this could instead be done by Launchpad inside translatePath if it knows it hasn't created it yet. What do you think about that?

[lp:~cjwatson/turnip/auto-create-repository]

93. By Colin Watson on 2015-02-13

Revert previous commit.

94. By Colin Watson on 2015-02-13

Ensure that a repository exists on disk before receiving pack data into it.

[Colin Watson (cjwatson)]

When trying to implement William's suggestion, I found that indeed it's quite reasonable for Launchpad to take care of creating a GitRepository row when asked to translate-for-writing a well-formed path it doesn't know about yet, but it's still useful for turnip to deal with the "git init" bit when trying to receive into a repository path that doesn't yet exist on disk. This makes the interface between Launchpad and turnip much simpler when creating repositories.

I've amended this branch with this new approach.

[William Grant (wgrant)]

On 14/02/15 05:28, Colin Watson wrote:
> When trying to implement William's suggestion, I found that indeed it's quite reasonable for Launchpad to take care of creating a GitRepository row when asked to translate-for-writing a well-formed path it doesn't know about yet, but it's still useful for turnip to deal with the "git init" bit when trying to receive into a repository path that doesn't yet exist on disk. This makes the interface between Launchpad and turnip much simpler when creating repositories.
>
> I've amended this branch with this new approach.

Will that still be useful when 99% of created repositories want to be
forks of existing ones, rather than fresh inits?

[Colin Watson (cjwatson)]

It probably won't be *useful* in that case, but also not harmful; I mean, if we have to do something first to fork the repository, then we have to do that no matter what, right?

[William Grant (wgrant)]

On 14/02/15 20:34, Colin Watson wrote:
> It probably won't be *useful* in that case, but also not harmful; I
> mean, if we have to do something first to fork the repository, then
> we have to do that no matter what, right?

If we run into this case on production it must indicate a bug, and
rather than reporting a problem it will just silently do the wrong thing.

In what case would this be useful in a real environment? Also remember
that we currently have no local state except the repository, but that
will eventually change, at which point implicit creation on ENOENT
becomes less desirable.

I would have Launchpad's translatePath create the LP GitRepository and
then call into turnip to create the repo.

[Colin Watson (cjwatson)]

I think I was misled by your comment on https://code.launchpad.net/~cjwatson/launchpad/git-namespace/+merge/248995 where you said "We might also be able to get away with doing this on the first repository access" into thinking that we could do part of it on the first access *in turnip*, and didn't think properly about the state requirements. Fair enough; I'll withdraw this MP, and include GitHostingClient again in Launchpad when I get far enough up the stack to have the XML-RPC service code.

[William Grant (wgrant)]

Ah, sorry, that was ambiguous indeed. I was meaning something sort of like the PPA key generation situation: we may assume that some reasonable subset of created repositories will never be accessed, so it may make sense to lazily create the turnip side of things. If we were to do that, LP's translatePath would notice that it has a GitRepository row but doesn't exist in turnip, and make the same call into turnip as it would if the GitRepository didn't exist. But Git repositories are cheaper than OpenPGP keys, so it may not be sensible to delay that except for app performance reasons.

Unmerged revisions

94. By Colin Watson on 2015-02-13

Ensure that a repository exists on disk before receiving pack data into it.

93. By Colin Watson on 2015-02-13

Revert previous commit.

92. By Colin Watson on 2015-02-09

Attempt to automatically create non-existent repositories on push.

=== modified file 'turnip/api.py'
--- turnip/api.py	2015-02-03 16:59:42 +0000
+++ turnip/api.py	2015-02-13 18:24:05 +0000
@@ -4,17 +4,17 @@
     unicode_literals,
     )
 
-import os
-
 from twisted.internet import defer
-from twisted.internet.utils import getProcessValue
 from twisted.web import (
     resource,
     server,
     static,
     )
 
-from turnip.helpers import compose_path
+from turnip.helpers import (
+    compose_path,
+    ensure_repository_exists,
+    )
 
 
 class TurnipAPIResource(resource.Resource):
@@ -44,11 +44,7 @@
     @defer.inlineCallbacks
     def createRepo(self, request, raw_path):
         repo_path = compose_path(self.root, raw_path)
-        if os.path.exists(repo_path):
-            raise Exception("Repository '%s' already exists" % repo_path)
-        ret = yield getProcessValue('git', ('init', '--bare', repo_path))
-        if ret != 0:
-            raise Exception("'git init' failed")
+        yield ensure_repository_exists(repo_path)
         request.write(b'OK')
         request.finish()
 

=== modified file 'turnip/helpers.py'
--- turnip/helpers.py	2015-01-22 06:14:32 +0000
+++ turnip/helpers.py	2015-02-13 18:24:05 +0000
@@ -6,6 +6,9 @@
 
 import os.path
 
+from twisted.internet import defer
+from twisted.internet.utils import getProcessValue
+
 
 def compose_path(root, path):
     # Construct the full path, stripping any leading slashes so we
@@ -15,3 +18,11 @@
     if not full_path.startswith(os.path.abspath(root)):
         raise ValueError('Path not contained within root')
     return full_path
+
+
[e-mail]
+def ensure_repository_exists(repo_path):
+    if not os.path.exists(repo_path):
+        ret = yield getProcessValue('git', ('init', '--bare', repo_path))
+        if ret != 0:
+            raise Exception("'git init' failed")

=== modified file 'turnip/pack/git.py'
--- turnip/pack/git.py	2015-02-09 10:23:55 +0000
+++ turnip/pack/git.py	2015-02-13 18:24:05 +0000
@@ -18,7 +18,10 @@
     from twisted.web import xmlrpc
 from zope.interface import implementer
 
-from turnip.helpers import compose_path
+from turnip.helpers import (
+    compose_path,
+    ensure_repository_exists,
+    )
 from turnip.pack.helpers import (
     decode_packet,
     decode_request,
@@ -271,12 +274,14 @@
     Invokes the reference C Git implementation.
     """
 
+    @defer.inlineCallbacks
     def requestReceived(self, command, raw_pathname, params):
         path = compose_path(self.factory.root, raw_pathname)
         if command == b'git-upload-pack':
             subcmd = b'upload-pack'
         elif command == b'git-receive-pack':
             subcmd = b'receive-pack'
+            yield ensure_repository_exists(path)
         else:
             self.die(b'Unsupport command in request')
             return