Re: [PATCH v3] remotes-hg: bugfix for fetching non local remotes

2013-07-26 Thread Jörn Hees

On 25 Jul 2013, at 21:12, Felipe Contreras  wrote:
>> […]
>> ---
>> contrib/remote-helpers/git-remote-hg | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/contrib/remote-helpers/git-remote-hg 
>> b/contrib/remote-helpers/git-remote-hg
>> index 0194c67..f4e9d1c 100755
>> --- a/contrib/remote-helpers/git-remote-hg
>> +++ b/contrib/remote-helpers/git-remote-hg
>> @@ -390,7 +390,7 @@ def get_repo(url, alias):
>> if not os.path.exists(dirname):
>> os.makedirs(dirname)
>> else:
>> -shared_path = os.path.join(gitdir, 'hg')
>> +shared_path = os.path.join(gitdir, 'hg', '.shared')
>> if not os.path.exists(shared_path):
>> try:
>> hg.clone(myui, {}, url, shared_path, update=False, pull=True)
>> --
>> 1.8.3.4
> 
> I don't like this approach because if it's a huge repository the user
> would have to clone again, not only if he was using v1.8.3, but also
> if he was using the latest and greatest (because you are changing the
> location again). t's relatively trivial to move from the old to the
> shared organization, so that's what I vote for. Besides, I don't see
> the point of having a '.shared/.hg' directory, and nothing else on
> that '.shared' folder.

Agreed… it just was the shortest possible fix with an in my POV minor 
optimisation drawback of once refetching...

--
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-26 Thread Jörn Hees
On 25 Jul 2013, at 23:10, Antoine Pelisse  wrote:

> On Thu, Jul 25, 2013 at 10:40 PM, Felipe Contreras
>  wrote:
>> That's true. Maybe something like:
>> 
>> for x in repos:
>>  local_hg = os.path.join(shared_path, x, 'clone', '.hg')
>>  if os.path.exists(local_hg):
>>shutil.copytree(local_hg, hg_path)
>>break
> 
> I think that would work

