threads / patch / 34662

patch, 2 partsremote-hg: shared repo upgrade fix

Subject: [PATCH 0/2] remote-hg: shared repo upgrade fix

## tl;dr

6 messages between Aug 9, 2013 and Aug 9, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Felipe Contreras· Aug 9, 2013, 20:02 UTC · lore

Hi,

Here's my proposal split into two patches, as I suggested. With the fix for when there's more than one shared repo.

Felipe Contreras (2):
  remote-hg: simplify share repo setup
  remote-hg: add shared repo upgrade

 contrib/remote-helpers/git-remote-hg | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

-- 
1.8.3.267.gbb4989f

Felipe Contreras· Aug 9, 2013, 20:03 UTC · re: Felipe Contreras · lore

[PATCH 1/2] remote-hg: simplify shared repo setup

We don't need the initial clone, if the repository is shared, pulling from the child updates the parent's storage; it's exactly the same as cloning.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Show changes to contrib/remote-helpers/git-remote-hg +6 −5

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 0194c67..cfd4f53 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -391,11 +391,12 @@ def get_repo(url, alias):
             os.makedirs(dirname)
     else:
         shared_path = os.path.join(gitdir, 'hg')
-        if not os.path.exists(shared_path):
-            try:
-                hg.clone(myui, {}, url, shared_path, update=False, pull=True)
-            except:
-                die('Repository error')
+
+        # setup shared repo (if not there)
+        try:
+            hg.peer(myui, {}, shared_path, create=True)
+        except error.RepoError:
+            pass
 
         if not os.path.exists(dirname):
             os.makedirs(dirname)

-- 
1.8.3.267.gbb4989f

Felipe Contreras· Aug 9, 2013, 20:03 UTC · re: Felipe Contreras · lore

[PATCH 2/2] remote-hg: add shared repo upgrade

6796d49 (remote-hg: use a shared repository store) introduced a bug by making the shared repository '.git/hg', which is already used before that patch, so clones that happened before that patch, fail after that patch, because there's no shared Mercurial repo.

It's trivial to upgrade to the new organization by copying the Mercurial repo from one of the remotes (e.g. 'origin'), so let's do so.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 11 +++++++++++
 1 file changed, 11 insertions(+)

Show changes to contrib/remote-helpers/git-remote-hg +11 −0

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index cfd4f53..9ec13da 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -392,6 +392,17 @@ def get_repo(url, alias):
     else:
         shared_path = os.path.join(gitdir, 'hg')
 
+        # check and upgrade old organization
+        hg_path = os.path.join(shared_path, '.hg')
+        if os.path.exists(shared_path) and not os.path.exists(hg_path):
+            repos = os.listdir(shared_path)
+            for x in repos:
+                local_hg = os.path.join(shared_path, x, 'clone', '.hg')
+                if not os.path.exists(local_hg):
+                    continue
+                shutil.copytree(local_hg, hg_path)
+                break
+
         # setup shared repo (if not there)
         try:
             hg.peer(myui, {}, shared_path, create=True)

-- 
1.8.3.267.gbb4989f

Antoine Pelisse· Aug 9, 2013, 20:19 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/2] remote-hg: add shared repo upgrade

On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> 6796d49 (remote-hg: use a shared repository store) introduced a bug by
> making the shared repository '.git/hg', which is already used before
> that patch, so clones that happened before that patch, fail after that
> patch, because there's no shared Mercurial repo.

Does that still hold ? You are creating the shared_path repository just below, so it should work without the patch. The real reason for this patch is to avoid having to re-clone from a potential slow source, is it not ?

Show 11 quoted lines

> +        # check and upgrade old organization
> +        hg_path = os.path.join(shared_path, '.hg')
> +        if os.path.exists(shared_path) and not os.path.exists(hg_path):
> +            repos = os.listdir(shared_path)
> +            for x in repos:
> +                local_hg = os.path.join(shared_path, x, 'clone', '.hg')
> +                if not os.path.exists(local_hg):
> +                    continue
> +                shutil.copytree(local_hg, hg_path)
> +                break
> +

By the way, I liked my version better, that is:

if os.path.exists(local_hg):
    shutil.copytree(local_hg, hg_path)
    break

Simplifying the if not condition: continue else: break

>          # setup shared repo (if not there)
>          try:
>              hg.peer(myui, {}, shared_path, create=True)

Felipe Contreras· Aug 9, 2013, 20:24 UTC · re: Antoine Pelisse · lore

Re: [PATCH 2/2] remote-hg: add shared repo upgrade

On Fri, Aug 9, 2013 at 3:19 PM, Antoine Pelisse <apelisse@gmail.com> wrote:

Show 11 quoted lines

> On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> 6796d49 (remote-hg: use a shared repository store) introduced a bug by
>> making the shared repository '.git/hg', which is already used before
>> that patch, so clones that happened before that patch, fail after that
>> patch, because there's no shared Mercurial repo.
>
> Does that still hold ? You are creating the shared_path repository
> just below, so it should work without the patch.
> The real reason for this patch is to avoid having to re-clone from a
> potential slow source, is it not ?

Yeah, that's true.

Show 19 quoted lines

>> +        # check and upgrade old organization
>> +        hg_path = os.path.join(shared_path, '.hg')
>> +        if os.path.exists(shared_path) and not os.path.exists(hg_path):
>> +            repos = os.listdir(shared_path)
>> +            for x in repos:
>> +                local_hg = os.path.join(shared_path, x, 'clone', '.hg')
>> +                if not os.path.exists(local_hg):
>> +                    continue
>> +                shutil.copytree(local_hg, hg_path)
>> +                break
>> +
>
> By the way, I liked my version better, that is:
>
> if os.path.exists(local_hg):
>     shutil.copytree(local_hg, hg_path)
>     break
>
> Simplifying the if not condition: continue else: break

I prefer my version because if there's any need to add more lines, they don't have to be indented. That's why a lot of code ends up having unnecessary indentation.

-- 
Felipe Contreras

Junio C Hamano· Aug 9, 2013, 21:47 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/2] remote-hg: add shared repo upgrade

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 14 quoted lines

> On Fri, Aug 9, 2013 at 3:19 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> 6796d49 (remote-hg: use a shared repository store) introduced a bug by
>>> making the shared repository '.git/hg', which is already used before
>>> that patch, so clones that happened before that patch, fail after that
>>> patch, because there's no shared Mercurial repo.
>>
>> Does that still hold ? You are creating the shared_path repository
>> just below, so it should work without the patch.
>> The real reason for this patch is to avoid having to re-clone from a
>> potential slow source, is it not ?
>
> Yeah, that's true.

So both of you are happy if we apply 1/2

Message-ID: <1376078581-24766-2-git-send-email-felipe.contreras@gmail.com>

and this one with an updated log message?