Re: [PATCH] remote-hg: Fix cloning and sharing bug

["Jörn Hees" <dev@joernhees.de>]

Hi,

On 4 Aug 2013, at 12:38, Antoine Pelisse <apelisse@gmail.com> wrote:
> […]
> I also decided to always clone local repositories because what Jörn Hees
> said makes sense:
> If you have a local clone of a big repository, and then want to add a slow
> remote, you would have to reclone everything.
> I think the trade-off is good, because clone from local should not be that
> time expensive (maybe it can be on disk-space though).

I was working on a similar patch in the meantime, this point was the only thing that
kept me from submitting… Can someone of you think of an easy way to do this lazily
on the first non-local remote being added? 
In case we don't have a non-local clone (a mercurial dir with a clone subdir) yet, we
would try to go though the local mercurial remotes and then clone them… Just would
need a way to get their URLs. I thought about going through all "git remote -v" 
This way we wouldn't need to copy by default (bad for big repos), but could still do this
in a cheap way if a slow remote is added later on.

Btw, is there any reason why we don't just use the local mercurial remotes as shared
repo? Cause it's not under our git dir and might be deleted?


> […]
> contrib/remote-helpers/git-remote-hg |   47 ++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 0194c67..487c13d 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -385,33 +385,42 @@ def get_repo(url, alias):
> 
>     extensions.loadall(myui)
> 
> -    if hg.islocal(url) and not os.environ.get('GIT_REMOTE_HG_TEST_REMOTE'):
> -        repo = hg.repository(myui, url)
> -        if not os.path.exists(dirname):
> -            os.makedirs(dirname)
> -    else:
> -        shared_path = os.path.join(gitdir, 'hg')
> -        if not os.path.exists(shared_path):
> +    hgdir = os.path.join(gitdir, 'hg')
> +    try:
> +        os.mkdir(hgdir)
> +    except OSError:
> +        pass
> +
> +    shared_path = os.path.join(hgdir, '.shared')

I thought we had agreed to use .git/hg as the shared directory before? (so that
a clone into that dir would end up in .git/hg/.hg instead of .git/hg/.shared/.hg)


> +    if not os.path.exists(shared_path):
> +        for remote in os.listdir(hgdir):
> +            try:
> +                hg.clone(myui, {}, os.path.join(hgdir, remote, 'clone'),
> +                         shared_path, update=False, pull=True)
> +                break
> +            except error.RepoError:
> +                pass
> +        else:

Elegant use of the for-else clause, but to my experience confuses many people.

This would also be the place to check for local remotes after not finding already
cloned non-local remotes (the lazy approach mentioned above). As this would
cause nested "for-else" loops, i'd rather repeatedly check for existence of .git/hg/.hg
and list the several fallback in order, the last one being this one:

>             try:
>                 hg.clone(myui, {}, url, shared_path, update=False, pull=True)
>             except:
>                 die('Repository error')

If you want i'll send around my patch as RFC.
In the end i don't care which one is accepted and how, most important that one is
accepted to fix the bug.

Cheers,
Jörn