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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 5, 2013, 21:02 UTC
Message-ID: <7vwqnzj1gp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1375730567-3240-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

Show 20 quoted lines

> From: Felipe Contreras <felipe.contreras@gmail.com>
>
> 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.
>
> ...
> +        # 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)

The log message talks about "one of the remotes (e.g. 'origin')" and you are creating a copy of one that you encounter in os.listdir(); I may be missing some underlying assumptions but I wonder what happens after you copy and create hg_path directory, which does not change in the loop, to the remaining iterations of the loop. Is the untold and obvious-to-those-who-are-familiar-with-this-codepath assumption that it is guaranteed that there is at most one "*/clone/.hg" under shared_path?

Show 8 quoted lines

> +        # 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)