yupp, might work, but holding you liable to the same optimality restriction you 
imposed on me before:
This will still refetch the whole repo once if it was cloned from a local hg 
repo first (they don't have a clone subdir).
Shouldn't we then also go through the additional effort and copy the .hg dir 
from local remotes when a "remote remote" is added and there's no other remote 
remote?

j

--
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-25 Thread Junio C Hamano
Antoine Pelisse  writes:

> On Thu, Jul 25, 2013 at 10:40 PM, Felipe Contreras
>  wrote:
>> That's true. Maybe something like:
>>
>> for x in repos:
>>   local_hg = os.path.join(shared_path, x, 'clone', '.hg')
>>   if os.path.exists(local_hg):
>> shutil.copytree(local_hg, hg_path)
>> break
>
> I think that would work, but I think the patch from Joern Hees would
> have to be reverted first (as it's merged in next)

The expectation is that not all things in 'next' today will be in
1.8.4 anyway, so it is perfectly OK to revert it as needed.

Thanks.


--
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-25 Thread Antoine Pelisse
On Thu, Jul 25, 2013 at 10:40 PM, Felipe Contreras
 wrote:
> That's true. Maybe something like:
>
> for x in repos:
>   local_hg = os.path.join(shared_path, x, 'clone', '.hg')
>   if os.path.exists(local_hg):
> shutil.copytree(local_hg, hg_path)
> break

I think that would work, but I think the patch from Joern Hees would
have to be reverted first (as it's merged in next)

Cheers,
Antoine
--
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-25 Thread Felipe Contreras
On Thu, Jul 25, 2013 at 2:53 PM, Antoine Pelisse  wrote:
> On Thu, Jul 25, 2013 at 9:12 PM, Felipe Contreras
>  wrote:
>> Besides, I don't see
>> the point of having a '.shared/.hg' directory, and nothing else on
>> that '.shared' folder.
>
> Is it not already true about the ".git/hg/$alias/clone/" directory ?

Yeah, but that directory is kind of useful. Somebody might want to
clone that, and it's self-explanatory; "Where is the clone of that
Mercurial remote? Oh, there".

>> So, here's my patch. If only Junio read them.
>>
>> Subject: [PATCH] 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.
>
> I agree with you that we should consider migration. But there's
> another use-case I think can fail.
> What happens with the following:
>
> git clone hg::/my/hg/repo
> cd repo && git remote add newremote hg::http://some/hg/url
>
> Git clone will create .git/hg/origin and with no hg clone (because
> it's a local repository), and then create marks-file in there.
>
>> Reported-by: Joern Hees 
>> Signed-off-by: Felipe Contreras 
>> ---
>>  contrib/remote-helpers/git-remote-hg.py | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/contrib/remote-helpers/git-remote-hg.py
>> b/contrib/remote-helpers/git-remote-hg.py
>> index 0194c67..57a8ec4 100755
>> --- a/contrib/remote-helpers/git-remote-hg.py
>> +++ b/contrib/remote-helpers/git-remote-hg.py
>> @@ -396,6 +396,13 @@ def get_repo(url, alias):
>>  hg.clone(myui, {}, url, shared_path, update=False, 
>> pull=True)
>>  except:
>>  die('Repository error')
>> +else:
>> +# check and upgrade old organization
>> +hg_path = os.path.join(shared_path, '.hg')
>> +if not os.path.exists(hg_path):
>> +repos = os.listdir(shared_path)
>> +local_hg = os.path.join(shared_path, repos[0], 'clone', 
>> '.hg')
>> +shutil.copytree(local_hg, hg_path)
>
> With the use-case I described above, I think shutil.copytree() would
> raise an exception because local_hg doesn't exist.

That's true. Maybe something like:

for x in repos:
  local_hg = os.path.join(shared_path, x, 'clone', '.hg')
  if os.path.exists(local_hg):
shutil.copytree(local_hg, hg_path)
break

-- 
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-25 Thread Antoine Pelisse
On Thu, Jul 25, 2013 at 9:12 PM, Felipe Contreras
 wrote:
> Besides, I don't see
> the point of having a '.shared/.hg' directory, and nothing else on
> that '.shared' folder.

Is it not already true about the ".git/hg/$alias/clone/" directory ?

> So, here's my patch. If only Junio read them.
>
> Subject: [PATCH] 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.

I agree with you that we should consider migration. But there's
another use-case I think can fail.
What happens with the following:

git clone hg::/my/hg/repo
cd repo && git remote add newremote hg::http://some/hg/url

Git clone will create .git/hg/origin and with no hg clone (because
it's a local repository), and then create marks-file in there.

> Reported-by: Joern Hees 
> Signed-off-by: Felipe Contreras 
> ---
>  contrib/remote-helpers/git-remote-hg.py | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-hg.py
> b/contrib/remote-helpers/git-remote-hg.py
> index 0194c67..57a8ec4 100755
> --- a/contrib/remote-helpers/git-remote-hg.py
> +++ b/contrib/remote-helpers/git-remote-hg.py
> @@ -396,6 +396,13 @@ def get_repo(url, alias):
>  hg.clone(myui, {}, url, shared_path, update=False, pull=True)
>  except:
>  die('Repository error')
> +else:
> +# check and upgrade old organization
> +hg_path = os.path.join(shared_path, '.hg')
> +if not os.path.exists(hg_path):
> +repos = os.listdir(shared_path)
> +local_hg = os.path.join(shared_path, repos[0], 'clone', 
> '.hg')
> +shutil.copytree(local_hg, hg_path)

With the use-case I described above, I think shutil.copytree() would
raise an exception because local_hg doesn't exist.
--
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 v3] remotes-hg: bugfix for fetching non local remotes

2013-07-25 Thread Felipe Contreras
On Wed, Jul 24, 2013 at 7:42 PM, Joern Hees  wrote:
> 6796d49 introduced a bug by making shared_path == ".git/hg' which
> will most likely exist already, causing a new remote never to be
> cloned and subsequently causing hg.share to fail with error msg:
> "mercurial.error.RepoError: repository .git/hg not found"
>
> Changing shared_path to ".git/hg/.shared" will solve this problem
> and create a shared local mercurial repository for non local remotes.
> The initial dot circumvents a name clash problem should a remote be
> called "shared".
>
> Signed-off-by: Joern Hees 
> Mentored-by: Antoine Pelisse 
> Thanks-to: Junio C Hamano 
> ---
>  contrib/remote-helpers/git-remote-hg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/remote-helpers/git-remote-hg 
> b/contrib/remote-helpers/git-remote-hg
> index 0194c67..f4e9d1c 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -390,7 +390,7 @@ def get_repo(url, alias):
>  if not os.path.exists(dirname):
>  os.makedirs(dirname)
>  else:
> -shared_path = os.path.join(gitdir, 'hg')
> +shared_path = os.path.join(gitdir, 'hg', '.shared')
>  if not os.path.exists(shared_path):
>  try:
>  hg.clone(myui, {}, url, shared_path, update=False, pull=True)
> --
> 1.8.3.4

I don't like this approach because if it's a huge repository the user
would have to clone again, not only if he was using v1.8.3, but also
if he was using the latest and greatest (because you are changing the
location again). It's relatively trivial to move from the old to the
shared organization, so that's what I vote for. Besides, I don't see
the point of having a '.shared/.hg' directory, and nothing else on
that '.shared' folder.

So, here's my patch. If only Junio read them.

Subject: [PATCH] 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.

Reported-by: Joern Hees 
Signed-off-by: Felipe Contreras 
---
 contrib/remote-helpers/git-remote-hg.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg.py
b/contrib/remote-helpers/git-remote-hg.py
index 0194c67..57a8ec4 100755
--- a/contrib/remote-helpers/git-remote-hg.py
+++ b/contrib/remote-helpers/git-remote-hg.py
@@ -396,6 +396,13 @@ def get_repo(url, alias):
 hg.clone(myui, {}, url, shared_path, update=False, pull=True)
 except:
 die('Repository error')
+else:
+# check and upgrade old organization
+hg_path = os.path.join(shared_path, '.hg')
+if not os.path.exists(hg_path):
+repos = os.listdir(shared_path)
+local_hg = os.path.join(shared_path, repos[0], 'clone', '.hg')
+shutil.copytree(local_hg, hg_path)

 if not os.path.exists(dirname):
 os.makedirs(dirname)
-- 
1.8.3.3

-- 
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


[PATCH v3] remotes-hg: bugfix for fetching non local remotes

2013-07-24 Thread Joern Hees
6796d49 introduced a bug by making shared_path == ".git/hg' which
will most likely exist already, causing a new remote never to be
cloned and subsequently causing hg.share to fail with error msg:
"mercurial.error.RepoError: repository .git/hg not found"

Changing shared_path to ".git/hg/.shared" will solve this problem
and create a shared local mercurial repository for non local remotes.
The initial dot circumvents a name clash problem should a remote be
called "shared".

Signed-off-by: Joern Hees 
Mentored-by: Antoine Pelisse 
Thanks-to: Junio C Hamano 
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 0194c67..f4e9d1c 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -390,7 +390,7 @@ def get_repo(url, alias):
 if not os.path.exists(dirname):
 os.makedirs(dirname)
 else:
-shared_path = os.path.join(gitdir, 'hg')
+shared_path = os.path.join(gitdir, 'hg', '.shared')
 if not os.path.exists(shared_path):
 try:
 hg.clone(myui, {}, url, shared_path, update=False, pull=True)
-- 
1.8.3.4

--
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