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

2013-08-09 Thread Junio C Hamano
Felipe Contreras  writes:

> On Fri, Aug 9, 2013 at 3:19 PM, Antoine Pelisse  wrote:
>> On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras
>>  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.contre...@gmail.com>

and this one with an updated log message?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-09 Thread Felipe Contreras
On Fri, Aug 9, 2013 at 3:19 PM, Antoine Pelisse  wrote:
> On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras
>  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.

>> +# 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-09 Thread Antoine Pelisse
On Fri, Aug 9, 2013 at 10:03 PM, Felipe Contreras
 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 ?

> +# 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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-09 Thread Felipe Contreras
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 
---
 contrib/remote-helpers/git-remote-hg | 11 +++
 1 file changed, 11 insertions(+)

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html